diff --git a/clc/modules/euare/src/main/java/com/eucalyptus/auth/AuthenticationProperties.java b/clc/modules/euare/src/main/java/com/eucalyptus/auth/AuthenticationProperties.java index 4c46f4296c7..b3ffed39f95 100644 --- a/clc/modules/euare/src/main/java/com/eucalyptus/auth/AuthenticationProperties.java +++ b/clc/modules/euare/src/main/java/com/eucalyptus/auth/AuthenticationProperties.java @@ -137,6 +137,9 @@ public class AuthenticationProperties { @ConfigurableField( description = "Maximum size for an IAM policy (bytes)", initial = DEFAULT_MAX_POLICY_SIZE_TEXT ) public static volatile int MAX_POLICY_SIZE = Integer.parseInt( DEFAULT_MAX_POLICY_SIZE_TEXT ); + @ConfigurableField( description = "Use strict validation for IAM policy syntax", initial = "true" ) + public static volatile boolean STRICT_POLICY_VALIDATION = true; + private static AtomicLong DEFAULT_PASSWORD_EXPIRY_MILLIS = new AtomicLong( TimeUnit.DAYS.toMillis( 60 ) ); private static AtomicLong AUTHORIZATION_EXPIRY_MILLIS = new AtomicLong( TimeUnit.SECONDS.toMillis( 5 ) ); @@ -242,6 +245,11 @@ public int getSigningCertificateLimitSpi( ) { public int getPolicySizeLimitSpi( ) { return MAX_POLICY_SIZE; } + + @Override + public boolean getUseValidatingPolicyParserSpi( ) { + return STRICT_POLICY_VALIDATION; + } } public static final class AuthenticationIntervalPropertyChangeListener implements PropertyChangeListener { diff --git a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/AuthenticationLimitProvider.java b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/AuthenticationLimitProvider.java index ff2aa58a0f2..5a7fc9aafb0 100644 --- a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/AuthenticationLimitProvider.java +++ b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/AuthenticationLimitProvider.java @@ -36,6 +36,8 @@ public interface AuthenticationLimitProvider { int getPolicySizeLimitSpi( ); + boolean getUseValidatingPolicyParserSpi( ); + class Values { public static long getDefaultPasswordExpiry( ) { return getLongValue( AuthenticationLongProperties.DEFAULT_PASSWORD_EXPIRY ); @@ -57,6 +59,10 @@ public static int getPolicySizeLimit( ) { public static int getOpenIdConnectProviderThumprintLimit( ) { return 5; } + public static boolean getUseValidatingPolicyParser( ) { + return getBooleanValue( AuthenticationBooleanProperties.USE_VALIDATING ); + } + static int getIntValue( final NonNullFunction valueFunction ) { return getValue( valueFunction ); } @@ -65,6 +71,10 @@ static long getLongValue( final NonNullFunction valueFunction ) { + return getValue( valueFunction ); + } + static VT getValue( final NonNullFunction valueFunction ) { @@ -105,4 +115,18 @@ public Long apply( final AuthenticationLimitProvider authenticationLimitProvider } }, } + + enum AuthenticationBooleanProperties implements NonNullFunction { + USE_VALIDATING { + @Nonnull + @Override + public Boolean apply( final AuthenticationLimitProvider authenticationLimitProvider ) { + return authenticationLimitProvider.getUseValidatingPolicyParserSpi(); + } + } + } + + } + + diff --git a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyEngineImpl.java b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyEngineImpl.java index 9b6cd6c705e..1cfc6ce509c 100644 --- a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyEngineImpl.java +++ b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyEngineImpl.java @@ -122,7 +122,7 @@ * The implementation of policy engine, which evaluates a request against specified policies. */ public class PolicyEngineImpl implements PolicyEngine { - + private static final Logger LOG = Logger.getLogger( PolicyEngineImpl.class ); private static final Cache> authorizationCache = CacheBuilder @@ -145,11 +145,11 @@ private enum Decision { DENY, // explicit deny ALLOW, // explicit allow } - + private interface Matcher { boolean match( String pattern, String instance ); } - + private static final Matcher PATTERN_MATCHER = new Matcher( ) { @Override public boolean match( String pattern, String instance ) { @@ -160,7 +160,7 @@ public boolean match( String pattern, String instance ) { return Pattern.matches( pattern, instance ); } }; - + private static final Matcher ADDRESS_MATCHER = new Matcher( ) { @Override public boolean match( String pattern, String instance ) { @@ -170,7 +170,7 @@ public boolean match( String pattern, String instance ) { return AddressUtil.addressRangeMatch( pattern, instance ); } }; - + private static final Matcher SERVER_CERTIFICATE_MATCHER = new Matcher( ) { @Override public boolean match( String pattern, String instance ) { @@ -179,7 +179,7 @@ public boolean match( String pattern, String instance ) { // instance is in full ARN form while pattern is /{cert_name}; if(! instance.startsWith("arn:aws:iam::")) return false; - + int idx = instance.indexOf(":server-certificate"); if(idx<0) return false; @@ -191,7 +191,7 @@ public boolean match( String pattern, String instance ) { return Pattern.matches( pattern, certPathAndName ); } }; - + public PolicyEngineImpl( @Nonnull final Supplier enableSystemQuotas, @Nonnull final Supplier region @@ -213,7 +213,7 @@ public PolicyEngineImpl( * The authorization evaluation algorithm is a combination of AWS IAM policy evaluation logic and * AWS inter-account permission checking logic (including EC2 image and snapshot permission, and * S3 bucket ACL and bucket policy). The algorithm is described in the following: - * + * * 1. If request user is system admin, access is GRANTED. * 2. Otherwise, check global (inter-account) authorizations, which are attached to account admin. * If explicitly denied, access is DENIED. @@ -225,7 +225,7 @@ public PolicyEngineImpl( * 4. Otherwise, check local (intra-account) authorizations. * If explicitly or default denied, access is DENIED. * If explicitly allowed, access is GRANTED. - * + * * (non-Javadoc) * @see com.eucalyptus.auth.api.PolicyEngine#evaluateAuthorization(java.lang.Class, java.lang.String, java.lang.String) */ @@ -238,11 +238,11 @@ public void evaluateAuthorization( @Nonnull final AuthEvaluationContext context try { final AuthEvaluationContextImpl evaluationContext = (AuthEvaluationContextImpl)context; if ( Decision.ALLOW != evaluateResourceAuthorization( - evaluationContext, + evaluationContext, authorizationMatch, !evaluationContext.isSystemUser( ), - resourceAccountNumber, - resourceName, + resourceAccountNumber, + resourceName, contracts ) ) { throw new AuthException( AuthException.ACCESS_DENIED ); } @@ -252,7 +252,7 @@ public void evaluateAuthorization( @Nonnull final AuthEvaluationContext context } catch ( Exception e ) { LOG.debug( e, e ); throw new AuthException( "An error occurred while trying to evaluate policy for resource access", e ); - } + } } @Override @@ -326,7 +326,7 @@ public void evaluateQuota( @Nonnull final AuthEvaluationContext context, quantity ); } } catch ( AuthException e ) { - //throw by the policy engine implementation + //throw by the policy engine implementation throw e; } catch ( Exception e ) { throw new AuthException( "An error occurred while trying to evaluate policy for resource allocation.", e ); @@ -440,11 +440,11 @@ private Decision processAuthorizations( @Nonnull final List auth return Decision.DENY; } else { result = Decision.ALLOW; - } + } } return result; } - + private boolean matchActions( Authorization auth, String action ) throws AuthException { return evaluateElement( matchOne( auth.getActions( ), action, PATTERN_MATCHER ), auth.isNotAction( ) ); } @@ -480,11 +480,11 @@ private boolean matchResources( @Nonnull Authorization auth, return evaluateElement( matchOne( auth.getPolicyVariables( ), auth.getResources( ), resource, PATTERN_MATCHER ), auth.isNotResource( ) ); } } - + private static boolean matchOne( Set patterns, String instance, Matcher matcher ) throws AuthException { return matchOne( Collections.emptySet( ), patterns, instance, matcher ); } - + private static boolean matchOne( Set variables, Set patterns, String instance, Matcher matcher ) throws AuthException { for ( String pattern : patterns ) { if ( matcher.match( variableExplode( variables, pattern ) , instance ) ) { @@ -496,18 +496,18 @@ private static boolean matchOne( Set variables, Set patterns, St private static String variableExplode( Set variables, String text ) throws AuthException { if ( variables.isEmpty( ) ) return text; - + String result = text; for ( final String variable : variables ) { final String variableValue = PolicyVariables.getPolicyVariable( variable ).evaluate( ); //TODO: variable values cannot currently contain ? or *, if they could we would need //TODO: to escape the values when they were used in regex matches - result = result.replace( variable, variableValue ); + result = result.replace( variable, variableValue ); } - + return result; } - + private String resolveAccount( final String accountNumberOrAlias ) { return accountResolver.apply( accountNumberOrAlias ); } @@ -515,16 +515,16 @@ private String resolveAccount( final String accountNumberOrAlias ) { private boolean evaluateElement( boolean patternMatched, boolean isNot ) { return ( ( patternMatched && !isNot ) || ( !patternMatched && isNot ) ); } - + /** * Evaluate conditions for an authorization. */ - private boolean evaluateConditions( + private boolean evaluateConditions( final Set policyVariables, - final List conditions, - final String action, - final CachedKeyEvaluator keyEval, - final ContractKeyEvaluator contractEval + final List conditions, + final String action, + final CachedKeyEvaluator keyEval, + final ContractKeyEvaluator contractEval ) throws AuthException { for ( Condition cond : conditions ) { ConditionOp op = Conditions.getOpInstance( cond.getType( ) ); @@ -547,10 +547,10 @@ private boolean evaluateConditions( } return true; } - + /** * Process each of the quota authorizations. If any of them is exceeded, deny access. - * + * * @param quotas The quota authorizations * @param action The request action. * @param resourceType The resource type for allocation @@ -603,10 +603,10 @@ private void processQuotas( final List> quotas } } } - + /** * Get the principal ID for an authorization based on scope. - * + * * @param scope The scope * @return The principal ID (account, group or user) * @throws AuthException for any error @@ -749,7 +749,7 @@ static List authorizations( final PolicyVersion policy, final boo return authorizationCache.get( policy.getPolicyHash( ), new Callable>() { @Override public ImmutableList call() throws Exception { - return ImmutableList.copyOf( ( resourcePolicy ? PolicyParser.getResourceInstance( ) : PolicyParser.getInstance( ) ).parse( policy.getPolicy( ) ).getAuthorizations( ) ); + return ImmutableList.copyOf( ( resourcePolicy ? PolicyParser.getLaxResourceInstance( ) : PolicyParser.getLaxInstance( ) ).parse( policy.getPolicy( ) ).getAuthorizations( ) ); } } ); } catch ( final ExecutionException e ) { diff --git a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyParser.java b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyParser.java index 6fb690183e2..643c10ba6cf 100644 --- a/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyParser.java +++ b/clc/modules/msgs/src/main/java/com/eucalyptus/auth/policy/PolicyParser.java @@ -64,6 +64,7 @@ import static org.hamcrest.Matchers.notNullValue; import static com.eucalyptus.auth.principal.Principal.PrincipalType; +import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -91,6 +92,7 @@ import com.eucalyptus.auth.principal.Authorization.EffectType; import com.eucalyptus.auth.principal.Condition; import com.eucalyptus.records.Logs; +import com.eucalyptus.util.Json; import com.eucalyptus.util.Parameters; import com.google.common.base.Joiner; import com.google.common.base.Optional; @@ -106,9 +108,9 @@ public class PolicyParser { private static final Logger LOG = Logger.getLogger( PolicyParser.class ); - + private static final Pattern VARIABLE_MATCHER = Pattern.compile( "\\$\\{(?:[a-zA-Z][a-zA-Z0-9]*:[a-zA-Z]+|[*]|[?]|[$])}" ); - + private enum PolicyAttachmentType { Identity( true/*requireResource*/, false/*requirePrincipal*/ ), Resource( false/*requireResource*/, true/*requirePrincipal*/ ); @@ -130,37 +132,51 @@ public boolean isPrincipalRequired() { return requirePrincipal; } } - + private static final class PolicyParseContext { private final String version; private PolicyParseContext( final String version ) { this.version = version; } - + public String getVersion() { return version; } } private final PolicyAttachmentType attachmentType; - + + private final boolean validating; // true for json validation + + public static PolicyParser getLaxInstance( ) { + return new PolicyParser( PolicyAttachmentType.Identity, false ); + } + + public static PolicyParser getLaxResourceInstance( ) { + return new PolicyParser( PolicyAttachmentType.Resource, false ); + } + public static PolicyParser getInstance( ) { - return new PolicyParser( PolicyAttachmentType.Identity ); + return new PolicyParser( PolicyAttachmentType.Identity, true ); } public static PolicyParser getResourceInstance( ) { - return new PolicyParser( PolicyAttachmentType.Resource ); + return new PolicyParser( PolicyAttachmentType.Resource, true ); } - private PolicyParser( final PolicyAttachmentType attachmentType ) { + private PolicyParser( + final PolicyAttachmentType attachmentType, + final boolean validating + ) { this.attachmentType = attachmentType; + this.validating = validating; } - + /** * Parse the input policy text and returns an PolicyEntity object that * represents the policy internally. - * + * * @param policy The input policy text. * @return The parsed the policy entity. * @throws PolicyParseException for policy syntax error. @@ -172,6 +188,14 @@ public PolicyPolicy parse( String policy ) throws PolicyParseException { if ( policy.length( ) > AuthenticationLimitProvider.Values.getPolicySizeLimit( ) ) { throw new PolicyParseException( PolicyParseException.SIZE_TOO_LARGE ); } + if ( validating && AuthenticationLimitProvider.Values.getUseValidatingPolicyParser( ) ) { + try { // parser that ensures policy is valid json + Json.parse( policy ); + } catch ( IOException e ) { + Debugging.logError( LOG, e, "Syntax error in input policy" ); + throw new PolicyParseException( e.getMessage( ), e ); + } + } try { JSONObject policyJsonObj = JSONObject.fromObject( policy ); String version = JsonUtils.getByType( String.class, policyJsonObj, PolicySpec.VERSION ); @@ -183,17 +207,17 @@ public PolicyPolicy parse( String policy ) throws PolicyParseException { throw new PolicyParseException( e ); } } - + /** * Parse all statements. - * + * * @param policy Input policy text. * @return A list of statement entities from the input policy. * @throws JSONException for syntax error. */ - private List parseStatements( + private List parseStatements( final PolicyParseContext context, - final JSONObject policy + final JSONObject policy ) throws JSONException { List objs; if ( policy.get( PolicySpec.STATEMENT ) instanceof JSONObject ) { @@ -207,20 +231,20 @@ private List parseStatements( } return authorizations; } - + /** * Parse one statement. A statement is internally represented by a list of authorizations * and a list of conditions. The action list and the resource list of the statement are * parsed into authorizations (which action is allowed on which resource). The condition * block is translated into conditions (keys, values and their relationships). - * + * * @param statement The JSON object of the statement * @return The parsed statement entity * @throws JSONException for syntax error */ private List parseStatement( final PolicyParseContext context, - final JSONObject statement + final JSONObject statement ) throws JSONException { // statement ID String sid = JsonUtils.getByType( String.class, statement, PolicySpec.SID ); @@ -243,7 +267,7 @@ private List parseStatement( * @throws JSONException for syntax error. */ private PolicyPrincipal parsePrincipal( - final JSONObject statement + final JSONObject statement ) { final String principalElement = JsonUtils.checkBinaryOption( statement, PolicySpec.PRINCIPAL, PolicySpec.NOTPRINCIPAL, attachmentType.isPrincipalRequired() ); @@ -275,7 +299,7 @@ private PolicyPrincipal parsePrincipal( /** * Parse the authorization part of a statement. - * + * * @param statement The input statement in JSON object. * @param effect The effect of the statement * @return A list of authorization entities. @@ -303,7 +327,7 @@ private List parseAuthorizations( // decompose actions and resources and re-combine them into a list of authorizations return decomposeStatement( context, effect, sid, actionElement, actions, resourceElement, resources, principal, conditions ); } - + /** * The algorithm of decomposing the actions and resources of a statement into authorizations: * 1. Group actions into different vendors. @@ -371,7 +395,7 @@ private List decomposeStatement( } } if ( !added ) { - results.add( new PolicyAuthorization( + results.add( new PolicyAuthorization( sid, EffectType.valueOf( effect ), principal, conditions, actionSet, notAction, variableSet( context, conditions ) ) ); } } @@ -380,20 +404,20 @@ private List decomposeStatement( /** * Parse the conditions of a statement - * + * * @param statement The JSON object of the statement * @param effect The effect of the statement * @return A list of parsed condition entity. * @throws JSONException for syntax error. */ private List parseConditions( - final JSONObject statement, - final String effect + final JSONObject statement, + final String effect ) throws JSONException { JSONObject condsObj = JsonUtils.getByType( JSONObject.class, statement, PolicySpec.CONDITION ); boolean isQuota = EffectType.Limit.name( ).equals( effect ); List results = Lists.newArrayList( ); - if ( condsObj != null ) { + if ( condsObj != null ) { for ( Object t : condsObj.keySet( ) ) { String type = ( String ) t; Class typeClass = checkConditionType( type ); @@ -413,7 +437,7 @@ private List parseConditions( /** * Check validity of the action value. - * + * * @param action The input action pattern. * @return The vendor of the action. * @throws JSONException for any error @@ -431,7 +455,7 @@ private String checkAction( String action ) throws JSONException { /** * Check the validity of a condition type. - * + * * @param type The condition type string. * @return The class represents the condition type. * @throws JSONException for syntax error. @@ -446,10 +470,10 @@ private Class checkConditionType( String type ) throws JS } return typeClass; } - + /** * Check the condition key and value validity. - * + * * @param key Condition key. * @param values Condition values. * @param typeClass The condition type @@ -485,7 +509,7 @@ private void checkConditionKeyAndValues( keyObj.validateValueType( v ); } } - + /** * Check the validity of effect. */ @@ -497,39 +521,39 @@ private void checkEffect( String effect ) throws JSONException { throw new JSONException( "Invalid Effect value: " + effect ); } } - + private String normalize( String value ) { return ( value != null ) ? value.trim( ).toLowerCase( ) : null; } private Set variableSet( final PolicyParseContext context, - final List conditions - ) { + final List conditions + ) { return variableSet( context, conditions, Collections.emptySet( ) ); } private static boolean supportsPolicyVariables( final PolicyParseContext context ) { - return context.getVersion( ) != null && "2012-10-17".compareTo( context.getVersion( ) ) <= 0; + return context.getVersion( ) != null && "2012-10-17".compareTo( context.getVersion( ) ) <= 0; } - + private Set variableSet( final PolicyParseContext context, - final List conditions, - final Set resources + final List conditions, + final Set resources ) { if ( !supportsPolicyVariables( context ) ) { return Collections.emptySet( ); } - + final Set variables = Sets.newHashSet( ); - + for ( final Condition condition : conditions ) { for ( final String value : condition.getValues( ) ) { addVariablesFrom( variables, value ); } } - + for ( final String resource : resources ) { try { addVariablesFrom( variables, resource ); @@ -537,17 +561,17 @@ private Set variableSet( Logs.exhaust( ).error( e, e ); } } - + return variables; } - + private void addVariablesFrom( final Set variables, final String text ) { - final Matcher matcher = VARIABLE_MATCHER.matcher( text ); - while ( matcher.find( ) ) { + final Matcher matcher = VARIABLE_MATCHER.matcher( text ); + while ( matcher.find( ) ) { variables.add( matcher.group( ) ); } } - + private static PolicyResourceSetKey key( final String region, final String account, final String type ) { return new PolicyResourceSetKey( Strings.emptyToNull( region ), Strings.emptyToNull( account ), type ); } diff --git a/clc/modules/msgs/src/test/java/com/eucalyptus/auth/util/TestAuthenticationLimitProvider.java b/clc/modules/msgs/src/test/java/com/eucalyptus/auth/util/TestAuthenticationLimitProvider.java index ef69c9a3403..d1071ec704f 100644 --- a/clc/modules/msgs/src/test/java/com/eucalyptus/auth/util/TestAuthenticationLimitProvider.java +++ b/clc/modules/msgs/src/test/java/com/eucalyptus/auth/util/TestAuthenticationLimitProvider.java @@ -46,4 +46,9 @@ public int getSigningCertificateLimitSpi( ) { public int getPolicySizeLimitSpi( ) { return 16384; } + + @Override + public boolean getUseValidatingPolicyParserSpi( ) { + return true; + } }