Merge pull request #899 from AndreKurait/ReplayerSigv4Fix

Fix replayer sigv4 logic without payload
opensearch-project · Aug 20, 2024 · 949cd25 · 949cd25
2 parents 8f95f0a + 335fce8
commit 949cd25
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 16 deletions.
diff --git a/...player/src/main/java/org/opensearch/migrations/transform/SigV4AuthTransformerFactory.java b/...player/src/main/java/org/opensearch/migrations/transform/SigV4AuthTransformerFactory.java
@@ -72,7 +72,9 @@ public String protocol() {
 
                 @Override
                 public Optional<String> getFirstHeaderValueCaseInsensitive(String key) {
-                    return Optional.ofNullable(message.headers().getInsensitive(key).get(0));
+                    return Optional.ofNullable(message.headers().getInsensitive(key))
+                        .filter(l -> !l.isEmpty())
+                        .map(l -> l.get(0));
                 }
 
                 @Override

diff --git a/...test/java/org/opensearch/migrations/replay/transform/SigV4AuthTransformerFactoryTest.java b/...test/java/org/opensearch/migrations/replay/transform/SigV4AuthTransformerFactoryTest.java
@@ -3,19 +3,21 @@
 import java.time.Clock;
 import java.time.Instant;
 import java.time.ZoneOffset;
+import java.util.List;
 import java.util.Random;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import org.opensearch.migrations.replay.TestUtils;
 import org.opensearch.migrations.testutils.WrapWithNettyLeakDetection;
 import org.opensearch.migrations.tracing.InstrumentationTest;
 import org.opensearch.migrations.transform.SigV4AuthTransformerFactory;
 
 import io.netty.handler.codec.http.DefaultHttpHeaders;
-import io.netty.util.ResourceLeakDetector;
 import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
@@ -31,22 +33,32 @@ public AwsCredentials resolveCredentials() {
         }
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"content-type", "Content-Type", "CoNteNt-Type"})
+    @WrapWithNettyLeakDetection(repetitions = 2)
+    public void testSignatureProperlyApplied(String contentTypeHeaderKey) throws Exception {
+        Random r = new Random(2);
+        var mockCredentialsProvider = new MockCredentialsProvider();
+
+        // Test with payload
+        testWithPayload(r, mockCredentialsProvider, contentTypeHeaderKey);
+    }
+
     @Test
     @WrapWithNettyLeakDetection(repetitions = 2)
