Skip to content

net.http.signature: add HTTP Message Signatures (RFC 9421) module - #27113

Open
davlgd wants to merge 36 commits into
vlang:masterfrom
davlgd:davlgd-http-signature
Open

net.http.signature: add HTTP Message Signatures (RFC 9421) module#27113
davlgd wants to merge 36 commits into
vlang:masterfrom
davlgd:davlgd-http-signature

Conversation

@davlgd

@davlgd davlgd commented May 8, 2026

Copy link
Copy Markdown
Contributor

This PR adds vlib/net/http/signature — a pure-V implementation of RFC 9421 (HTTP Message Signatures). Sign and verify HTTP requests and responses with the four algorithms backed by vlib/crypto: hmac-sha256, ecdsa-p256-sha256, ecdsa-p384-sha384, ed25519.

No new C code, no new third-party crypto: everything sits on vlib/crypto (ecdsa, ed25519, hmac, sha256) and vlib/crypto/pem for Key.from_pem.

Why

RFC 9421 was published in February 2024. It supersedes the long-running Cavage drafts (draft-cavage-http-signatures-*) that ActivityPub / Mastodon-style federation has been using in non-interoperable variants for years, and gives the ecosystem a single normative spec with stable header names and a stable signature-base format. The signed-request model authenticates HTTP messages end-to-end across TLS terminators and lets multiple per-hop signatures (client → proxy → backend) coexist on the same request. The new IETF Web Bot Auth draft is built directly on top of it. With vlib/net/http already in the stdlib, V apps could not produce or verify RFC 9421 signatures until now.

What's covered

Algorithm name (IANA HTTP Signature Algorithms registry) Reference Status
hmac-sha256 RFC 9421 §3.3.3
ecdsa-p256-sha256 RFC 9421 §3.3.4
ecdsa-p384-sha384 RFC 9421 §3.3.5
ed25519 RFC 9421 §3.3.6

The RFC 9421 §2.2 derived components implemented are @method, @target-uri, @authority, @scheme, @request-target, @path, @query, @status — the @query-param selector (§2.2.8) is the only one missing and is called out below as deferred. Plain HTTP fields are matched by lowercased name, multi-value joined as ", ", OWS trimmed (RFC 9421 §2.1). Optional outer behaviours: multiple co-existing signatures merged into a single Structured Field per RFC 8941 §3.2; expires enforced when the caller passes now_unix > 0.

rsa-pss-sha512 and rsa-v1_5-sha256 are intentionally out of scope — vlib/crypto does not yet ship an RSA implementation. Adding them is mechanical once it does. @query-param, sf / key / bs parameter handling are deferred to a follow-up PR.

Module surface

import net.http
import net.http.signature
import time
// Sign an outbound request (Ed25519 + PEM-encoded private key).
priv := signature.Key.from_pem(alice_private_pem)!.with_keyid('alice')
signature.sign_request(mut req, priv,
    components: ['@method', '@target-uri', '@authority', 'date'])!
// Verify on the server side.
pub_key := signature.Key.from_pem(alice_public_pem)!
signature.verify_request(req, pub_key, now_unix: time.now().unix())!

Key.from_pem accepts the canonical PKCS#8 / SPKI / SEC1 PEM blocks openssl genpkey and friends produce; the raw-coordinate constructors (Key.ed25519_private(seed), Key.ecdsa_p256_public(x, y), …) remain for callers that have JWK-shaped material. created defaults to time.now().unix() when omitted, since RFC 9421 §7.2.1 RECOMMENDS it for replay protection.

A complete example program lives at examples/http_signature.v.

Conformance / test vectors

RFC 9421 Appendix B vectors are vendored under vlib/net/http/signature/tests/rfc9421/ and exercised by rfc9421_test.v:

Vector Algorithm Mode
§B.2.5 hmac-sha256 bytes-exact
§B.2.6 ed25519 bytes-exact
§B.2.4 ecdsa-p256-sha256 verify (ECDSA non-deterministic)

Both byte-exact tests reproduce the RFC reference signature down to the last base64 character; the ECDSA case verifies the reference signature and adds an independent sign-then-verify roundtrip.

