Reclient issues requests with duplicate Authorization headers

[MarshallOfSound]

As in the title, outgoing requests from reclient include the Authorization header twice (with the exact same value)

On paper this doesn't sound too bad but nginx enforces there is only a single authorization header. This means that folks using nginx as tls termination in front of their RBE cluster (hi, that's me) can't use reclient with authentication.

My configuration was mostly reverse engineered as the docs are a bit all over the place but I think I pieced it all together correctly.

// reproxy.cfg
async_reproxy_termination=true
cas_concurrency=1000
# compression_threshold=0
deps_cache_max_mb=256
depsscanner_address=exec:///Users/samuel/projects/electron/electron/src/buildtools/reclient/scandeps_server
fail_early_min_action_count=4000
fail_early_min_fallback_ratio=0.5
fast_log_collection=true
log_format=text
max_concurrent_requests_per_conn=50
max_concurrent_streams_per_conn=50
min_grpc_connections=50
use_batches=false
use_unified_uploads=true

I'm then also configuring two options via environment variables:

RBE_service: 'my.rbe.com:443',
RBE_experimental_credentials_helper: 'my/creds/helper',

The credential helper I'm using at the moment is just a binary that echos hardcoded json so nothing funky going on there.

Included below is a screenshot from a mitm proxy (burpsuite) showing the 400 response and the duplicate auth header

I tracked down the "duplicate header injection" part to this core code in grpc-go 😅 And I "fixed" the issue locally by building a version of reproxy with these lines removed. The comment in that file indicates this behavior is intentional so I'm not sure what the path forward is here.

Maybe fixing it so that use_rpc_credentials actually works for credential helper backed requests 🤔 At the moment that only impacts a subset of authentication strategies based on their impl in remote-apis-sdk. Either that or patching grpc-go to never set duplicate Authorization headers specifically to avoid this issue. I'm not familiar enough with go or this ecosystem at large to actually offer a change in the right place but if someone has an answer I'm happy to write the code in the place I'm pointed to 😄

[gkousik]

Just a heads up: the experimental_credentials_helper flag might change in the next few months (and hence the experimental_ prefix). The way we envision credentials helper to actually work is that it will directly return the headers to be included and reproxy will include only those headers with the request (#16). However, it will still use PerRPCCreds so the issue will remain.

That aside, thanks for tracking the issue down - that was some great debugging 😄

The gRPC comment indicates that these will be included twice if both PerRPCCreds are included as part of CallOptions and DialOptions. I have a patch to the remote-apis-sdks where if I remove the PerRPCCreds to clientopts, I no longer see duplicate headers. Can you test and see if this works for you in avoiding the redundant header?

[gkousik]

@MarshallOfSound
Separate question: Burpsuite actually didn't work for me, I got an error like:

I0105 17:17:06.645384 4042768 capabilities.go:68] call failed with err=rpc error: code = Internal desc = server closed the stream without sending trailers, retrying.

Is there a specific extension I should be using to make it work with gRPC?

[MarshallOfSound]

The way we envision credentials helper to actually work is that it will directly return the headers to be included and reproxy will include only those headers with the request

Yup I've been following that issue and any progress on the credential helper work, our current helper returns both the current format and the proposed headers format at the same time so it'll keep working when the switchover is made 😅

Can you test and see if this works for you in avoiding the redundant header?

I actually swapped out the nginx ingress portion of my RBE setup, I can spin it back up again quite quickly to test though this evening.

Separate question: Burpsuite actually didn't work for me, I got an error like:

Ah no, unfortunately burpsuite doesn't successfully proxy GRPC requests 🙃 It just proxies enough of the initial HTTP/2 request to diagnose this issue, I couldn't find a very nice GRPC-capable proxy and burpsuite worked just barely to figure this out so I didn't really look harder 😆

[MarshallOfSound]

I actually swapped out the nginx ingress portion of my RBE setup, I can spin it back up again quite quickly to test though this evening.

Confirmed this appears to fix it 👍

[gkousik]

Ah no, unfortunately burpsuite doesn't successfully proxy GRPC requests 🙃 It just proxies enough of the initial HTTP/2 request to diagnose this issue, I couldn't find a very nice GRPC-capable proxy and burpsuite worked just barely to figure this out so I didn't really look harder 😆

Got it. I was able to setup and make mitmproxy CLI to work (didn't try intercepting requests, but it atleast logs them which gave me sufficient info to confirm the fix).

Confirmed this appears to fix it 👍

thanks for confirming! I'll close this out now, you should see the SDK version bumped in reclient soon.