HDDS-15141. Support HMAC-SHA256 signature verification for S3 chunked uploads - #11061
HDDS-15141. Support HMAC-SHA256 signature verification for S3 chunked uploads#11061rich7420 wants to merge 2 commits into
HMAC-SHA256 signature verification for S3 chunked uploads#11061Conversation
… verification Add ChunksValidator (HMAC-SHA256 per-chunk signature verification, verified against the AWS SigV4 streaming test vector) and wire it into SignedChunksInputStream as an optional validator. Add SIGNATURE_DOES_NOT_MATCH.
Wire ChunksValidator into the signed multi-chunk PUT paths: single-object PUT (streaming and non-streaming) and multipart UploadPart (streaming and non-streaming). The OM-derived signing key (HDDS-15140) is only available after the key is opened, so SignedChunksInputStream gains a guarded attachValidator() that must be called before the first read; the endpoint requests the key via derivedKeyPiggyBacking and attaches the validator once the key is open. Enforcement follows secure mode: in secure mode OM always returns the derived key for a signed upload, so a missing key is rejected (INTERNAL_ERROR) rather than stored unverified; in non-secure mode there is no secret to verify against and verification is skipped. Teach OzoneBucketStub the HDDS-15140 derivedKeyPiggyBacking stream overloads, and add attachValidator tests (deferred attach, tamper rejection, before-read and attach-once guards).
|
cc @chungen0126 , @jojochuang and @peterxcli |
chungen0126
left a comment
chungen0126
left a comment
There was a problem hiding this comment.
Thanks @rich7420 for working on this.
| /** The signed chunk stream, if the payload is a signed multi-chunk upload. */ | ||
| private final SignedChunksInputStream signedChunksInputStream; |
There was a problem hiding this comment.
I think the purpose of this field is a bit unclear. If we only need to indicate whether S3ChunkInputStreamInfo is a signed stream, adding a boolean flag would suffice. Furthermore, if we need to get the signedChunksInputStream instance, we could probably retrieve it from multiDigestInputStream instead.
|
|
||
| private static byte[] hmacSha256(byte[] key, String msg) { | ||
| try { | ||
| Mac mac = Mac.getInstance(HMAC_SHA256); |
There was a problem hiding this comment.
Calling Mac.getInstance() every time here can incur some performance overhead. A better approach would be to cache it using a ThreadLocal instance to improve performance.
| /** Set on the first read; blocks attaching a validator once reading began. */ | ||
| private boolean readStarted; |
There was a problem hiding this comment.
Could you clarify under what scenario the validator would be attached after the reading has already started?
| private static byte[] signingKey(String date, String region, String service) { | ||
| byte[] key = hmac(("AWS4" + SECRET_KEY).getBytes(UTF_8), date); | ||
| key = hmac(key, region); | ||
| key = hmac(key, service); | ||
| return hmac(key, "aws4_request"); | ||
| } | ||
|
|
||
| private static byte[] hmac(byte[] key, String msg) { | ||
| try { | ||
| Mac mac = Mac.getInstance("HmacSHA256"); | ||
| mac.init(new SecretKeySpec(key, "HmacSHA256")); | ||
| return mac.doFinal(msg.getBytes(UTF_8)); | ||
| } catch (Exception e) { | ||
| throw new IllegalStateException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
It looks like this is actually calculating the HMAC here, but I don't think this logic belongs in this specific test class. Could we extract these signing helpers into a shared test utility class (or perhaps mock them) to keep this class focused solely on testing the stream's behavior?
What changes were proposed in this pull request?
This task aims to introduce
ChunksValidatorto verify signature for the STREAMING-AWS4-HMAC-SHA256-PAYLOAD algorithm within the S3 Gateway.The primary goal is to enable SignedChunksInputStream to verify the authenticity of each data chunk in real-time. By calculating the HMAC-SHA256 hash of the streaming payload and comparing it against the chunk-signature provided in the S3 chunk header, S3G can ensure data integrity and reject tampered or unauthorized data before it reaches the backend storage.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15141
How was this patch tested?
https://github.com/rich7420/ozone/actions/runs/32246615796