In addition to the public corpus:

  • http_message_test.v — sign/verify roundtrips for HMAC, Ed25519, ECDSA P-256 (RFC key) and ECDSA P-384 (fresh keypair via ecdsa.generate_key); tampered-URL rejection; missing-header rejection; expires enforcement; two-signature coexistence; alg mismatch rejection; label grammar (Structured Field key form).
  • structured_field_test.v — Inner List + parameter serialisation pinned (("@method" "host");created=N;keyid="…" byte-for-byte), escape rules for quoted strings, multi-entry / single-entry parsing, raw signature_params_value preservation, and non-canonical wire-order verification — proves the verifier replays the wire param substring verbatim instead of re-serialising in a fixed canonical order, which is what makes interop with stacks that emit ;keyid=…;created=… (instead of the inverse) work.
  • key_test.vKey.from_pem round-trip with the RFC §B.1.3 P-256 PEM and §B.1.4 Ed25519 PEM, byte-exact RFC §B.2.6 signature reproduction via Key.from_pem, and rejection of unsupported PEM block types (RSA PRIVATE KEY etc).

I've also tested these modules against lib in other languages to check interop.

Out of scope (deliberate)

  • RSA signaturesrsa-pss-sha512 and rsa-v1_5-sha256 need an RSA-PSS implementation in vlib/crypto, which is a separate effort. Adding them is purely additive once it lands.
  • @query-param derived component (RFC 9421 §2.2.8) — defers per-parameter selection rules; rare in practice and easy to add later.
  • Structured-field re-serialisation parameters (sf, key, bs from §2.1.x) — used when signing structured-field values themselves; out of v1 to keep the parser narrow.
  • Body covered components beyond what Components.fields already supportsContent-Digest is signed as a regular header field; computing the digest itself stays the caller's concern (matches what every other RFC 9421 stack does).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b79a1623db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment thread vlib/net/http/signature/key.v
Comment thread vlib/net/http/signature/components.v
@medvednikov

Copy link
Copy Markdown
Member

vfmt

@davlgd
davlgd force-pushed the davlgd-http-signature branch from f3e774d to f9dea48 Compare May 8, 2026 18:58
@davlgd

davlgd commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

vfmt

Sorry I missed it before the last push. It's fixed.

@JalonSolov

Copy link
Copy Markdown
Collaborator

Run v git-fmt-hook install in your repo, and v fmt will be run on all your V files on every commit.

@medvednikov medvednikov reopened this May 20, 2026
@medvednikov

Copy link
Copy Markdown
Member
  builder error: 'openssl/param_build.h' not found
  FAIL  [  1/271] C:  4079.1 ms, R:     0.000 ms examples/http_signature.v

@davlgd

davlgd commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

So I should add // vtest build: present_openssl? && !(openbsd && gcc) && !(sanitize-memory-clang || docker-ubuntu-musl) ?

@JalonSolov

Copy link
Copy Markdown
Collaborator

@medvednikov ☝🏼

