Skip to content

Signed writes break on a same-origin 301/302/303: the redirect rewrites POST to a bodyless GET but keeps the signature headers #336

Description

@beardthelion

Both signing clients follow a same-origin 301, 302 or 303 by rewriting the request to a bodyless GET while keeping the RFC 9421 headers attached. The signature then describes a POST and a body that no longer exist, so the node rejects the write with 401 invalid_signature. Only 307 and 308 preserve the method.

The signing string covers @method, @path and content-digest (COVERED_COMPONENTS, crates/gitlawb-core/src/http_sig.rs), and require_signature rebuilds all three from the request it actually received. The redirect predicate gitlawb_core::redirect::may_follow decides on host, port, path, query and scheme-downgrade only; it never sees the response status or the request method, so a status that rewrites the method passes it.

reqwest 0.12.28 delegates redirect following to tower-http's FollowRedirect. On MOVED_PERMANENTLY or FOUND it converts a POST to GET and empties the body; on SEE_OTHER it does so for any non-HEAD method. Its drop_payload_headers removes only Content-Type, Content-Length, Content-Encoding and Transfer-Encoding, so Signature, Signature-Input and Content-Digest ride along untouched.

What was measured

Against the PR #173 branch, driving NodeClient::post (the path post, put and delete all share via send_signed) at a mockito origin whose Location is the identical path and query, and recording what the target received:

301: target reached as GET, body_len=0, signature-input PRESENT, signature PRESENT,
     content-digest still sha-256=:J1A8i1XWzdkl...:  (the discarded body), content-type absent
302: same
303: same
307: no GET arrives, method preserved
308: no GET arrives, method preserved

Then the arrived request was run through the same verification the middleware performs, rebuilding @method and @path from the request as received:

method_as_received=GET  verified=false   (the signature was made over POST)

So the 401 is demonstrated, not inferred. content-type being absent while content-digest survives is the drop_payload_headers boundary showing through: the digest header is not in its list, so it stays and describes bytes that were dropped.

One caveat on the 307/308 rows. The fixture redirects to the identical path, so a preserved POST loops back into the bounce mock until the chain bound. That proves the method is not rewritten on 307/308, which is the point, but it is not evidence about whether the body survives a 307.

Blast radius

Both signing clients, since they share the predicate:

  • gl writes: NodeClient::post, put and delete all route through send_signed, so issue creation, PR creation, comments, reviews, webhooks, bounties and profile writes are all exposed.
  • git-remote-gitlawb: build_pack_post_request signs a POST carrying the pack and uses the same client policy, so a push is exposed. This is the higher-stakes path.

Nothing in the node emits a 3xx outside tests (grepped for the redirect statuses, Redirect:: and Location in crates/gitlawb-node/src), so the trigger is a fronting proxy or load balancer rather than the node itself. The ordinary case is a proxy that answers http:// with a 301 to https://, which is exactly the same-origin hop the predicate was written to allow.

Failure is loud: the write fails with a 401 rather than silently doing the wrong thing. There is no signature leak, since the hop is same-origin by construction.

Suggested direction

reqwest::redirect::Attempt exposes the response status, so the two policy closures (crates/gl/src/http.rs, crates/git-remote-gitlawb/src/main.rs) can refuse 301, 302 and 303 while continuing to follow an identical-target 307 or 308. That keeps the http-to-https upgrade working for reads and for the method-preserving statuses, and it refuses exactly the hops that invalidate the signature. Sending body-carrying signed writes through a client built with Policy::none() would also work and is simpler, at the cost of the upgrade on those routes.

Worth a test on both clients that drives a signed POST into an identical-target 301 and asserts the target is never reached.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:coregitlawb-core — identity, certs, encrypt, DID/UCANcrate:git-remotegit-remote-gitlawb — the git remote helpercrate:glgl — the contributor CLIcrate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:mediumDegraded but workaround existssubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions