letsencrypt / letsencrypt/pebble

ARI replaces lookup misses when the certificate serial's top byte is >= 0x80

Open
#550 1 comment 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
787
Forks
176
PR merge metrics
No merged PRs in 30d

Description

Pebble stores issued serials in one encoding and looks them up in another when validating an ARI replaces field, so a newOrder that carries a spec-compliant certID gets rejected with a 500 whenever the serial's most significant byte is >= 0x80.

Tested against v2.10.1, and main (200b05c) still has both call sites unchanged.

What happens
POST /order-plz
{"identifiers": [{"type":"dns","value":"*.example.com"}],
 "replaces": "RTAINB8QAcdWz54id2aMkYdCnDI.AIWf94paRX4"}

HTTP 500
{
  "type": "urn:ietf:params:acme:error:serverInternal",
  "detail": "could not find an order for the given certificate: could not find order resulting in the given certificate serial number",
  "status": 500
}

The certificate is there, it was issued by this same Pebble process minutes earlier, and the same certID works fine against renewalInfo.

AIWf94paRX4 is base64url for 00 85 9F F7 8A 5A 45 7E, i.e. the serial 859FF78A5A457E with the DER sign byte in front. RFC 9773 section 4.1 asks for the DER encoded value bytes of serialNumber, and DER prepends 0x00 when the first byte has the high bit set, so a client that produces that byte is doing the right thing. Boulder accepts it.

Where the two encodings diverge

Stored key, from big.Int.Bytes(), which is the minimal magnitude with no sign byte:

// ca/ca.go:166
hexSerial := hex.EncodeToString(cert.SerialNumber.Bytes())

Lookup key, from the raw base64url decoded bytes, sign byte included:

// core/types.go:232 (NewCertID)
id: hex.EncodeToString(serial),

validateReplacementOrder then passes that through CertID.SerialHex():

// wfe/wfe.go:1695
originalOrder, err := wfe.db.GetOrderByIssuedSerial(certID.SerialHex())

so the map in db/memorystore.go is keyed on 859ff78a5a457e and queried for 00859ff78a5a457e, and misses.

What makes me fairly confident this is unintentional rather than a deliberate strictness choice: the other consumer of the same parsed certID normalizes correctly, because it uses the *big.Int rather than the hex string.

// wfe/wfe.go:1931 (RenewalInfo)
cert := wfe.db.GetCertificateBySerial(certID.SerialNumber)

That is also why this is easy to miss in practice. The ARI GET succeeds, the client gets a renewal window back and happily proceeds, and only the newOrder that follows blows up.

How often

makeSerial draws from [0, math.MaxInt64). An 8 byte magnitude can never have a top byte >= 0x80, since that would need a value of at least 2^63, so this is driven by the 7 byte case and works out at about 2^55/2^63, or 1 in 256. That is rare enough to look like an intermittent client bug in CI, which is how I ended up here.

Reproducing it

Since the natural rate is 1 in 256, the quickest way is to force the condition in ca/ca.go:

func makeSerial() *big.Int {
	serial, err := rand.Int(rand.Reader, big.NewInt(1<<55))
	if err != nil {
		panic(fmt.Sprintf("unable to create random serial number: %s", err.Error()))
	}
	serial.Or(serial, new(big.Int).Lsh(big.NewInt(1), 55)) // [2^55, 2^56): 7 bytes, top byte >= 0x80
	return serial
}

Then issue a certificate with any ARI aware client and renew it. With acme.sh (which adds the sign byte per RFC 9773) I get a clean A/B:

serial=6D569DBC36BFE7  top byte < 0x80   renew rc=0  "Cert success."
serial=859FF78A5A457E  top byte >= 0x80  renew rc=1  "could not find an order for the given certificate"

Dropping the Or above, so the top byte lands under 0x80, makes it pass every time.

Possible fix

Normalizing in NewCertID looks like the smallest change, something like keying id off new(big.Int).SetBytes(serial).Bytes() so both sides agree on the minimal encoding. That also keeps SerialHex consistent with what GetCertificateBySerial already does. Happy to send a PR if that is the direction you would want, or if you would rather have the fix on the db side I can do that instead.

One side note while you are in there: the resulting problem document is serverInternal, and at least one client only retries without replaces when the response mentions malformed, alreadyReplaced, replaces or ARI. So the mismatch is currently fatal rather than degrading to a plain order. A malformed problem here would be friendlier regardless of whether the encoding is fixed.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with core/types.go at NewCertID, then trace wfe/wfe.go through validateReplacementOrder and RenewalInfo, comparing their serial handling. Reproduce the issue by forcing the high-bit serial range in ca/ca.go; done means a DER sign-byte certID finds the issued order and the existing renewal path remains successful.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.