refactor(remote-config): Trusted-entitlements signing for the binary remote config response#3601
Conversation
4193b51 to
17f6f5d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3601 +/- ##
=======================================
Coverage 80.30% 80.31%
=======================================
Files 378 378
Lines 15475 15503 +28
Branches 2147 2154 +7
=======================================
+ Hits 12427 12451 +24
- Misses 2189 2193 +4
Partials 859 859 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
17f6f5d to
e30cd22
Compare
4cd4c47 to
2538600
Compare
e30cd22 to
c161d23
Compare
| assertThat(parsed.config.data.let { buf -> ByteArray(buf.remaining()).also { buf.duplicate().get(it) } }) | ||
| .isEqualTo(config) | ||
| assertThat(parsed.contentElements).hasSize(1) | ||
| assertThat(verification).isEqualTo(VerificationResult.VERIFIED) |
There was a problem hiding this comment.
I will want to add an integration test for this request once the backend is working and we have tested it... for now, I haven't been able to test this yet.
7ea7a07 to
678860e
Compare
e730be7 to
7df7393
Compare
42d3db7 to
84b96ec
Compare
7df7393 to
18e1888
Compare
3799228 to
aca57d7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aca57d7. Configure here.
aca57d7 to
c07f7b1
Compare
📸 Snapshot Test593 unchanged
🛸 Powered by Emerge Tools |
rickvdl
left a comment
There was a problem hiding this comment.
Awesome work! As far as I can see this is in line with what we discussed. Just two questions :)
| nonce = null, | ||
| bodyBytes = config.checksumBytes(), | ||
| requestTime = getRequestTimeHeader(connection), | ||
| eTag = getETagHeader(connection), |
There was a problem hiding this comment.
How is this handled, since we don't have etag support for these?
There was a problem hiding this comment.
Right, I left this a bit as a failsafe TBH... We are indeed not storing etags for this endpoint. The idea here is that, if the backend does send an etag, even if we don't store it, as long as it uses the same signing mechanism, it should be part of the signature, so we get it from the response headers to calculate the signature. This might not happen if the backend does not provide an etag, but this would also handle it.
So yeah, basically a guard just in case the backend does send etags.
There was a problem hiding this comment.
Makes sense, thanks for clarifying!
| @@ -393,7 +394,11 @@ internal class HTTPClient( | |||
| val verificationResult = if (shouldSignResponse && | |||
| RCHTTPStatusCodes.isSuccessful(responseCode) | |||
There was a problem hiding this comment.
How do we handle 204's here? Should we early exit and not do any verification since we don't have any content?
There was a problem hiding this comment.
That's a valid question!! And yeah, right now I wasn't handling it specially or anything, but if we get a 204 (and no content), I don't think there is a need to verify anything (no chance of MiTM attack I would say, so we can just assume verified. Let me know if you think that's ok!
Enables Ed25519 signature verification for GetRemoteConfig: the backend signs the config element's 24-byte truncated SHA-256 checksum, and the SDK both verifies that signature and independently confirms the config bytes hash to it (SigningManager now accepts a raw-bytes body; HTTPClient.verifyBinaryResponse ties the two checks together). The resulting VerificationResult is surfaced through the getRemoteConfig callback and stored on RemoteConfigManager's cache entry for the follow-up that consumes blobs/entitlements. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c07f7b1 to
86e78b3
Compare
Stacked on #3607 (which adds the binary RC Container request/parsing).
Enables trusted-entitlements signature verification for the binary
GET /v2/configresponse. This hasn't been tested against the actual endpoint yet, but this endpoint is currently unused so should be safe to merge.What
GetRemoteConfig.supportsSignatureVerification = true. The response signature is verified over the config element's 24-byte truncated SHA-256 checksum (no nonce). Two independent checks are both required, elseFAILED:X-Signature(Ed25519) covers the checksum -> backend authenticity.RCElement.isChecksumValid()ties the checksum to the actual config bytes.SigningManager: signed body is nowByteArray?(binary-safe).HTTPClient.verifyBinaryResponseparses the container, checks integrity, and verifies the signature over the config checksum.VerificationResultis surfaced through thegetRemoteConfigcallback.Note
Medium Risk
Touches signature verification and changes the getRemoteConfig callback shape, but scope is limited to the still-unused remote config endpoint with strong test coverage.
Overview
Enables trusted-entitlements signature verification for
GET /v2/configRC Container responses and threads the outcome to callers.GetRemoteConfigis now in the set of endpoints that require response signing. Verification is not over the raw body: the client parses the container, requiresRCElement.isChecksumValid()on the config element, then verifiesX-Signatureover that element’s 24-byte truncated SHA-256 checksum (no nonce / post-params). Empty 204 responses are treated asVERIFIEDwithout calling the signer.SigningManager.verifyResponsenow takesByteArray?for the signed payload so JSON bodies and binary checksums use the same API.getRemoteConfigsuccess callbacks are(RCContainer?, VerificationResult)instead of container-only.Reviewed by Cursor Bugbot for commit de07e2b. Bugbot is set up for automated code reviews on this repo. Configure here.