-    public void testSignatureProperlyApplied() throws Exception {
-        ResourceLeakDetector.setLevel(ResourceLeakDetector.Level.PARANOID);
+    public void testSignatureProperlyAppliedNoPayload() throws Exception {
+        var mockCredentialsProvider = new MockCredentialsProvider();
 
-        Random r = new Random(2);
+        // Test without payload
+        testWithoutPayload(mockCredentialsProvider);
+    }
+
+    private void testWithPayload(Random r, MockCredentialsProvider mockCredentialsProvider, String contentTypeHeaderKey) throws Exception {
         var stringParts = IntStream.range(0, 1)
             .mapToObj(i -> TestUtils.makeRandomString(r, 64))
             .collect(Collectors.toList());
 
-        var mockCredentialsProvider = new MockCredentialsProvider();
         DefaultHttpHeaders expectedRequestHeaders = new DefaultHttpHeaders();
-        // netty's decompressor and aggregator remove some header values (& add others)
-
-        // Using unusual spelling to verify SigV4 signer behavior with list insensitive map
-        var contentTypeHeaderKey = "CoNteNt-Type";
         var contentTypeHeaderValue = "application/json";
 
         expectedRequestHeaders.add("Host", "localhost");
@@ -62,9 +74,26 @@ public void testSignatureProperlyApplied() throws Exception {
             "x-amz-content-sha256",
             "fc0e8e9a1f7697f510bfdd4d55b8612df8a0140b4210967efd87ee9cb7104362"
         );
+        expectedRequestHeaders.add("X-Amz-Date", "19700101T000000Z");
+        runTest(mockCredentialsProvider, contentTypeHeaderKey, contentTypeHeaderValue, expectedRequestHeaders, stringParts);
+    }
 
-        expectedRequestHeaders.add("x-amz-date", "19700101T000000Z");
+    private void testWithoutPayload(MockCredentialsProvider mockCredentialsProvider) throws Exception {
+        DefaultHttpHeaders expectedRequestHeaders = new DefaultHttpHeaders();
 
+        expectedRequestHeaders.add("Host", "localhost");
+        expectedRequestHeaders.add("Content-Length", "0");
+        expectedRequestHeaders.add("X-Amz-Date", "19700101T000000Z");
+        expectedRequestHeaders.add(
+            "Authorization",
+            "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/19700101/us-east-1/es/aws4_request, "
+                + "SignedHeaders=" + "host;x-amz-date, "
+                + "Signature=0c80b2640a1de42eb5cf957c6bc080731d8f83ccc1d4ebe40e72cd847ed4648e"
+        );
+        runTest(mockCredentialsProvider, null, null, expectedRequestHeaders, List.of());
+    }
+
+    private void runTest(MockCredentialsProvider mockCredentialsProvider, String contentTypeHeaderKey, String contentTypeHeaderValue, DefaultHttpHeaders expectedRequestHeaders, java.util.List<String> stringParts) throws Exception {
         try (var factory = new SigV4AuthTransformerFactory(
             mockCredentialsProvider,
             "es",
@@ -75,12 +104,11 @@ public void testSignatureProperlyApplied() throws Exception {
             TestUtils.runPipelineAndValidate(
                 rootContext,
                 factory,
-                contentTypeHeaderKey + ": "+ contentTypeHeaderValue + "\r\n",
+                (contentTypeHeaderKey != null) ? contentTypeHeaderKey + ": " + contentTypeHeaderValue + "\r\n" : null,
                 stringParts,
                 expectedRequestHeaders,
                 TestUtils::resolveReferenceString
             );
         }
-
     }
 }
diff --git a/...ure/trafficReplayer/src/testFixtures/java/org/opensearch/migrations/replay/TestUtils.java b/...ure/trafficReplayer/src/testFixtures/java/org/opensearch/migrations/replay/TestUtils.java
@@ -13,6 +13,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
 import java.util.function.IntFunction;
+import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.Assertions;
 
@@ -132,6 +133,9 @@ protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) thro
 
         Assertions.assertEquals((expectedPayloadString), getStringFromContent(fullRequest));
         Assertions.assertEquals(expectedRequestHeaders, fullRequest.headers());
+        // Test casing on keys match
+        Assertions.assertEquals(expectedRequestHeaders.names().stream().sorted().collect(Collectors.toList()),
+            fullRequest.headers().names().stream().sorted().collect(Collectors.toList()));
         fullRequest.release();
     }
 

diff --git a/...nsformerProvider/src/test/java/org/opensearch/migrations/replay/PayloadRepackingTest.java b/...nsformerProvider/src/test/java/org/opensearch/migrations/replay/PayloadRepackingTest.java
@@ -62,8 +62,13 @@ public void testSimplePayloadTransform(boolean doGzip, boolean doChunked) throws
 
         DefaultHttpHeaders expectedRequestHeaders = new DefaultHttpHeaders();
         // netty's decompressor and aggregator remove some header values (& add others)
-        expectedRequestHeaders.add("host", "localhost");
-        expectedRequestHeaders.add("Content-Length", "46");
+        expectedRequestHeaders.add("Host", "localhost");
+        if (!doGzip && !doChunked) {
+            expectedRequestHeaders.add("Content-Length", "46");
+        } else {
+            // Content-Length added with different casing with netty
+            expectedRequestHeaders.add("content-length", "46");
+        }
 
         TestUtils.runPipelineAndValidate(
             rootContext,
@@ -115,7 +120,7 @@ public void testJsonPayloadTransformation() throws Exception {
 
         DefaultHttpHeaders expectedRequestHeaders = new DefaultHttpHeaders();
         // netty's decompressor and aggregator remove some header values (& add others)
-        expectedRequestHeaders.add("host", "localhost");
+        expectedRequestHeaders.add("Host", "localhost");
         expectedRequestHeaders.add("content-type", "application/json; charset=UTF-8");
         expectedRequestHeaders.add("Content-Length", "55");