feat: add direct S2A authentication support for HTTP transport - #5069
sruthi-talluri wants to merge 8 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Secure Application-to-Application (S2A) authentication in gcsfuse by adding new configuration options (s2a-address and s2a-spiffe-id), updating flag validation to support alphanumeric characters, and configuring the HTTP client transport to use S2A-based mTLS. A critical compilation error was identified in internal/storage/storageutil/client.go where s2a.NewS2ADialTLSContextFunc is called without capturing its second return value (an error), which will cause a build failure.
fa90aa5 to
424496d
Compare
98170ed to
f8f6c8f
Compare
sruthi-talluri
added
execute-perf-test
anushka567
added
execute-integration-tests
ed1f972 to
614ad41
Compare
sruthi-talluri
removed
execute-perf-test
614ad41 to
979c26c
Compare
sruthi-talluri
added
the
remind-reviewers
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
4074756 to
95bbaeb
Compare
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
| if storageClientConfig.ClientProtocol == cfg.HTTP1 { | ||
| transport = &http.Transport{ | ||
| DialContext: dialer.DialContext, | ||
| DialTLSContext: dialTLSContext, |
There was a problem hiding this comment.
did you test if http1 & http2 are working fine. I am wondering if setting empty dialTLSContext has any implications
There was a problem hiding this comment.
Both were tested, but I did have to make few changes.
The empty dialTLSContext itself is harmless: it's a func-typed field, so nil is identical to leaving it unset — hasCustomTLSDialer() returns false and dialConn takes the standard path, unchanged from before this PR.
The http1 path was broken though. s2a-go hardcodes ALPN to h2 and its dialer returns a *tls.Conn, so http.Transport sees NegotiatedProtocol h2. Our http1 branch sets TLSNextProto to an empty map to disable HTTP/2, so it would send HTTP/1.1 over an h2-negotiated connection and fail with malformed HTTP response "\x00\x00$\x04..." (the server's SETTINGS frame).
My earlier http1 check only hit GetStorageLayout, which goes over the gRPC control client, so it missed this.
Fixed: the http1 branch now builds the config via s2a.NewTLSClientConfigFactory and pins NextProtos to http/1.1. http2 is unchanged. Verified against s2a-go's fake S2A server — http1 negotiates HTTP/1.1, http2 negotiates h2, and the old path reproducibly fails.
Happy to add regression tests if you'd like them here.
652f03a to
60d619a
Compare
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
3 similar comments
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
…mappings Add configuration fields and CLI flags for direct S2A authentication: - s2a-address: daemon address for direct S2A mTLS authentication - s2a-spiffe-id: local SPIFFE ID to assume for direct S2A authentication - update config-gen parser to support alphanumeric tokens in flag names - map flags to StorageClientConfig in cmd/legacy_main.go and add flag parsing unit tests
Implement S2A mTLS transport dialer for HTTP storage client: - construct S2A DialTLSContext using s2a.NewClientOptions when S2AAddress is configured - support both HTTP/1 and HTTP/2 client protocols with S2A transport - bypass token acquisition and TokenSource in CreateHttpClient when S2A is active - add unit tests verifying S2A transport creation and mock S2A daemon dialer connection
… control clients Configure S2A transport credentials for gRPC storage and control clients: - use s2av2.NewClientCredentials in createClientOptionForGRPCClient when S2AAddress is configured - bypass standard token authentication when S2A credentials are used - add unit tests verifying client options assembly for gRPC and HTTP handles with S2A
Two issues prevented an S2A-authenticated mount from working.
First, http1 could not talk to GCS at all. s2a-go hardcodes the client TLS
config's NextProtos to {"h2"}, and NewS2ADialTLSContextFunc returns a
*tls.Conn, so http.Transport records a negotiated protocol of "h2". The http1
branch deliberately sets TLSNextProto to an empty map to disable HTTP/2, so
the h2 handler lookup misses and the transport ends up speaking HTTP/1.1 over
an h2-negotiated connection. Every request then fails while parsing the
server's first HTTP/2 frame.
Build the TLS config for the http1 branch via s2a.NewTLSClientConfigFactory
and pin NextProtos to http/1.1 before dialing. The http2 branch continues to
use s2a.NewS2ADialTLSContextFunc unchanged.
Second, neither transport set VerificationMode on s2a.ClientOptions, so S2A
received UNSPECIFIED. The legacy S2A certificate verifier treats UNSPECIFIED
the same as CONNECT_TO_GOOGLE, so this went unnoticed, but the verifier used
on the delegated-SPIFFE path falls through to SPIFFE peer verification
instead and rejects the GCS server certificate with "Leaf SVID must have
exactly one URI SAN representing the SPIFFE ID". GCS is a Google endpoint
serving a WebPKI certificate, so ask for CONNECT_TO_GOOGLE explicitly on both
the HTTP and gRPC clients, which is what s2a.DefaultClientOptions already
does.
Verified end to end against s2a-go's fake S2A v2 server: http1 negotiates
HTTP/1.1 and succeeds, http2 still negotiates HTTP/2.0, and the previous
http1 path reproducibly fails. Also verified on an ACE staging host against
the local S2A daemon with a delegated SPIFFE identity: without the
verification mode the handshake fails with the SVID error above, and with it
the handshake completes and requests reach GCS.
S2A secures the channel but conveys no IAM principal to GCS, so an access token is still required. Both the HTTP and the gRPC path treated S2A and the token as mutually exclusive, so an S2A mount sent no caller identity at all and GCS rejected every request with: Unauthenticated: Anonymous caller does not have storage.objects.list access storageutil/client.go: the oauth2.Transport wrapper was gated on `!AnonymousAccess && S2AAddress == ""`. Drop the S2A term so the token is attached whenever access is not anonymous. storage_handle.go: createClientOptionForGRPCClient built the S2A transport credentials and then appended option.WithoutAuthentication(), which suppressed the per-RPC token. Split the function into two independent blocks, one that selects the transport (S2A or default) and one that selects the identity (anonymous, google lib auth, or token source), and drop WithoutAuthentication() from the S2A path. The two compose correctly in google-api-go-client: transport/grpc/dial.go installs the default transport credentials first and appends the caller's GRPCDialOpts last, so the S2A credentials win, while the token source is installed separately as per-RPC credentials. Tests: TestCreateHttpClientWithS2A_DialVerification now supplies a static token source, since it previously failed the token fetch before reaching the dial. Adds TestCreateHttpClientWithS2A_AttachesOAuthTransport and TestCreateHttpClientWithAnonymousAccess_NoOAuthTransport, which assert the RoundTripper chain directly rather than only checking for a nil error.
proxyTokenSource.Token() issued a bare client.Get with no headers. The token URL points at a GCE-metadata-compatible endpoint -- the metadata server itself, or a proxy in front of it such as ACE's MDS Proxy -- and those servers require the Metadata-Flavor: Google header. Requiring a non-standard header is what prevents a browser or other naive HTTP client from being coerced into fetching a token on an attacker's behalf, since a cross-origin request cannot set one. Because gcsfuse did not send it, ACE had to exempt the token endpoint from the check, leaving an SSRF/CSRF hole open (b/553201361). Sending the header unconditionally lets that exemption be removed, and is harmless against endpoints that do not inspect it. Build the request explicitly and set the header rather than using the client.Get shorthand. Tests: adds Test_NewTokenSourceFromURL_SendsMetadataFlavorHeader and Test_NewTokenSourceFromURL_UnixSocket_SendsMetadataFlavorHeader, covering both the TCP and the unix-socket transports, since the UDS path rewrites the request URL and is worth asserting separately.
v0.1.10 includes 489047f, "Client TLS configs should plumb identity to the
S2A server", which fixes LocalIdentity being dropped on the client TLS config
path. GCSFuse builds its http1 dialer through s2a.NewTLSClientConfigFactory,
so on v0.1.9 a delegated SPIFFE identity never reached the S2A server and the
handshake used the host identity instead.
The ALPN workaround in storageutil.newS2ADialTLSContextForHTTP1 is still
required. v0.1.10 continues to hardcode the client TLS config's NextProtos to
{"h2"}; google/s2a-go#159 makes that selectable and
the workaround can be removed once it ships.
Verified: go build ./... and go test -count=1 ./internal/storage/... ./cfg/...
both pass.
b48a057 to
b0e7ea9
Compare
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
1 similar comment
|
Hi @anushka567, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
3 participants
Description
This PR adds Direct S2A (Secure Session Agent) authentication support to GCSFuse over HTTP (client-protocol:
http1andhttp2) and gRPC.Direct S2A authentication is enabled whenever the
--s2a-addressCLI flag (ors2a-addressconfiguration field) is specified:s2a.NewS2ADialTLSContextFuncwith optional local SPIFFE ID identity (--s2a-spiffe-id) to establish mTLS transport channels directly with Google Cloud Storage endpoints.s2a.NewClientCreds) for both storage and control client channels.--s2a-addressis provided, standard OAuth2 token fetching is bypassed (option.WithoutAuthentication()), relying entirely on S2A mTLS handshake credentials.--s2a-addressis omitted or empty, standard credential resolution paths (OAuth2, ADC, etc.) are preserved unchanged.Verification & Testing
All unit tests in
cfg,cmd, andinternal/storagepass cleanly (100% OK):cfgandcmd.internal/storage/storage_handle_test.goandinternal/storage/storageutil/client_test.go.Verified in the Agent Compute Engine (ACE) staging environment against host pool
gcsfuse-s2a-integration(gcsfuse-s2a-integration-0inus-central1-staginga) running the E2E shared storage test suite (UvmSharedStorageE2eIntegrationTest):/shared/test_shared_<nonce>.txt./shared/test_shared_<nonce>.txt, and verified cross-uVM consistency.FILE_NOT_FOUND).Verified direct S2A mounting on host instance
gcsfuse-s2a-integration-0with local S2A daemon (169.254.169.254:8083):--client-protocol=http1): Initiated S2A mTLS handshake with Google Cloud Storage (GetStorageLayoutreturned in 79ms), mounted bucket, and unmounted cleanly.--client-protocol=http2): Initiated S2A mTLS handshake with Google Cloud Storage (GetStorageLayoutreturned in 54ms), mounted bucket, and unmounted cleanly.Link to Issue / Bug
Bug: b/550402283