Conversation
…S3 requests, if they don't pass in the value, use credential provider for default accountID.
| @MutuallyExclusiveWith(tag = "configFile or config") | ||
| private String configJson; | ||
|
|
||
| @Option(name = "--expected-bucket-owner", description = "Expected bucket owner account ID for S3 bucket verification.") |
There was a problem hiding this comment.
I'm a bit concerned that this could lead to confusion as we are essentially adding 2 independent and unrelated expectedBucketOwner parameters. From what I can tell, the only purpose of this one is to protect the one GetObject request in JSONResource.getFromS3(), while the parameter in NeptuneExportLambda guards all PutObject requests from the program. If a user wanted to configure both of these values in a export service request, their request would look something like this:
neptune-export.sh nesvc --json '
"command" : "export-pg",
"params" : {
"endpoint": <neptune endpoint>,
"configFile": <config file s3 URI>,
"expectedBucketOwner": <config file bucket account id>
},
"outputS3Path": <target s3 URI>,
"expectedBucketOwner": <target bucket account id>
I'm not convinced that such a guard is necessary for retrieving the config file from S3, especially considering users are also permitted to provide any public https URI as a location for this file as well. Unfortunately due to the structure of Neptune export and the export service, it's quite difficult to combine these into a single parameter. If you believe this guard is necessary, I'd ask that we rename it to something such as --config-file-expected-bucket-owner to distinguish it from the more general expectedBucketOwner parameter.
| .bucket(s3ObjectInfo.bucket()) | ||
| .prefix(s3ObjectInfo.key()) | ||
| .maxKeys(1) | ||
| .expectedBucketOwner(StringUtils.isBlank(expectedBucketOwner) ? this.s3CredentialsProvider.resolveCredentials().accountId().get() : expectedBucketOwner) |
There was a problem hiding this comment.
StringUtils.isBlank(expectedBucketOwner) ? this.s3CredentialsProvider.resolveCredentials().accountId().get() : expectedBucketOwner is repeated quite a bit through out this PR, could we move this account resolution logic to the source in NeptuneExportLambda such that it only needs to be done once?
| Expected bucket owner account ID for S3 bucket verification. When provided, | ||
| verifies the bucket is owned by the specified AWS account. |
There was a problem hiding this comment.
| Expected bucket owner account ID for S3 bucket verification. When provided, | |
| verifies the bucket is owned by the specified AWS account. | |
| Expected bucket owner account ID for S3 bucket verification. When provided, | |
| verifies the bucket is owned by the specified AWS account. Defaults to the | |
| AWS account id associated with the S3 credentials. |
| | `CONFIG_FILE_S3_PATH` | `configFileS3Path` | S3 location of a JSON config file to be used when exporting a property graph from a config file | Optional | | ||
| | `COMPLETION_FILE_S3_PATH` | `completionFileS3Path` | S3 location to which a completion file should be written once all export files have been copied to S3 | Optional | | ||
| | `SSE_KMS_KEY_ID` | `sseKmsKeyId` | ID of the customer managed AWS-KMS symmetric encryption key to used for server-side encryption when exporting to S3 | Optional | | ||
| | `EXPECTED_BUCKET_OWNER` | `expectedBucketOwner` | Expected bucket owner account ID for S3 bucket verification. When provided, verifies the bucket is owned by the specified AWS account | Optional | |
There was a problem hiding this comment.
| | `EXPECTED_BUCKET_OWNER` | `expectedBucketOwner` | Expected bucket owner account ID for S3 bucket verification. When provided, verifies the bucket is owned by the specified AWS account | Optional | | |
| | `EXPECTED_BUCKET_OWNER` | `expectedBucketOwner` | Expected bucket owner account ID for S3 bucket verification. When provided, verifies the bucket is owned by the specified AWS account. Defaults to the AWS account id associated with the S3 credentials. | Optional | |
2 participants
Add S3 Bucket Owner Verification Support
This PR adds optional S3 bucket owner verification to neptune-export, enhancing security by allowing clients to verify that S3 buckets are owned by expected AWS accounts.
Changes
New Parameter: expectedBucketOwner
Security Benefits
This feature helps prevent unauthorized access to S3 buckets by verifying bucket ownership before performing operations, adding an extra layer of security for cross-account scenarios.