@davlgd
davlgd force-pushed the davlgd-http-signature branch from f9dea48 to cb046fa Compare August 20, 2026 17:35
@davlgd
davlgd requested a review from medvednikov August 20, 2026 17:36
@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T07:42:34.487242Z 864f8cc Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb046fa4d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +226 to +227
if parsed.path != '' {
c.path = parsed.path

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve escaped path bytes in derived components

When a request path contains a percent-encoded reserved character, such as /files/a%2Fb, urllib.parse exposes /files/a/b through parsed.path and retains the original spelling separately in raw_path. Assigning the decoded value here therefore changes both @path and the subsequently reconstructed @request-target; a compliant peer signing the encoded path will fail verification, and signatures generated here cover a semantically different path.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +174 to +178
existing := h.get_custom(name) or {
h.add_custom(name, addition)!
return
}
h.set_custom(name, existing + ', ' + addition)!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Merge every existing dictionary field line before appending

When a request already carries multiple Signature-Input or Signature field lines, as allowed and explicitly handled by merged_dict_field, get_custom reads only the first value and set_custom clears the remaining same-cased duplicates. A proxy adding its own signature consequently deletes signatures stored on later field lines, breaking the advertised multi-signature forwarding scenario; the existing values need to be joined before the new dictionary member is appended.

Useful? React with 👍 / 👎.

@medvednikov

medvednikov commented Sep 9, 2026

Copy link
Copy Markdown
Member

Fixed both exact-head review findings in 87ba34c97c: preserved percent-encoded bytes for @path and @request-target, and merged every existing Signature dictionary field line before appending. Added focused regressions. Tests: ./vnew -old-compiler -silent vlib/net/http/signature/http_message_test.v; ./vnew -silent test vlib/net/http/signature/ (5/5 passed).

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87ba34c97c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +229 to +232
c.target_uri = if is_origin_form && authority != '' {
'${scheme}://${authority}${req.url}'
} else {
req.url

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Normalize absolute URLs to the transmitted request target

When signing an outbound request whose absolute URL has no path, such as POST https://example.com, this branch signs https://example.com as @target-uri, while net.http.Request.method_and_url_to_response sends / for the empty path. The receiving peer therefore reconstructs https://example.com/ from the origin-form request and cannot verify the default signature; construct the target URI from the parsed scheme and authority plus the same escaped request target that net.http transmits.

Useful? React with 👍 / 👎.

Comment on lines +84 to +88
for c in s {
if c == `\\` || c == `"` {
out << `\\`
}
out << c

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject control bytes in signature parameter strings

When a caller-supplied keyid, nonce, or tag contains CR or LF—for example, when a server incorporates a client-provided nonce—this serializer passes the control byte through because it only escapes backslashes and quotes. http.Header.add_custom validates only the header name, and header rendering writes values verbatim, so the generated Signature-Input value can inject an additional HTTP header line; reject bytes outside the RFC 8941 string character range before serialization.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed both exact-head P1 findings in d272b80b2c: normalized @target-uri from the encoded transmitted request target, including / for empty paths, and rejected invalid RFC 8941 string bytes before Signature-Input serialization. Added focused regressions. Tests: ./vnew -old-compiler -silent vlib/net/http/signature/http_message_test.v; ./vnew -old-compiler -silent vlib/net/http/signature/structured_field_test.v; ./vnew -old-compiler -silent test vlib/net/http/signature/ (5/5 passed).

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d272b80b2c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
} else {
c.path = '/'
}
c.query = if parsed.raw_query != '' { '?' + parsed.raw_query } else { '?' }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Sign the query bytes that net.http transmits

For outbound URLs whose raw query is not already in urllib's canonical form, such as ?q=a%20b, ?flag=, or a trailing empty ?, these components preserve parsed.raw_query. However, the HTTP/1.x send path in vlib/net/http/request.v:243-247 reparses the query and transmits url.query().encode(), which can turn %20 into +, remove =, or omit the empty query marker. The receiver consequently reconstructs different values for the default @target-uri and for @request-target/@query, so an otherwise valid signature cannot verify; derive the signed values from the same request target passed to the transport.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +228 to +229
is_origin_form := req.url.starts_with('/')
request_target := parsed.request_uri()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-origin-form request targets

When an HTTP proxy sends or receives an absolute-form target, req.url is the full URI and that exact URI is the RFC request target, but parsed.request_uri() reduces it to path and query. The proxy send path confirms the absolute URI is placed on the request line in vlib/net/http/http_proxy.v:162-163; CONNECT authority-form targets are affected similarly. Signatures explicitly covering @request-target therefore fail across proxies, so this value must preserve the actual request-line form rather than always converting it to origin form.

Useful? React with 👍 / 👎.

Comment on lines +124 to +128
mut trimmed := []string{cap: values.len}
for v in values {
trimmed << trim_ows(v)
}
return trimmed.join(', ')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match HTTP/1.x serialization for repeated fields

For an outbound HTTP/1.x request with repeated values for a covered field, such as two Accept entries, this produces text/html, application/json. The actual request writer in vlib/net/http/request.v:355-363 instead collapses custom_values with '; ', transmitting text/html; application/json; the receiver then signs/verifies that single semicolon-separated value and cannot match the sender's base. Either fix the HTTP/1.x serializer to preserve/comma-combine repeated fields or canonicalize from the exact value it will transmit.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/key.v Outdated
Comment on lines +82 to +86
pub fn Key.ecdsa_p384_private(x []u8, y []u8, d []u8) Key {
return Key{
algorithm: .ecdsa_p384_sha384
is_private: true
bytes: concat3(x, y, d)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pad scalar inputs in raw ECDSA constructors

When callers pass the minimal-length scalar returned by ecdsa.PrivateKey.bytes()—as the new P-384 roundtrip test does in http_message_test.v:77-78—a key whose scalar starts with zero yields fewer than 48 bytes. This constructor merely concatenates the slices, so ecdsa_sign rejects the resulting key because its total length is not coord_size * 3. The current tree's PEM helper now pads this value, but the fresh evidence is that both public raw private-key constructors still contradict their fixed-width documentation and can fail for valid generated keys; pad each coordinate in the constructors as well.

Useful? React with 👍 / 👎.

Comment on lines +109 to +110
pub fn signature_input_value(label string, p SignatureParams) !string {
return label + '=' + serialize_signature_params(p)!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate labels in public header serializers

When a caller uses this public helper directly with a client-controlled label containing CR or LF, the label is prefixed verbatim even though the function documents its result as ready for an HTTP header. http.Header.add_custom validates only the header name, so attaching the returned value permits header-line injection; the high-level sign path calls check_label, but direct calls to this helper bypass it. Validate the Structured Field key grammar here and in signature_header_value before returning header-ready data.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a9e6535ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/request.v Outdated
val := header.custom_values(key).join('; ')
// RFC 9110 §5.2 combines repeated field lines with a comma. This also
// keeps HTTP/1.x serialization consistent with the HTTP/2 path.
val := header.custom_values(key).map(it.trim_space()).join(', ')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve trailing empty repeated field values

When a covered repeated field ends with an empty value, such as X-Foo: a plus X-Foo:, this joins the values into X-Foo: a, . The HTTP/1 request parser subsequently treats the delimiter's trailing space as OWS, so verification reconstructs a, while the signer covered a, . Fresh evidence beyond the earlier repeated-value trimming issue is that trimming each value does not preserve a terminal empty member; emit separate field lines or otherwise keep the receiver's reconstructed value identical.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/h2_client.v Outdated
Comment on lines 50 to 51
if !header.contains(.content_length) {
extra << H2HeaderField{'content-length', data.len.str()}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid adding Content-Length to bodyless TRACE requests

For an HTTPS TRACE request with no explicit Content-Length, the HTTP/2 path now unconditionally emits content-length: 0. RFC 9110 prohibits clients from sending Content-Length in TRACE requests, so an HTTP/2 server can reject a request that previously carried no such field; retain the body-length guard for TRACE or reject this request shape explicitly.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +150 to +153
if adds_content_length {
// Insert the field so both HTTP/1.x and HTTP/2 actually transmit the
// value covered by the signature.
resp.header.set(.content_length, resp.body.len.str())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Defer response mutation until signing succeeds

When content-length is covered but absent, this mutates the response before sign runs. Any later signing error—for example expires <= created, an invalid nonce byte, a duplicate component, or malformed key material—therefore returns failure while leaving a new Content-Length header behind, so callers that handle the error can unknowingly send a modified unsigned response. Compute the effective component value without mutating, or roll back/apply the header only after signing succeeds.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head field-value findings: HTTP/1 preserves trailing empty repeated values as separate field lines, TRACE no longer gets a synthesized Content-Length in HTTP/1, HTTP/2, or signature reconstruction, and response Content-Length insertion is deferred until signing succeeds. Added round-trip, TRACE, and failure-transaction regressions. Validation: rebuilt ./vnew; focused request-header, HTTP/2 client, and signature suites pass; full vlib/net/http/ suite passes (30 passed, 4 platform skips).

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d9520670e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +116 to +118
if opts.now_unix > 0 {
if exp_v := entry.params['expires'] {
if exp_v is i64 && opts.now_unix >= exp_v {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject signatures created after the verification time

When now_unix is set and a valid signature carries created later than that time, this block checks only expires, so the signature is accepted before its claimed creation time. RFC 9421 verification requires future created values to be rejected (subject to an explicit clock-skew policy); compare created with now_unix whenever time validation is enabled.

Useful? React with 👍 / 👎.

h.delete_custom(key)
}
key := if matching_keys.len > 0 { matching_keys[0] } else { name }
h.set_custom(key, existing_values.join(', ') + ', ' + addition)!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject empty existing signature dictionaries

When a request or response already contains an empty Signature-Input or Signature field, the availability check accepts it because the parsers return an empty collection, and this concatenation produces a value beginning with , (for example, , sig1=(...)). The signing call therefore reports success but leaves a malformed Structured Field that cannot subsequently verify; reject empty existing dictionaries before mutation instead of appending to them.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head validation findings: time-aware verification now rejects signatures whose created value is later than now_unix with a typed SignatureNotYetValid error, and request/response signing rejects empty pre-existing signature dictionaries before mutation. Added future-created and empty-dictionary regressions; documented the expanded time validation. Validation: signature module 5/5 and vcheck-md 0 errors.

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c41ff47b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
}

fn validate_request_component_coverage(req http.Request, components []string, default_scheme string) ! {
if req.disable_connection_reuse && components.any(it.to_lower() == 'connection') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat proxy requests as connection-close transports

When a plain-HTTP request uses req.proxy, leaves disable_connection_reuse at its default, and explicitly covers Connection (for example, keep-alive), this guard allows signing. The proxy paths in http_proxy.v:163-176 call build_request_headers_with, which enables connection_close and appends a second Connection: close at request.v:397-398; the signer therefore covers only keep-alive, while the recipient reconstructs keep-alive, close and rejects the signature. Fresh evidence beyond the earlier direct-request case is that proxied requests take this one-shot serializer regardless of disable_connection_reuse.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +388 to +390
if host := req.header.get(.host) {
if host != '' {
authority = host.trim_space()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize explicit Host before building HTTP/2 authority

When an HTTPS request has an explicit Host value with boundary OWS and negotiates HTTP/2, this line signs the trimmed authority, but to_h2_request still assigns the untrimmed Host directly to :authority at h2_client.v:41-43. The HTTP/1 fallback works because its serializer trims values, whereas the HTTP/2 request carries an invalid leading/trailing-whitespace pseudo-header that conforming peers can reject. Fresh evidence after the earlier outbound Host fix is that its new regression test disables HTTP/2, leaving the negotiated HTTP/2 path inconsistent.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head transport findings: request signing now rejects Connection coverage for proxy transports that append Connection: close, and HTTP/2 trims explicit Host OWS before using it as :authority, matching signature reconstruction. Added proxy-coverage and H2 authority regressions. Validation: rebuilt ./vnew; focused H2 client and signature suites pass; full vlib/net/http/ suite passes (30 passed, 4 platform skips).

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6dd086a12

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ensure_signature_label_available(resp.header, opts.label)!
mut comps := opts.components.clone()
if comps.len == 0 {
comps = ['@status']

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require payload integrity in response defaults

When callers use the high-level defaults for a response with a body, the signature covers only @status, and verify_response likewise requires only @status; an intermediary can therefore replace the body and content headers on a signed 200 response without invalidating verification. Fresh evidence after the earlier component-policy finding is that the newly added response policy still treats status-only coverage as the safe default; require coverage of a caller-supplied content-digest for body-bearing responses or otherwise reject this default.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +389 to +392
if host := req.header.get(.host) {
if host != '' {
authority = host.trim_space()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject empty Host overrides before signing

When HTTP/2 is disabled and an explicit Host value is empty or whitespace-only, this branch keeps the URL authority and sign_request succeeds, but build_request_headers_opts suppresses its synthesized Host because the field is present and emits an empty Host: line. The parsed recipient consequently has no authority and cannot verify the default @authority/@target-uri coverage; reject an empty explicit Host instead of falling back to the URL authority.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head policy findings: default signing of body-bearing responses now requires and covers caller-supplied Content-Digest, default verification requires its coverage, and explicit profiles remain available; request signing now rejects an empty/OWS-only explicit Host instead of signing a URL authority that will not be sent. Added signing/verification policy and empty-Host regressions; documented digest validation responsibility. Validation: signature module 5/5 and vcheck-md 0 errors.

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed081b12f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +51 to +54
mut comps := opts.components.clone()
if comps.len == 0 {
comps = default_request_components(req)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require payload integrity in request defaults

When req.data is nonempty and callers omit components, the default profile covers only the method and destination (plus Date), and verify_request requires the same profile. An intermediary can therefore replace a POST/PUT body—using a same-length payload or adjusting the unsigned Content-Length—and the default verification still succeeds. Mirror the response behavior by requiring and covering a Content-Digest for request bodies.

Useful? React with 👍 / 👎.

Comment thread vlib/net/http/http_proxy.v Outdated
Comment on lines 163 to 164
s := req.build_request_headers_with(method, host_name, port, default_port,
'${host.scheme}://${host_name}${port_part}${path}', data, header)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve IPv6 brackets in proxy request targets

For a plain-HTTP request through a proxy whose destination is an IPv6 literal, host_name comes from URL.hostname() without brackets, so this emits an absolute-form target such as http://2001:db8::1/path. Signature extraction uses the bracket-preserving transport_authority, however, and signs http://[2001:db8::1]/path, so the default @target-uri signature cannot verify and the proxy target itself is malformed. Fresh evidence after the earlier IPv6 fixes is that this proxy-specific absolute-form path still bypasses authority_host.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head request findings: default signing of body-bearing requests now requires and covers caller-supplied Content-Digest, default verification requires its coverage, and explicit profiles remain available; proxy absolute-form targets now preserve IPv6 brackets through a tested formatter. Updated policy docs. Validation: rebuilt ./vnew; proxy test and signature module pass; full vlib/net/http/ suite passes (30 passed, 4 platform skips); vcheck-md reports 0 errors.

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d04e9dfe0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/signature/http_message.v Outdated
Comment on lines +525 to +526
} else {
500

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject nonzero statuses instead of signing a different code

After the earlier zero/unassigned status fixes, a response whose nonzero status_code falls outside 100–599 still gets signed as 500, while Response.bytes() serializes the original value and the HTTP/2 sender likewise transmits every nonzero value unchanged (h2_server.v:924-926). Thus, for example, sign_response reports success for status 700 but the emitted @status is not the value covered by the signature. Either reject these response states during signing or normalize the actual response consistently before producing the signature.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head status finding: response component extraction now rejects every nonzero status outside 100–599 for both signing and verification rather than signing a synthesized 500 while transports emit the original value. Added a no-mutation regression for status 700. Validation: signature module 5/5.

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70886ce1a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vlib/net/http/request.v
sb.write_string('\r\n')
}
if !header.contains(.content_length) {
if method != .trace && !header.contains(.content_length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject unframed TRACE request bodies

When a TRACE request has nonempty data and no explicit Content-Length, this guard suppresses the only framing header while the serializer still appends data after the header block. On the pooled HTTP/1 path, the server treats the TRACE as bodyless and can parse those leftover bytes as the next request on the reused connection; reject body-bearing TRACE requests rather than emitting an unframed body.

Useful? React with 👍 / 👎.

Comment on lines +292 to +294
fn (mut p SfParser) skip_sp() {
for p.pos < p.src.len && (p.src[p.pos] == ` ` || p.src[p.pos] == `\t`) {
p.pos++

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject HTAB around Structured Field dictionary members

When either signature field contains HTAB at a dictionary boundary, such as sig1=:(...):,\tsig2=:(...):, both dictionary parsers consume it here and can authenticate a value that conforming RFC 8941 parsers reject, because dictionary whitespace permits SP rather than HTAB. Fresh evidence beyond the earlier inner-list finding is that inner-list padding now has an SP-only helper, but the outer Signature-Input and Signature loops still call this broader helper.

Useful? React with 👍 / 👎.

@medvednikov

Copy link
Copy Markdown
Member

Fixed the exact-head protocol findings: HTTP request-header construction now returns an error for body-bearing TRACE across direct, pooled, TLS, SChannel, and proxy paths; request signing rejects the shape before mutation. Structured Field dictionary boundaries now permit SP only and reject HTAB. Added serializer/signing and both-dictionary regressions. Validation: rebuilt ./vnew; request-header test and signature module pass; full vlib/net/http/ suite passes (30 passed, 4 platform skips).

@medvednikov

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 864f8cc77c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +125 to +128
} else if req.data != '' {
['@method', '@target-uri', '@authority', 'content-digest']
} else {
['@method', '@target-uri', '@authority']

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require digest coverage independently of the received body

When a valid explicitly configured signature omits content-digest on a request with a body, an attacker can remove the entire body and alter or remove its unsigned framing; req.data is then empty, so this branch drops the digest requirement and accepts the same signature even though the unmodified request would be rejected. Fresh evidence after the earlier request-default finding is that the new policy is selected from attacker-controlled received body length; use an application-declared body expectation or a stable method/profile policy instead. The equivalent resp.body != '' branch in verify_response has the same downgrade.

Useful? React with 👍 / 👎.

Comment on lines +195 to +196
if adds_content_length {
resp.header.set(.content_length, resp.body.len.str())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject forbidden Content-Length synthesis

When sign_response covers content-length on a 1xx or 204 response without that field, this inserts Content-Length: 0, which RFC 9110 prohibits servers from sending for those status codes. Because both the HTTP/1 serializer and HTTP/2 sender emit the inserted field, signing an otherwise valid no-content response makes the wire response malformed; reject this coverage for those statuses instead of synthesizing the field.

Useful? React with 👍 / 👎.

Comment on lines +24 to +26
signature.sign_request(mut req, priv,
components: ['@method', '@target-uri', '@authority', 'date', 'content-type']
)!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make the quick-start signing profile verifiable

The quick start creates a POST with the nonempty body {} but signs an explicit component list without content-digest; the subsequent default verify_request therefore selects the body-bearing policy and returns signature does not cover required component "content-digest". Add and cover a valid Content-Digest, use an empty-body request, or pass a matching explicit verification policy so the documented quick start actually completes.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants