diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java index 361806545403b..ff77c7848ff5a 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java @@ -785,6 +785,13 @@ private Constants() { public static final String S3_ENCRYPTION_CONTEXT = "fs.s3a.encryption.context"; + /** + * Default S3-SSE encryption context. + * value:{@value} + */ + public static final String DEFAULT_S3_ENCRYPTION_CONTEXT = + ""; + /** * Client side encryption (CSE-CUSTOM) with custom cryptographic material manager class name. * Custom keyring class name for CSE-KMS. diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/EncryptionSecrets.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/EncryptionSecrets.java index f421ecca24cf9..c89233fff812b 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/EncryptionSecrets.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/EncryptionSecrets.java @@ -25,12 +25,18 @@ import java.io.Serializable; import java.util.Objects; +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.fs.s3a.S3AEncryptionMethods; import org.apache.hadoop.io.LongWritable; import org.apache.hadoop.io.Text; import org.apache.hadoop.io.Writable; +import static org.apache.hadoop.fs.s3a.Constants.DEFAULT_S3_ENCRYPTION_CONTEXT; + /** * Encryption options in a form which can serialized or marshalled as a hadoop * Writeable. @@ -52,9 +58,24 @@ */ public class EncryptionSecrets implements Writable, Serializable { + private static final Logger LOG = + LoggerFactory.getLogger(EncryptionSecrets.class); + public static final int MAX_SECRET_LENGTH = 2048; - private static final long serialVersionUID = 1208329045511296375L; + /** + * Change this after any change to the payload: {@value}. + */ + private static final long serialVersionUID = 8834417969966697162L; + + @VisibleForTesting + public static final long SERIAL_VERSION_UID_CURRENT = serialVersionUID; + + /** + * Serial version ID prior to {@link #encryptionContext} field being added: {@value}. + */ + @VisibleForTesting + public static final long SERIAL_VERSION_UID_1 = 1208329045511296375L; /** * Encryption algorithm to use: must match one in @@ -70,7 +91,7 @@ public class EncryptionSecrets implements Writable, Serializable { /** * Encryption context: base64-encoded UTF-8 string. */ - private String encryptionContext = ""; + private String encryptionContext = DEFAULT_S3_ENCRYPTION_CONTEXT; /** * This field isn't serialized/marshalled; it is rebuilt from the @@ -86,7 +107,24 @@ public EncryptionSecrets() { } /** - * Create a pair of secrets. + * Create a tuple of secrets. The encryption context is set to "". + * This constructor is used in external implementations of S3A delegation + * tokens, sp MUST be retained even if there is no use in our own + * production code. + * @param encryptionAlgorithm algorithm enumeration. + * @param encryptionKey key/key reference. + * @throws IOException failure to initialize. + * @deprecated use {@link #EncryptionSecrets(S3AEncryptionMethods, String, String)} + * which takes an encryption context. + */ + public EncryptionSecrets(final S3AEncryptionMethods encryptionAlgorithm, + final String encryptionKey) throws IOException { + this(encryptionAlgorithm.getMethod(), encryptionKey, + DEFAULT_S3_ENCRYPTION_CONTEXT); + } + + /** + * Create a 3/tuple of secrets. * @param encryptionAlgorithm algorithm enumeration. * @param encryptionKey key/key reference. * @param encryptionContext base64-encoded string with the encryption context key-value pairs. @@ -99,7 +137,7 @@ public EncryptionSecrets(final S3AEncryptionMethods encryptionAlgorithm, } /** - * Create a pair of secrets. + * Create a 3/tuple of secrets. * @param encryptionAlgorithm algorithm name * @param encryptionKey key/key reference. * @param encryptionContext base64-encoded string with the encryption context key-value pairs. @@ -137,13 +175,25 @@ public void write(final DataOutput out) throws IOException { public void readFields(final DataInput in) throws IOException { final LongWritable version = new LongWritable(); version.readFields(in); - if (version.get() != serialVersionUID) { + boolean readContext; + + final long versionId = version.get(); + if (versionId == SERIAL_VERSION_UID_1) { + LOG.info("Unmarshalling Encryption Secrets from older client; setting encryption context to \"\""); + readContext = false; + } else if (versionId == serialVersionUID) { + readContext = true; + } else { throw new DelegationTokenIOException( - "Incompatible EncryptionSecrets version"); + "Incompatible EncryptionSecrets version: " + versionId); } encryptionAlgorithm = Text.readString(in, MAX_SECRET_LENGTH); encryptionKey = Text.readString(in, MAX_SECRET_LENGTH); - encryptionContext = Text.readString(in); + if (readContext) { + encryptionContext = Text.readString(in); + } else { + encryptionContext = DEFAULT_S3_ENCRYPTION_CONTEXT; + } init(); } diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AEncryption.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AEncryption.java index a720d2ca10000..d252d4f595e93 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AEncryption.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AEncryption.java @@ -31,6 +31,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.s3a.S3AUtils; +import static org.apache.hadoop.fs.s3a.Constants.DEFAULT_S3_ENCRYPTION_CONTEXT; import static org.apache.hadoop.fs.s3a.Constants.S3_ENCRYPTION_CONTEXT; /** @@ -61,7 +62,7 @@ public static String getS3EncryptionContext(String bucket, Configuration conf) } if (encryptionContext == null) { // no encryption context, return "" - return ""; + return DEFAULT_S3_ENCRYPTION_CONTEXT; } return encryptionContext; } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/TestMarshalledCredentials.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/TestMarshalledCredentials.java index 3c18599a9a2c4..1d5f2f2a979ea 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/TestMarshalledCredentials.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/TestMarshalledCredentials.java @@ -18,9 +18,13 @@ package org.apache.hadoop.fs.s3a.auth; +import java.io.EOFException; +import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.Optional; +import org.assertj.core.api.Assertions; import software.amazon.awssdk.auth.credentials.AwsCredentials; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -28,9 +32,15 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.s3a.S3AEncryptionMethods; import org.apache.hadoop.fs.s3a.S3ATestUtils; +import org.apache.hadoop.fs.s3a.auth.delegation.DelegationTokenIOException; import org.apache.hadoop.fs.s3a.auth.delegation.EncryptionSecrets; +import org.apache.hadoop.io.DataInputBuffer; +import org.apache.hadoop.io.DataOutputBuffer; +import org.apache.hadoop.io.LongWritable; +import org.apache.hadoop.io.Text; import org.apache.hadoop.test.HadoopTestBase; +import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.SSE_S3; import static org.apache.hadoop.test.LambdaTestUtils.intercept; /** @@ -78,13 +88,33 @@ public void testRoundTripNoSessionData() throws Throwable { @Test public void testRoundTripEncryptionData() throws Throwable { + final String context = "encryptionContext"; EncryptionSecrets secrets = new EncryptionSecrets( S3AEncryptionMethods.SSE_KMS, - "key", - "encryptionContext"); + KEY, + context); EncryptionSecrets result = S3ATestUtils.roundTrip(secrets, new Configuration()); assertEquals(secrets, result, "round trip"); + Assertions.assertThat(result .getEncryptionContext()) + .describedAs("encryptionContext") + .isEqualTo(context); + } + + @Test + public void testRoundTripEncryptionSecretsNoContext() throws Throwable { + EncryptionSecrets secrets = new EncryptionSecrets( + S3AEncryptionMethods.SSE_KMS, + KEY); + EncryptionSecrets result = S3ATestUtils.roundTrip(secrets, + new Configuration()); + assertEquals(secrets, result, "round trip"); + // not equal to secrets with a context + Assertions.assertThat(result) + .isNotEqualTo(new EncryptionSecrets( + S3AEncryptionMethods.SSE_KMS, + KEY, + "encryptionContext")); } @Test @@ -134,4 +164,110 @@ public void testCredentialProviderNullURI() throws Throwable { credentials, MarshalledCredentials.CredentialTypeRequired.FullOnly)); } + + @org.junit.Test + public void testUnmarshallOldEncryptionSecrets() throws Throwable { + + } + + /** + * Generate the equivalent to a marshalled EncryptionSecrets value. + * @param id serialization ID. + * @param encryptionAlgorithm algorithm. + * @param encryptionKey key + * @param encryptionContext optional context + * @return the input + * @throws IOException write failure. + */ + private DataInputBuffer writeEncryptionSecrets(long id, + final String encryptionAlgorithm, + final String encryptionKey, + final Optional encryptionContext) throws IOException { + DataOutputBuffer out = new DataOutputBuffer(); + new LongWritable(id).write(out); + Text.writeString(out, encryptionAlgorithm); + Text.writeString(out, encryptionKey); + if (encryptionContext.isPresent()) { + Text.writeString(out, encryptionContext.get()); + } + + DataInputBuffer dib = new DataInputBuffer(); + dib.reset(out.getData(), out.getLength()); + return dib; + } + + private EncryptionSecrets readEncryptionSecrets(DataInputBuffer dib) throws IOException { + final EncryptionSecrets secrets = new EncryptionSecrets(); + secrets.readFields(dib); + return secrets; + } + + private static final String ENCRYPTION_ALGORITHM = SSE_S3.getMethod(); + + private static final String KEY = "key"; + + private static final String CONTEXT = "context"; + + /** + * Verify that the low level marshalling code works. + */ + @Test + public void testMarshallCurrentSecrets() throws Throwable { + EncryptionSecrets src = new EncryptionSecrets(ENCRYPTION_ALGORITHM, + KEY, + CONTEXT); + final DataInputBuffer in = + writeEncryptionSecrets(EncryptionSecrets.SERIAL_VERSION_UID_CURRENT, + ENCRYPTION_ALGORITHM, KEY, Optional.of(CONTEXT)); + final EncryptionSecrets read = readEncryptionSecrets(in); + Assertions.assertThat(read) + .isEqualTo(src); + } + + /** + * Generate the layout of an old secret entry, unmarshall it to the new one. + */ + @Test + public void testUnmarshallOldSecrets() throws Throwable { + final DataInputBuffer dib = writeEncryptionSecrets(EncryptionSecrets.SERIAL_VERSION_UID_1, + ENCRYPTION_ALGORITHM, KEY, Optional.empty()); + final EncryptionSecrets read = readEncryptionSecrets(dib); + + // all the data has been read in + Assertions.assertThat(dib.read()) + .describedAs("Input stream read() at end of unmarshalling") + .isEqualTo(-1); + Assertions.assertThat(read) + .matches(s -> !s.hasEncryptionContext()) + .hasFieldOrPropertyWithValue("encryptionAlgorithm", ENCRYPTION_ALGORITHM) + .hasFieldOrPropertyWithValue("getEncryptionKey", KEY); + } + + /** + * Generate the layout of an old secret entry, unmarshall it to the new one. + */ + @Test + public void testCurrentSecretsRequireContext() throws Throwable { + final DataInputBuffer in = writeEncryptionSecrets( + EncryptionSecrets.SERIAL_VERSION_UID_CURRENT, + ENCRYPTION_ALGORITHM, KEY, Optional.empty()); + intercept(EOFException.class, "", () -> + readEncryptionSecrets(in)); + } + + /** + * Usea unknown version ID; expect an exception with the version ID in the message. + */ + @Test + public void testUnmarshallUnknownSecretVersion() throws Throwable { + EncryptionSecrets src = new EncryptionSecrets(ENCRYPTION_ALGORITHM, KEY, CONTEXT); + final DataInputBuffer in = + writeEncryptionSecrets(12345L, + src.getEncryptionAlgorithm(), src.getEncryptionKey(), + Optional.of("context1")); + intercept(DelegationTokenIOException.class, "12345", () -> { + readEncryptionSecrets(in); + }); + } + } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/TestRequestFactory.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/TestRequestFactory.java index 36a0b8102b6a2..b44daab7b825d 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/TestRequestFactory.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/TestRequestFactory.java @@ -56,6 +56,7 @@ import org.apache.hadoop.test.AbstractHadoopTestBase; import static org.apache.hadoop.fs.s3a.Constants.DEFAULT_PART_UPLOAD_TIMEOUT; +import static org.apache.hadoop.fs.s3a.Constants.DEFAULT_S3_ENCRYPTION_CONTEXT; import static org.apache.hadoop.fs.s3a.impl.PutObjectOptions.defaultOptions; import static org.apache.hadoop.test.LambdaTestUtils.intercept; import static org.assertj.core.api.Assertions.assertThat; @@ -89,7 +90,7 @@ public void testRequestFactoryWithEncryption() throws Throwable { .withBucket("bucket") .withEncryptionSecrets( new EncryptionSecrets(S3AEncryptionMethods.SSE_KMS, - "kms:key", "")) + "kms:key", DEFAULT_S3_ENCRYPTION_CONTEXT)) .build(); createFactoryObjects(factory); } @@ -329,7 +330,7 @@ public void testCompleteMultipartUploadRequestWithChecksumAlgorithmAndSSEC() thr .encodeToString(encryptionKey); final String encryptionKeyMd5 = Md5Utils.md5AsBase64(encryptionKey); final EncryptionSecrets encryptionSecrets = new EncryptionSecrets(S3AEncryptionMethods.SSE_C, - encryptionKeyBase64, null); + encryptionKeyBase64); RequestFactory factory = RequestFactoryImpl.builder() .withBucket("bucket") .withChecksumAlgorithm(ChecksumAlgorithm.CRC32_C)