fix(publisher): pad ECDSA P-384 signature to fixed width

[JosephDoUrden]

The in-process ECDSA P-384 signer built the signature with append(r.Bytes(), s.Bytes()...). big.Int.Bytes() strips leading zero bytes, so whenever r or s had a high zero byte (roughly 1 in 128 signatures) the concatenation came out shorter than 96 bytes and the registry rejected the publish with "invalid signature size for ECDSA P-384". Intermittent and confusing to debug. Fixed by left-padding each component to 48 bytes with FillBytes, same as the googlekms signer already does. Added a test pinning the fixed-width encoding plus a sign/verify round-trip.

[JosephDoUrden]

Green and mergeable since 16 July with no review yet, so flagging it in case it slipped past.

The failure mode is intermittent, which is what makes it awkward to diagnose: big.Int.Bytes() strips leading zero bytes, so whenever r or s has a high zero byte — roughly 1 signature in 128 — append(r.Bytes(), s.Bytes()...) yields fewer than 96 bytes and the publish is rejected with invalid signature size for ECDSA P-384. The googlekms path already assembles a fixed-width buffer; this makes the in-process signer behave the same way, with a test pinning the encoding plus a sign/verify round-trip.

Base is 5 commits behind main — happy to rebase if that helps. @rdimitrov

[UgaTheDev]

This is a real bug and the fix is correct. I reproduced the failure rate
empirically and confirmed the new encoding matches what the verifier expects, so
here are the numbers in case they help this get merged.

Reproduction

Signed 30,000 random messages with a freshly generated P-384 key, encoded each
one both ways, and ran both through the actual server-side verifier
(PublicKeyInfo.VerifySignature from internal/api/handlers/v0/auth/common.go)
rather than a stand-in:

signatures generated:        30000
r short (leading zero byte):    135  (0.450%, 1 in 222)
s short (leading zero byte):    100  (0.333%, 1 in 300)
both short:                       1  (0.0033%)
OLD encoding rejected:          234  (0.780%, 1 in 128)
NEW encoding rejected:            0  (0.000%)

Which matches the analysis in your comment exactly: each component drops its
leading byte about 1 time in 256, and since either one is enough to break the
length, the observed publish failure rate is ~1 in 128. Zero failures out of
30,000 with FillBytes.

Both existing test suites pass on the head commit:

$ go test ./cmd/publisher/auth/ -v -run 'ECDSA|InProcess'
--- PASS: TestEncodeECDSAP384Signature_FixedWidth (0.00s)
--- PASS: TestInProcessSigner_ECDSAP384_SignatureVerifies (0.00s)

$ go test ./cmd/publisher/auth/... ./internal/api/handlers/v0/auth/...
ok  	github.com/modelcontextprotocol/registry/cmd/publisher/auth	1.962s
ok  	github.com/modelcontextprotocol/registry/internal/api/handlers/v0/auth	1.445s

Verifier assumption confirmed

The fixed-width encoding is not just one valid choice here — it is the only one
the verifier accepts. internal/api/handlers/v0/auth/common.go:199-203:

if len(signature) != 96 {
    return fmt.Errorf("invalid signature size for ECDSA P-384")
}
r := new(big.Int).SetBytes(signature[:48])
s := new(big.Int).SetBytes(signature[48:])

A hard length check plus a split at a fixed offset, so the producer has to
left-pad. Your change makes the in-process signer agree with that.

Worth noting for anyone reading this later: the old encoding could only ever
produce a clean rejection, never a silent mis-verification. A short r shifts
bytes left, but the total then falls under 96 and trips the length check first —
and s can never exceed 48 bytes, so there is no combination that lands on 96
with a wrong split. That makes this a publish-reliability bug rather than a
security one, which is the right way to describe it in the PR body.

I also checked the sibling signers for the same pattern.
cmd/publisher/auth/googlekms/common.go:41-60 already left-pads correctly in
derToRS, and the Azure path uses azkeys.SignatureAlgorithmES384, which
returns fixed-width r||s from the service. So the in-process signer was the
only one affected — no follow-up needed elsewhere.

Two small notes, neither blocking

FillBytes panics on overflow, and that is fine here — but it is worth a
word. FillBytes panics if the value does not fit in the destination. For
P-384, r and s are always below the group order and therefore always fit in
48 bytes, and they come straight from ecdsa.Sign in the same function, so the
panic is unreachable. The contrast with derToRS is instructive: that one
explicitly errors on oversize components because its input is DER from an
external KMS, where a malformed response is possible. Might be worth one clause
in your comment noting the values are locally generated and bounded, so a future
reader does not "fix" it by adding an error return that can never fire.

Magic numbers. 96 and 48 are now spelled literally in three places: this
function, the verifier, and the test. googlekms instead derives its width with
size := (curve.Params().BitSize + 7) / 8. I would not restructure this PR for
it — the function name says P-384 and the scope is small — but if you wanted one
more line of insurance, p384SigSize = 96 / p384CompSize = 48 constants shared
with the verifier would make the two sides visibly agree. Entirely your call;
the current version is correct as written.

Nice find on an intermittent bug — the ~1-in-128 rate is exactly the kind that
gets written off as a flake instead of fixed.

[JosephDoUrden]

Thanks for running the numbers, 234 out of 30000 landing right on the 1 in 128 estimate is satisfying to see. Agree with the framing too, publish reliability bug, not security, the hard length check means a short r can only ever fail clean. I'll add a short comment saying r and s are locally generated and bounded so FillBytes can't panic, and I'll rebase onto main so it's fresh. Keeping 96/48 as literals for now to keep the diff small.

[JosephDoUrden]

Done, brought the branch up to date with main and added the FillBytes bounds comment.

[UgaTheDev]

Checked the updated branch (132878a) — both follow-ups look right to me and I have nothing further.

The bounds comment is accurate as written: r and s do come from the ecdsa.Sign call in this same file and are reduced mod the P-384 group order, so each fits in 48 bytes and neither FillBytes call can panic. Keeping 96/48 as literals is fine by me; the doc comment above the function already explains where the numbers come from, which was the only reason I raised it.

Nothing blocking from my side. CI is green and this is ready for a maintainer whenever one has a moment.