Security hardening: credential scoping, S3 encryption, scanner logging (#788)

Author: Schmarvinius

.github/workflows/pipeline.yml

@@ -2,20 +2,6 @@ name: Reusable Workflow
 
 env:
   MAVEN_VERSION: '3.9.12'
-  # Cloud storage environment variables (available to all jobs that need them)
-  ## AWS
-  AWS_S3_HOST: ${{ secrets.AWS_S3_HOST }}
-  AWS_S3_BUCKET: ${{ secrets.AWS_S3_BUCKET }}
-  AWS_S3_REGION: ${{ secrets.AWS_S3_REGION }}
-  AWS_S3_ACCESS_KEY_ID: ${{ secrets.AWS_S3_ACCESS_KEY_ID }}
-  AWS_S3_SECRET_ACCESS_KEY: ${{ secrets.AWS_S3_SECRET_ACCESS_KEY }}
-  ## Azure
-  AZURE_CONTAINER_URI: ${{ secrets.AZURE_CONTAINER_URI }}
-  AZURE_SAS_TOKEN: ${{ secrets.AZURE_SAS_TOKEN }}
-  ## GCP
-  GS_BASE_64_ENCODED_PRIVATE_KEY_DATA: ${{ secrets.GS_BASE_64_ENCODED_PRIVATE_KEY_DATA }}
-  GS_BUCKET: ${{ secrets.GS_BUCKET }}
-  GS_PROJECT_ID: ${{ secrets.GS_PROJECT_ID }}
 
 on:
   workflow_call:
@@ -65,6 +51,20 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 30
     needs: build
+    env:
+      ## AWS
+      AWS_S3_HOST: ${{ secrets.AWS_S3_HOST }}
+      AWS_S3_BUCKET: ${{ secrets.AWS_S3_BUCKET }}
+      AWS_S3_REGION: ${{ secrets.AWS_S3_REGION }}
+      AWS_S3_ACCESS_KEY_ID: ${{ secrets.AWS_S3_ACCESS_KEY_ID }}
+      AWS_S3_SECRET_ACCESS_KEY: ${{ secrets.AWS_S3_SECRET_ACCESS_KEY }}
+      ## Azure
+      AZURE_CONTAINER_URI: ${{ secrets.AZURE_CONTAINER_URI }}
+      AZURE_SAS_TOKEN: ${{ secrets.AZURE_SAS_TOKEN }}
+      ## GCP
+      GS_BASE_64_ENCODED_PRIVATE_KEY_DATA: ${{ secrets.GS_BASE_64_ENCODED_PRIVATE_KEY_DATA }}
+      GS_BUCKET: ${{ secrets.GS_BUCKET }}
+      GS_PROJECT_ID: ${{ secrets.GS_PROJECT_ID }}
     strategy:
       fail-fast: false
       matrix:

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java

@@ -94,7 +94,7 @@ private MalwareScanResultStatus findAndScanAttachments(
     return selectionResults.stream()
         .filter(result -> validateAndFilter(result, contentId))
         .findFirst()
-        .map(result -> scanDocument(result.result().single(Attachments.class)))
+        .map(result -> scanDocument(result.result().single(Attachments.class), result.entity))
         .orElse(null);
   }
 
@@ -158,7 +158,7 @@ private Result readData(String contentId, CdsEntity entity) {
     return result;
   }
 
-  private MalwareScanResultStatus scanDocument(Attachments attachment) {
+  private MalwareScanResultStatus scanDocument(Attachments attachment, CdsEntity attachmentEntity) {
     if (malwareScanClient != null) {
       try {
         InputStream content =
@@ -168,7 +168,11 @@ private MalwareScanResultStatus scanDocument(Attachments attachment) {
         logger.debug("Start scanning attachment {}.", attachment.getContentId());
         return malwareScanClient.scanContent(content);
       } catch (RuntimeException e) {
-        logger.error("Error while scanning attachment {}.", attachment.getContentId(), e);
+        logger.error(
+            "Error while scanning attachment {} in entity {}.",
+            attachment.getContentId(),
+            attachmentEntity.getQualifiedName(),
+            e);
         return MalwareScanResultStatus.FAILED;
       }
     }

storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java

@@ -32,6 +32,7 @@
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.model.S3Error;
+import software.amazon.awssdk.services.s3.model.ServerSideEncryption;
 
 public class AWSClient implements OSClient {
   private final S3Client s3Client;
@@ -89,6 +90,9 @@ public Future<Void> uploadContent(
             .bucket(this.bucketName)
             .key(completeFileName)
             .contentType(contentType)
+            // Azure and Google Cloud Storage encrypt at rest by default; S3 requires explicit
+            // opt-in
+            .serverSideEncryption(ServerSideEncryption.AES256)
             .build();
 
     CompletableFuture<PutObjectResponse> putFuture = this.s3AsyncClient.putObject(putRequest, body);

storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java

@@ -42,6 +42,7 @@
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.model.S3Object;
+import software.amazon.awssdk.services.s3.model.ServerSideEncryption;
 
 class AWSClientTest {
   ExecutorService executor = Executors.newCachedThreadPool();
@@ -73,19 +74,29 @@ void testReadContent() throws Exception {
   void testUploadContent() throws Exception {
     S3AsyncClient mockAsyncClient = mock(S3AsyncClient.class);
     AWSClient awsClient = new AWSClient(mock(S3Client.class), mockAsyncClient, "bucket", executor);
+    configureSuccessfulPut(mockAsyncClient);
 
-    PutObjectResponse mockPutRes = mock(PutObjectResponse.class);
-    SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class);
-    when(mockHttpRes.isSuccessful()).thenReturn(true);
-    when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes);
-    CompletableFuture<PutObjectResponse> successFuture =
-        CompletableFuture.completedFuture(mockPutRes);
-    when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class)))
-        .thenReturn(successFuture);
+    awsClient
+        .uploadContent(new ByteArrayInputStream("test".getBytes()), "test.txt", "text/plain")
+        .get();
+  }
+
+  @Test
+  void testUploadContentSetsServerSideEncryption() throws Exception {
+    S3AsyncClient mockAsyncClient = mock(S3AsyncClient.class);
+    AWSClient awsClient = new AWSClient(mock(S3Client.class), mockAsyncClient, "bucket", executor);
+    configureSuccessfulPut(mockAsyncClient);
 
     awsClient
         .uploadContent(new ByteArrayInputStream("test".getBytes()), "test.txt", "text/plain")
         .get();
+
+    verify(mockAsyncClient)
+        .putObject(
+            argThat(
+                (PutObjectRequest req) ->
+                    req.serverSideEncryption() == ServerSideEncryption.AES256),
+            any(AsyncRequestBody.class));
   }
 
   @Test
@@ -287,6 +298,17 @@ void testDeleteContentByPrefixWithPagination() throws Exception {
     verify(mockS3Client, times(2)).deleteObjects(any(DeleteObjectsRequest.class));
   }
 
+  private void configureSuccessfulPut(S3AsyncClient mockAsyncClient) {
+    PutObjectResponse mockPutRes = mock(PutObjectResponse.class);
+    SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class);
+    when(mockHttpRes.isSuccessful()).thenReturn(true);
+    when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes);
+    CompletableFuture<PutObjectResponse> successFuture =
+        CompletableFuture.completedFuture(mockPutRes);
+    when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class)))
+        .thenReturn(successFuture);
+  }
+
   private ServiceBinding getDummyBinding() {
     ServiceBinding binding = mock(ServiceBinding.class);
     HashMap<String, Object> creds = new HashMap<>();