amazon-s3-encryption-client-java icon indicating copy to clipboard operation
amazon-s3-encryption-client-java copied to clipboard

Inconsistency behavior of the EncryptionClient wrt the regular client about end of stream behavior

Open NathanEckert opened this issue 1 year ago • 4 comments

Problem:

When using ranged queries with the encryption client, I get a different behavior than with the regular S3 client. It is not clear there which one is incorrect:

  • the encryption client is consistent with java.io.InputStream#read(byte[], int, int) implementation and javadoc
  • the encryption client is not consistent with the behavior with the SDK v1
package com.test.aws;

import java.io.InputStream;
import java.nio.ByteBuffer;
import java.security.KeyFactory;
import java.security.KeyPair;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.spec.PKCS8EncodedKeySpec;
import java.security.spec.X509EncodedKeySpec;
import java.time.Duration;
import java.util.Base64;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
import software.amazon.awssdk.core.ResponseInputStream;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.http.apache.ApacheHttpClient;
import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.s3.S3AsyncClient;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.DeleteObjectRequest;
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.encryption.s3.S3EncryptionClient;

public class TestEndOfStreamBehavior {
	private static final Region DEFAULT_REGION = AwsTestUtil.DEFAULT_REGION;
	private static final String BUCKET = AwsTestUtil.AWS_TEST_BUCKET;
	private static final String KEY = "filename.txt";
	private static final byte[] CONTENT = "abcdefghijklmnopqrstuvwxyz0123456789".repeat(4).getBytes();

	/** The encryption key to use in client-side encryption tests. */
	protected static final KeyPair KEY_PAIR;


	static {
		final String publicKeyString = "yourPublicKey";
		final String privateKeyString = "yourPrivateKey";
		try {
			final KeyFactory factory = KeyFactory.getInstance("RSA");
			final PublicKey publicKey =
					factory.generatePublic(
							new X509EncodedKeySpec(Base64.getDecoder().decode(publicKeyString.getBytes())));
			final PrivateKey privateKey =
					factory.generatePrivate(
							new PKCS8EncodedKeySpec(Base64.getDecoder().decode(privateKeyString.getBytes())));
			KEY_PAIR = new KeyPair(publicKey, privateKey);
		} catch (Exception e) {
			throw new RuntimeException(e);
		}
	}

	@Test
	void testEndOfStreamBehavior() throws Exception {

		// Pick the client to use, inconsistent behavior between the two
		final S3Client client = getClient(DEFAULT_REGION);
		// final S3Client client = getEncryptionClient(KEY_PAIR, DEFAULT_REGION);

		// Delete the data if it exists
		final DeleteObjectRequest deleteRequest = DeleteObjectRequest.builder()
				.bucket(BUCKET)
				.key(KEY)
				.build();

		client.deleteObject(deleteRequest);

		// Upload the data
		final PutObjectRequest uploadRequest =
				PutObjectRequest.builder().bucket(BUCKET).key(KEY).build();
		client.putObject(uploadRequest, RequestBody.fromBytes(CONTENT));
		// wait for the data to be uploaded
		Thread.sleep(Duration.ofSeconds(5));

		// Actual test

		final GetObjectRequest downloadRequest =
				GetObjectRequest.builder()
						.bucket(BUCKET)
						.key(KEY)
						.range("bytes=0-15")
						.build();

		final InputStream stream = client.getObject(downloadRequest);

		// Buffer capacity matters !!!
		// Behavior difference when the capacity is same as the content length (i.e. 16) of the ranged query
		final ByteBuffer buffer = ByteBuffer.allocate(16);
		final byte[] underlyingBuffer = buffer.array();
		final int capacity = buffer.capacity();

		final int END_OF_STREAM = -1;
		int byteRead = 0;
		int startPosition = 0;
		while (byteRead != END_OF_STREAM) {
			int lenToRead = capacity - startPosition;
			System.out.println("Start position: " + startPosition + " Length to read: " + lenToRead);
                        // Difference of behavior here when reading the end of stream with a 0 lenToRead
			byteRead = stream.read(underlyingBuffer, startPosition, lenToRead);
			System.out.println("Read " + byteRead + " bytes");
			startPosition += byteRead;
			if (byteRead == 0) {
				System.out.println("Looping indefinitely with an encryption client, as startPosition is not increasing");
				break;
			}
		}
	}



	public static S3Client getEncryptionClient(final KeyPair keyPair, final Region region) {

		return S3EncryptionClient.builder()
				.rsaKeyPair(keyPair)
				.enableLegacyUnauthenticatedModes(true)
				.wrappedClient(getClient(region))
				.wrappedAsyncClient(getAsyncClient(region))
				.build();
	}


	public static S3Client getClient(final Region region) {

		return S3Client.builder()
				.region(region)
				.credentialsProvider(DefaultCredentialsProvider.create())
				.httpClientBuilder(
						ApacheHttpClient.builder().maxConnections(128) // Default is 50
				)
				.build();
	}


	public static S3AsyncClient getAsyncClient(final Region region) {

		final SdkAsyncHttpClient nettyHttpClient =
				NettyNioAsyncHttpClient.builder().maxConcurrency(100).build();

		return S3AsyncClient.builder()
				.region(region)
				.credentialsProvider(DefaultCredentialsProvider.create())
				.httpClient(nettyHttpClient)
				.build();
	}
}

I do not know which behavior is the correct one, but i wanted to report it as it is an inconsistency and a bug that can arise while migrating to the new SDK.

NathanEckert avatar Jun 19 '24 17:06 NathanEckert