From 62883b1e2153281f9eeb82a35cac773c24dd24c1 Mon Sep 17 00:00:00 2001 From: suresh Date: Fri, 12 Jun 2026 13:59:15 -0700 Subject: [PATCH 1/3] Switch from granting broad public access to CloudFront access to Objects in the content bucket. Co-authored-by: Claude --- cicd/3-app/javabuilder/template.yml.erb | 30 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 7ece8203..6e9b5c7a 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -487,16 +487,32 @@ Resources: Status: Enabled ExpirationInDays: 1 + ContentOriginAccessControl: + Type: AWS::CloudFront::OriginAccessControl + Properties: + OriginAccessControlConfig: + Name: !Sub "${SubdomainName}-${BaseDomainName}-content-oac" + OriginAccessControlOriginType: s3 + SigningBehavior: always + SigningProtocol: sigv4 + ContentBucketPolicy: Type: AWS::S3::BucketPolicy Properties: Bucket: !Ref ContentBucket PolicyDocument: + Version: '2012-10-17' Statement: - - Action: ['s3:GetObject'] - Effect: Allow - Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" - Principal: '*' + - Sid: AllowCloudFrontRead + Effect: Allow + Principal: + Service: cloudfront.amazonaws.com + Action: + - s3:GetObject + Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" + Condition: + StringEquals: + AWS:SourceArn: !Sub "arn:aws:cloudfront::${AWS::AccountId}:distribution/${ContentCDN}" ContentApiCertificate: Type: AWS::CertificateManager::Certificate @@ -537,8 +553,10 @@ Resources: # Prefix: !Sub "${SubdomainName}-content.${BaseDomainName}" Origins: - Id: ContentBucket - DomainName: !GetAtt ContentBucket.DomainName - S3OriginConfig: {} + DomainName: !GetAtt ContentBucket.RegionalDomainName + OriginAccessControlId: !GetAtt ContentOriginAccessControl.Id + S3OriginConfig: + OriginAccessIdentity: "" DefaultCacheBehavior: TargetOriginId: ContentBucket AllowedMethods: [DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT] From deb8a0e823af3c4f2ce5294af69a3e09c79de999 Mon Sep 17 00:00:00 2001 From: Molly Moen Date: Tue, 28 Jul 2026 12:52:42 -0700 Subject: [PATCH 2/3] block more --- .../JavabuilderSecurityPolicy.java | 23 ++++++++++++++----- .../JavabuilderSecurityPolicyTest.java | 17 +++++++++++++- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java b/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java index 55188cb3..b4b05c16 100644 --- a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java +++ b/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java @@ -7,6 +7,7 @@ import java.security.Permission; import java.security.Policy; import java.security.ProtectionDomain; +import java.security.SecurityPermission; import java.security.cert.Certificate; import java.util.List; @@ -24,6 +25,7 @@ * (so class loading, fonts, and bundled assets keep working), *
  • never allows execute *
  • denies all other filesystem access + *
  • denies replacing or removing the SecurityManager and this policy. * * *

    NOTE: {@link SecurityManager} / {@link Policy} are deprecated in Java 17 and removed in Java @@ -62,6 +64,14 @@ public boolean implies(ProtectionDomain domain, Permission permission) { if (permission instanceof FilePermission) { return isAllowedFileAccess((FilePermission) permission); } + + if (permission instanceof RuntimePermission + && "setSecurityManager".equals(permission.getName())) { + return false; + } + if (permission instanceof SecurityPermission && "setPolicy".equals(permission.getName())) { + return false; + } return true; } @@ -69,12 +79,11 @@ public boolean implies(ProtectionDomain domain, Permission permission) { * Forces every class that #implies references to load. Called from the constructor so it always * runs before a SecurityManager is installed. * - * #implies runs on every permission check, including the File.exists() calls - * the class loader makes to locate a class. If a class #implies references - * is still unloaded when the first such check fires, loading it re-enters - * #implies before the load completes and throws ClassCircularityError. Exercising - * the policy here, while no SecurityManager is active, guarantees those classes are already - * loaded by the time checks begin. + *

    #implies runs on every permission check, including the File.exists() calls the class loader + * makes to locate a class. If a class #implies references is still unloaded when the first such + * check fires, loading it re-enters #implies before the load completes and throws + * ClassCircularityError. Exercising the policy here, while no SecurityManager is active, + * guarantees those classes are already loaded by the time checks begin. */ private void warmUp() { final ProtectionDomain studentDomain = @@ -89,6 +98,8 @@ private void warmUp() { null); this.implies(studentDomain, new FilePermission("warmup", "read")); this.implies(studentDomain, new FilePermission("warmup", "write")); + this.implies(studentDomain, new RuntimePermission("setSecurityManager")); + this.implies(studentDomain, new SecurityPermission("setPolicy")); } private boolean isConfinedStudentCode(ProtectionDomain domain) { diff --git a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java index 825d8dd0..4e4f1fa2 100644 --- a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java +++ b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java @@ -6,6 +6,7 @@ import java.net.URL; import java.security.CodeSource; import java.security.ProtectionDomain; +import java.security.SecurityPermission; import java.security.cert.Certificate; import java.util.List; import java.util.PropertyPermission; @@ -76,6 +77,19 @@ public void studentCannotExecuteEvenUnderTmp() { assertFalse(policy.implies(validatorDomain, new FilePermission(tmpBinary, "execute"))); } + @Test + public void studentCannotDisableTheSandbox() { + // System.setSecurityManager(null) would remove the sandbox; Policy.setPolicy() would let + // student code replace this policy with an allow-everything one. + assertFalse(policy.implies(userDomain, new RuntimePermission("setSecurityManager"))); + assertFalse(policy.implies(userDomain, new SecurityPermission("setPolicy"))); + assertFalse(policy.implies(validatorDomain, new RuntimePermission("setSecurityManager"))); + assertFalse(policy.implies(validatorDomain, new SecurityPermission("setPolicy"))); + // Framework code (LambdaRequestHandler) installs the manager and policy itself. + assertTrue(policy.implies(frameworkDomain, new RuntimePermission("setSecurityManager"))); + assertTrue(policy.implies(frameworkDomain, new SecurityPermission("setPolicy"))); + } + @Test public void validatorRunIsAlsoConfined() { // The validation run loads and executes student code in a VALIDATOR-level UserClassLoader, so @@ -88,7 +102,8 @@ public void validatorRunIsAlsoConfined() { @Test public void constructorWarmUpDoesNotThrowAndPolicyStillEnforces() { // Construction warms the policy up; it must not throw and must not weaken enforcement. - final JavabuilderSecurityPolicy freshPolicy = assertDoesNotThrow(JavabuilderSecurityPolicy::new); + final JavabuilderSecurityPolicy freshPolicy = + assertDoesNotThrow(JavabuilderSecurityPolicy::new); assertFalse(freshPolicy.implies(userDomain, new FilePermission("/etc/passwd", "read"))); } From 7f440339b62070945b03c2d664051dbc711db35d Mon Sep 17 00:00:00 2001 From: Molly Moen Date: Tue, 28 Jul 2026 13:53:27 -0700 Subject: [PATCH 3/3] Revert "Merge remote-tracking branch 'origin/switch-content-bucket-to-cloudfront-origin-access-control' into molly/policy-update" This reverts commit fa29f721941ea353a475f3aaafed9de2f7604e7a, reversing changes made to deb8a0e823af3c4f2ce5294af69a3e09c79de999. --- cicd/3-app/javabuilder/template.yml.erb | 30 +++++-------------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 6e9b5c7a..7ece8203 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -487,32 +487,16 @@ Resources: Status: Enabled ExpirationInDays: 1 - ContentOriginAccessControl: - Type: AWS::CloudFront::OriginAccessControl - Properties: - OriginAccessControlConfig: - Name: !Sub "${SubdomainName}-${BaseDomainName}-content-oac" - OriginAccessControlOriginType: s3 - SigningBehavior: always - SigningProtocol: sigv4 - ContentBucketPolicy: Type: AWS::S3::BucketPolicy Properties: Bucket: !Ref ContentBucket PolicyDocument: - Version: '2012-10-17' Statement: - - Sid: AllowCloudFrontRead - Effect: Allow - Principal: - Service: cloudfront.amazonaws.com - Action: - - s3:GetObject - Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" - Condition: - StringEquals: - AWS:SourceArn: !Sub "arn:aws:cloudfront::${AWS::AccountId}:distribution/${ContentCDN}" + - Action: ['s3:GetObject'] + Effect: Allow + Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" + Principal: '*' ContentApiCertificate: Type: AWS::CertificateManager::Certificate @@ -553,10 +537,8 @@ Resources: # Prefix: !Sub "${SubdomainName}-content.${BaseDomainName}" Origins: - Id: ContentBucket - DomainName: !GetAtt ContentBucket.RegionalDomainName - OriginAccessControlId: !GetAtt ContentOriginAccessControl.Id - S3OriginConfig: - OriginAccessIdentity: "" + DomainName: !GetAtt ContentBucket.DomainName + S3OriginConfig: {} DefaultCacheBehavior: TargetOriginId: ContentBucket AllowedMethods: [DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT]