Enhance S3 Web Identity authentication & security mapping #24645

imsayari404 · 2025-02-27T19:31:55Z

Description

This PR enhances S3 Web Identity authentication by updating Presto’s security mapping to support Web Identity credentials. It introduces new configurations and ensures compatibility with existing S3 authentication mechanisms.

Changes

Updated createAwsCredentialsProvider to support Web Identity authentication.
Modified PrestoS3ConfigurationUpdater, S3ConfigurationUpdater, and PrestoS3FileSystem.
Introduced new config properties:
- hive.s3.web-identity-token-file
- hive.s3.webidentity.enabled
Updated TestHiveS3Config to validate Web Identity authentication.

Motivation and Context

Enables Web Identity-based authentication for S3 without requiring static credentials.
Improves security by leveraging IAM roles for authentication.

Impact

Ensured backward compatibility with existing authentication methods.

Test Plan

Unit tests updated to validate Web Identity auth flow.

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Hive Changes
* Added support for Web Identity authentication in S3 security mapping.
* Introduced `hive.s3.web-identity-token-file` and `hive.s3.webidentity.enabled` for Web Identity authentication.

imjalpreet

@imsayari404 thanks for the enhancement. It mostly looks good. I've added a few minor suggestions for improvement.

imjalpreet · 2025-02-28T06:12:48Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/HiveS3Config.java

+        return s3WebIdentityEnabled;
+    }
+
+    @Config("hive.s3.webidentity.enabled")


nit:

Suggested change

@Config("hive.s3.webidentity.enabled")

@Config("hive.s3.web.identity.auth.enabled")

Open to suggestions here.

imjalpreet · 2025-02-28T06:23:04Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3ConfigurationUpdater.java

+        if (webIdentityEnabled) {
+            config.setBoolean(S3_WEB_IDENTITY_ENABLED, true);


nit: We can always set the config irrespective of true/false and set the token file only if webIdentityEnabled is true.

Suggested change

if (webIdentityEnabled) {

config.setBoolean(S3_WEB_IDENTITY_ENABLED, true);

config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled);

if (webIdentityEnabled) {

imjalpreet · 2025-02-28T06:28:04Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

+        boolean webIdentityEnabled = conf.getBoolean("presto.hive.s3.webidentity.enabled", false);
+        String webIdentityTokenFile = conf.get("presto.hive.s3.web-identity-token-file");


nit: let's make these two class variables and move their initialization to the initialize method, where other configs are retrieved.

presto/presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

Lines 201 to 252 in f6c9750

public void initialize(URI uri, Configuration conf)

throws IOException

{

requireNonNull(uri, "uri is null");

requireNonNull(conf, "conf is null");

super.initialize(uri, conf);

setConf(conf);

this.uri = URI.create(uri.getScheme() + "://" + uri.getAuthority());

this.workingDirectory = new Path(PATH_SEPARATOR).makeQualified(this.uri, new Path(PATH_SEPARATOR));

HiveS3Config defaults = new HiveS3Config();

this.stagingDirectory = new File(conf.get(S3_STAGING_DIRECTORY, defaults.getS3StagingDirectory().toString()));

this.maxAttempts = conf.getInt(S3_MAX_CLIENT_RETRIES, defaults.getS3MaxClientRetries()) + 1;

this.maxBackoffTime = Duration.valueOf(conf.get(S3_MAX_BACKOFF_TIME, defaults.getS3MaxBackoffTime().toString()));

this.maxRetryTime = Duration.valueOf(conf.get(S3_MAX_RETRY_TIME, defaults.getS3MaxRetryTime().toString()));

int maxErrorRetries = conf.getInt(S3_MAX_ERROR_RETRIES, defaults.getS3MaxErrorRetries());

boolean sslEnabled = conf.getBoolean(S3_SSL_ENABLED, defaults.isS3SslEnabled());

Duration connectTimeout = Duration.valueOf(conf.get(S3_CONNECT_TIMEOUT, defaults.getS3ConnectTimeout().toString()));

Duration socketTimeout = Duration.valueOf(conf.get(S3_SOCKET_TIMEOUT, defaults.getS3SocketTimeout().toString()));

int maxConnections = conf.getInt(S3_MAX_CONNECTIONS, defaults.getS3MaxConnections());

this.multiPartUploadMinFileSize = conf.getLong(S3_MULTIPART_MIN_FILE_SIZE, defaults.getS3MultipartMinFileSize().toBytes());

this.multiPartUploadMinPartSize = conf.getLong(S3_MULTIPART_MIN_PART_SIZE, defaults.getS3MultipartMinPartSize().toBytes());

this.isPathStyleAccess = conf.getBoolean(S3_PATH_STYLE_ACCESS, defaults.isS3PathStyleAccess());

this.useInstanceCredentials = conf.getBoolean(S3_USE_INSTANCE_CREDENTIALS, defaults.isS3UseInstanceCredentials());

this.pinS3ClientToCurrentRegion = conf.getBoolean(S3_PIN_CLIENT_TO_CURRENT_REGION, defaults.isPinS3ClientToCurrentRegion());

this.s3IamRole = conf.get(S3_IAM_ROLE, defaults.getS3IamRole());

this.s3IamRoleSessionName = conf.get(S3_IAM_ROLE_SESSION_NAME, defaults.getS3IamRoleSessionName());

verify(!(useInstanceCredentials && conf.get(S3_IAM_ROLE) != null),

"Invalid configuration: either use instance credentials or specify an iam role");

verify((pinS3ClientToCurrentRegion && conf.get(S3_ENDPOINT) == null) || !pinS3ClientToCurrentRegion,

"Invalid configuration: either endpoint can be set or S3 client can be pinned to the current region");

this.sseEnabled = conf.getBoolean(S3_SSE_ENABLED, defaults.isS3SseEnabled());

this.sseType = PrestoS3SseType.valueOf(conf.get(S3_SSE_TYPE, defaults.getS3SseType().name()));

this.sseKmsKeyId = conf.get(S3_SSE_KMS_KEY_ID, defaults.getS3SseKmsKeyId());

this.s3AclType = PrestoS3AclType.valueOf(conf.get(S3_ACL_TYPE, defaults.getS3AclType().name()));

String userAgentPrefix = conf.get(S3_USER_AGENT_PREFIX, defaults.getS3UserAgentPrefix());

this.skipGlacierObjects = conf.getBoolean(S3_SKIP_GLACIER_OBJECTS, defaults.isSkipGlacierObjects());

this.s3StorageClass = conf.getEnum(S3_STORAGE_CLASS, defaults.getS3StorageClass());

ClientConfiguration configuration = new ClientConfiguration()

.withMaxErrorRetry(maxErrorRetries)

.withProtocol(sslEnabled ? Protocol.HTTPS : Protocol.HTTP)

.withConnectionTimeout(toIntExact(connectTimeout.toMillis()))

.withSocketTimeout(toIntExact(socketTimeout.toMillis()))

.withMaxConnections(maxConnections)

.withUserAgentPrefix(userAgentPrefix)

.withUserAgentSuffix(S3_USER_AGENT_SUFFIX);

this.credentialsProvider = createAwsCredentialsProvider(uri, conf);

this.s3 = createAmazonS3Client(conf, configuration);

}

imjalpreet · 2025-02-28T06:30:46Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

+
+        if (!isNullOrEmpty(s3IamRole)) {
+            if (webIdentityEnabled) {
+                log.info("Using Web Identity Token Credentials Provider.");


nit: We should minimize info logs in production and use them only when absolutely necessary. If logging is required in this case, we can add a debug log.

imjalpreet · 2025-02-28T06:30:58Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

+                }
+                return providerBuilder.build();
+            }
+            log.info("Using STS Assume Role Session Credentials Provider.");


nit: same here

imjalpreet · 2025-02-28T06:34:34Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

+            if (webIdentityEnabled) {
+                log.info("Using Web Identity Token Credentials Provider.");
+                WebIdentityTokenCredentialsProvider.Builder providerBuilder = WebIdentityTokenCredentialsProvider.builder()
+                        .roleArn(s3IamRole);


nit: please set roleSessionName here along with roleArn.

imjalpreet · 2025-02-28T06:36:07Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/S3ConfigurationUpdater.java

@@ -28,6 +28,8 @@ public interface S3ConfigurationUpdater
    String S3_PIN_CLIENT_TO_CURRENT_REGION = "presto.s3.pin-client-to-current-region";
    String S3_USE_INSTANCE_CREDENTIALS = "presto.s3.use-instance-credentials";
    String S3_IAM_ROLE = "presto.hive.s3.iam-role";
+    String S3_WEB_IDENTITY_TOKEN_FILE = "presto.hive.s3.web-identity-token-file";
+    String S3_WEB_IDENTITY_ENABLED = "presto.hive.s3.webidentity.enabled";


nit: we can align this with config name change above.

imjalpreet

Forgot to mention, let's also add a test in TestPrestoS3FileSystem.

For reference:

presto/presto-hive/src/test/java/com/facebook/presto/hive/s3/TestPrestoS3FileSystem.java

Lines 151 to 162 in f6c9750

    
           public void testAssumeRoleCredentials() 
        
                   throws Exception 
        
           { 
        
               Configuration config = new Configuration(); 
        
               config.set(S3_IAM_ROLE, "role"); 
        
               config.setBoolean(S3_USE_INSTANCE_CREDENTIALS, false); 
        
               try (PrestoS3FileSystem fs = new PrestoS3FileSystem()) { 
        
                   fs.initialize(new URI("s3n://test-bucket/"), config); 
        
                   assertInstanceOf(getAwsCredentialsProvider(fs), STSAssumeRoleSessionCredentialsProvider.class); 
        
               } 
        
           }

imsayari404 · 2025-02-28T08:14:05Z

@imjalpreet , I’ve made the suggested changes. Could you review again when you have time? Thanks!

imjalpreet · 2025-02-28T08:59:08Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/TestPrestoS3FileSystem.java

+        config.setBoolean("presto.hive.s3.web.identity.auth.enabled", true);
+        config.set("presto.hive.s3.web-identity-token-file", "/path/to/token");


nit: let's use static variables defined in S3ConfigurationUpdater for the config names here

agrawalreetika · 2025-03-02T09:51:07Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3ConfigurationUpdater.java

+        config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled);
+        if (webIdentityEnabled) {
+            if (webIdentityTokenFile != null) {
+                config.set(S3_WEB_IDENTITY_TOKEN_FILE, webIdentityTokenFile);
+            }
+        }


Are these 2 are dependent config? Then we can set both after the condition check?

Suggested change

config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled);

if (webIdentityEnabled) {

if (webIdentityTokenFile != null) {

config.set(S3_WEB_IDENTITY_TOKEN_FILE, webIdentityTokenFile);

}

}

if (webIdentityEnabled && webIdentityTokenFile != null) {

config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled);

config.set(S3_WEB_IDENTITY_TOKEN_FILE, webIdentityTokenFile);

}

These two configurations are related but not strictly dependent. S3_WEB_IDENTITY_ENABLED determines if web identity authentication is used, but the token file is optional. Keeping them separate ensures flexibility, as web identity mode can be enabled without necessarily requiring a token file.

Ok in that case we can modify it to something like -

if (webIdentityEnabled && webIdentityTokenFile != null) { config.set(S3_WEB_IDENTITY_TOKEN_FILE, webIdentityTokenFile); }

agrawalreetika · 2025-03-02T09:56:49Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3FileSystem.java

+                if (!isNullOrEmpty(webIdentityTokenFile)) {
+                    providerBuilder.webIdentityTokenFile(webIdentityTokenFile);
+                }
+                return providerBuilder.build();


Can s3 identity auth work w/o token file as well?

Yes, web identity auth can work without explicitly setting the token file in the config. If the environment variable AWS_WEB_IDENTITY_TOKEN_FILE is set, AWS SDK will pick it up automatically, and authentication will still function.

agrawalreetika · 2025-03-03T07:18:43Z

presto-hive/src/main/java/com/facebook/presto/hive/s3/PrestoS3ConfigurationUpdater.java

@@ -124,6 +128,10 @@ public void updateConfiguration(Configuration config)
        if (sseKmsKeyId != null) {
            config.set(S3_SSE_KMS_KEY_ID, sseKmsKeyId);
        }
+        if (webIdentityEnabled && webIdentityTokenFile != null) {
+            config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled);


@imsayari404 You had mentioned, We can still use webidentity auth if flag is true but token file is not given right?

If that's the case then you should set config.setBoolean(S3_WEB_IDENTITY_ENABLED, webIdentityEnabled); outside condition itself? Please verify

You're right! Since web identity auth can work without explicitly setting the token file, S3_WEB_IDENTITY_ENABLED should be set outside the condition. My bad, I'll update it accordingly.

- Updated `createAwsCredentialsProvider` to support Web Identity. - Modified `PrestoS3ConfigurationUpdater`, `S3ConfigurationUpdater`, and `PrestoS3FileSystem` for Web Identity auth. - Web Identity configs added `hive.s3.web-identity-token-file` and `hive.s3.webidentity.enabled` - Updated `TestHiveS3Config` to validate changes.

prestodb-ci added the from:IBM PR from IBM label Feb 27, 2025

imsayari404 requested review from imjalpreet and agrawalreetika February 27, 2025 19:32

imjalpreet requested changes Feb 28, 2025

View reviewed changes

imjalpreet reviewed Feb 28, 2025

View reviewed changes

imsayari404 force-pushed the web_identity_feature branch from 3174f42 to 09804ab Compare February 28, 2025 08:12

imsayari404 force-pushed the web_identity_feature branch from 09804ab to 6a8273c Compare February 28, 2025 08:26

imjalpreet reviewed Feb 28, 2025

View reviewed changes

imsayari404 force-pushed the web_identity_feature branch from 6a8273c to 7004856 Compare February 28, 2025 09:37

imsayari404 requested a review from imjalpreet February 28, 2025 10:13

imsayari404 force-pushed the web_identity_feature branch from 7004856 to 095f6bd Compare February 28, 2025 11:03

agrawalreetika reviewed Mar 2, 2025

View reviewed changes

imsayari404 force-pushed the web_identity_feature branch from 095f6bd to 652de66 Compare March 3, 2025 05:30

imsayari404 requested a review from agrawalreetika March 3, 2025 05:30

agrawalreetika reviewed Mar 3, 2025

View reviewed changes

imsayari404 force-pushed the web_identity_feature branch from 652de66 to 9e4c43b Compare March 3, 2025 07:30

imsayari404 requested a review from agrawalreetika March 3, 2025 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance S3 Web Identity authentication & security mapping #24645

Enhance S3 Web Identity authentication & security mapping #24645

imsayari404 commented Feb 27, 2025 •

edited

Loading

imjalpreet left a comment

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet Feb 28, 2025

imjalpreet left a comment

imsayari404 commented Feb 28, 2025 •

edited

Loading

imjalpreet Feb 28, 2025

agrawalreetika Mar 2, 2025 •

edited

Loading

imsayari404 Mar 2, 2025

agrawalreetika Mar 2, 2025

agrawalreetika Mar 2, 2025

imsayari404 Mar 2, 2025

agrawalreetika Mar 3, 2025

imsayari404 Mar 3, 2025 •

edited

Loading

	@Config("hive.s3.webidentity.enabled")
	@Config("hive.s3.web.identity.auth.enabled")

		if (webIdentityEnabled) {
		config.setBoolean(S3_WEB_IDENTITY_ENABLED, true);

		boolean webIdentityEnabled = conf.getBoolean("presto.hive.s3.webidentity.enabled", false);
		String webIdentityTokenFile = conf.get("presto.hive.s3.web-identity-token-file");

	public void initialize(URI uri, Configuration conf)
	throws IOException
	{
	requireNonNull(uri, "uri is null");
	requireNonNull(conf, "conf is null");
	super.initialize(uri, conf);
	setConf(conf);

	this.uri = URI.create(uri.getScheme() + "://" + uri.getAuthority());
	this.workingDirectory = new Path(PATH_SEPARATOR).makeQualified(this.uri, new Path(PATH_SEPARATOR));

	HiveS3Config defaults = new HiveS3Config();
	this.stagingDirectory = new File(conf.get(S3_STAGING_DIRECTORY, defaults.getS3StagingDirectory().toString()));
	this.maxAttempts = conf.getInt(S3_MAX_CLIENT_RETRIES, defaults.getS3MaxClientRetries()) + 1;
	this.maxBackoffTime = Duration.valueOf(conf.get(S3_MAX_BACKOFF_TIME, defaults.getS3MaxBackoffTime().toString()));
	this.maxRetryTime = Duration.valueOf(conf.get(S3_MAX_RETRY_TIME, defaults.getS3MaxRetryTime().toString()));
	int maxErrorRetries = conf.getInt(S3_MAX_ERROR_RETRIES, defaults.getS3MaxErrorRetries());
	boolean sslEnabled = conf.getBoolean(S3_SSL_ENABLED, defaults.isS3SslEnabled());
	Duration connectTimeout = Duration.valueOf(conf.get(S3_CONNECT_TIMEOUT, defaults.getS3ConnectTimeout().toString()));
	Duration socketTimeout = Duration.valueOf(conf.get(S3_SOCKET_TIMEOUT, defaults.getS3SocketTimeout().toString()));
	int maxConnections = conf.getInt(S3_MAX_CONNECTIONS, defaults.getS3MaxConnections());
	this.multiPartUploadMinFileSize = conf.getLong(S3_MULTIPART_MIN_FILE_SIZE, defaults.getS3MultipartMinFileSize().toBytes());
	this.multiPartUploadMinPartSize = conf.getLong(S3_MULTIPART_MIN_PART_SIZE, defaults.getS3MultipartMinPartSize().toBytes());
	this.isPathStyleAccess = conf.getBoolean(S3_PATH_STYLE_ACCESS, defaults.isS3PathStyleAccess());
	this.useInstanceCredentials = conf.getBoolean(S3_USE_INSTANCE_CREDENTIALS, defaults.isS3UseInstanceCredentials());
	this.pinS3ClientToCurrentRegion = conf.getBoolean(S3_PIN_CLIENT_TO_CURRENT_REGION, defaults.isPinS3ClientToCurrentRegion());
	this.s3IamRole = conf.get(S3_IAM_ROLE, defaults.getS3IamRole());
	this.s3IamRoleSessionName = conf.get(S3_IAM_ROLE_SESSION_NAME, defaults.getS3IamRoleSessionName());
	verify(!(useInstanceCredentials && conf.get(S3_IAM_ROLE) != null),
	"Invalid configuration: either use instance credentials or specify an iam role");
	verify((pinS3ClientToCurrentRegion && conf.get(S3_ENDPOINT) == null) \|\| !pinS3ClientToCurrentRegion,
	"Invalid configuration: either endpoint can be set or S3 client can be pinned to the current region");
	this.sseEnabled = conf.getBoolean(S3_SSE_ENABLED, defaults.isS3SseEnabled());
	this.sseType = PrestoS3SseType.valueOf(conf.get(S3_SSE_TYPE, defaults.getS3SseType().name()));
	this.sseKmsKeyId = conf.get(S3_SSE_KMS_KEY_ID, defaults.getS3SseKmsKeyId());
	this.s3AclType = PrestoS3AclType.valueOf(conf.get(S3_ACL_TYPE, defaults.getS3AclType().name()));
	String userAgentPrefix = conf.get(S3_USER_AGENT_PREFIX, defaults.getS3UserAgentPrefix());
	this.skipGlacierObjects = conf.getBoolean(S3_SKIP_GLACIER_OBJECTS, defaults.isSkipGlacierObjects());
	this.s3StorageClass = conf.getEnum(S3_STORAGE_CLASS, defaults.getS3StorageClass());

	ClientConfiguration configuration = new ClientConfiguration()
	.withMaxErrorRetry(maxErrorRetries)
	.withProtocol(sslEnabled ? Protocol.HTTPS : Protocol.HTTP)
	.withConnectionTimeout(toIntExact(connectTimeout.toMillis()))
	.withSocketTimeout(toIntExact(socketTimeout.toMillis()))
	.withMaxConnections(maxConnections)
	.withUserAgentPrefix(userAgentPrefix)
	.withUserAgentSuffix(S3_USER_AGENT_SUFFIX);

	this.credentialsProvider = createAwsCredentialsProvider(uri, conf);
	this.s3 = createAmazonS3Client(conf, configuration);
	}

	public void testAssumeRoleCredentials()
	throws Exception
	{
	Configuration config = new Configuration();
	config.set(S3_IAM_ROLE, "role");
	config.setBoolean(S3_USE_INSTANCE_CREDENTIALS, false);

	try (PrestoS3FileSystem fs = new PrestoS3FileSystem()) {
	fs.initialize(new URI("s3n://test-bucket/"), config);
	assertInstanceOf(getAwsCredentialsProvider(fs), STSAssumeRoleSessionCredentialsProvider.class);
	}
	}

		config.setBoolean("presto.hive.s3.web.identity.auth.enabled", true);
		config.set("presto.hive.s3.web-identity-token-file", "/path/to/token");

Enhance S3 Web Identity authentication & security mapping #24645

Are you sure you want to change the base?

Enhance S3 Web Identity authentication & security mapping #24645

Conversation

imsayari404 commented Feb 27, 2025 • edited Loading

Description

Changes

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

imjalpreet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjalpreet left a comment

Choose a reason for hiding this comment

imsayari404 commented Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

agrawalreetika Mar 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imsayari404 Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

imsayari404 commented Feb 27, 2025 •

edited

Loading

imsayari404 commented Feb 28, 2025 •

edited

Loading

agrawalreetika Mar 2, 2025 •

edited

Loading

imsayari404 Mar 3, 2025 •

edited

Loading