You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
"body": "\nSection \"Issuance Logs\" describes an issuance log as an append-only sequence of entries identified by an index starting from zero, \nwith no stated upper bound. The only explicitly bounded quantity in this area is the log number itself:\n\n> \"Log numbers are numbered consecutively from 1 to at most 65535 (2^16-1).\"\n\nHowever, the certificate format depends on the log entry index and subtree boundaries fitting in 48 bits.\nSection \"Certificate Format\" defines:\n\n> \"The `serialNumber` MUST be equal to `(log_number << 48) | index`.\"\n\nand `MTCProof`'s `start` and `end` fields are typed as `uint48`. Verification reconstructs the split the same way:\n\n> \"Let `index` be the least significant 48 bits of `serial` and let `log_number` be `serial >> 48`.\"\n\nNone of this is tied to a normative bound on `index` or the log's `tree_size`. \nThere does not appear to be a statement requiring an issuance log to remain small enough for all certificate-relevant values (`index`, `start`, and `end`) to fit in `uint48`.\n\nIf a log grew beyond the range representable by the certificate fields, the certificate format would no longer have a well-defined way to represent later entry indexes in `serialNumber` or subtree boundaries in `MTCProof.start` / `MTCProof.end`. \nThis is the same kind of asymmetry that is already handled for log numbers, which are explicitly bounded at 65535; `index` / `tree_size` do not seem to have the corresponding statement.\n\n**Suggested fix:** Add an explicit requirement defining the maximum allowed issuance log tree size, chosen so that all certificate-relevant indexes and subtree boundaries fit in `uint48`.\n",
3728
+
"createdAt": "2026-06-25T22:00:24Z",
3729
+
"updatedAt": "2026-06-25T22:18:43Z",
3730
+
"closedAt": null,
3731
+
"comments": [
3732
+
{
3733
+
"author": "davidben",
3734
+
"authorAssociation": "COLLABORATOR",
3735
+
"body": "Coincidentally did this in https://github.com/ietf-plants-wg/merkle-tree-certs/pull/261 before I saw this bug.\n\n(This is, of course, not *really* a concern. Even if you issued 1,000 certificates a second, it would still take you almost 9,000 years to reach 2^48.)",
3736
+
"createdAt": "2026-06-25T22:18:43Z",
3737
+
"updatedAt": "2026-06-25T22:18:43Z"
3738
+
}
3739
+
]
3716
3740
}
3717
3741
],
3718
3742
"pulls": [
@@ -18380,6 +18404,130 @@
18380
18404
"comments": []
18381
18405
}
18382
18406
]
18407
+
},
18408
+
{
18409
+
"number": 261,
18410
+
"id": "PR_kwDOJIBkVc7qwsMI",
18411
+
"title": "Add a maxSerial field to the CA format",
"body": "I suspect the discussion around revoking, log numbers, min/max indices, the CA format, etc., can be refactored given how scattered it has become. Ah well.\r\n\r\n(Also closes #260)",
"body": "This reads slightly better to me, but either is fine\n```suggestion\nEach issuance log describes an append-only sequence of *entries* ({{log-entries}}). Each entry is identified by an integer *index*, assigned consecutively starting from zero. Indices are at most 2<sup>48</sup>-1. Each entry is an assertion that the CA has certified. The entries in the issuance log are represented as a Merkle Tree, described in {{Section 2.1 of !RFC9162}}.\n```",
18448
+
"createdAt": "2026-06-26T00:15:40Z",
18449
+
"updatedAt": "2026-06-26T00:30:59Z"
18450
+
}
18451
+
]
18452
+
}
18453
+
]
18454
+
},
18455
+
{
18456
+
"number": 262,
18457
+
"id": "PR_kwDOJIBkVc7rXkgq",
18458
+
"title": "demo: fix UnusedBit panic on signing error",
"body": "Fixes a panic in `CreateCertificate` when `CertificateConfig.UnusedBit` is set and a cosigner fails to sign.\r\n\r\nThe existing condition had an operator-precedence bug:\r\n\r\n```go\r\nif sig, err := certSig.Bytes(); err == nil && len(sig) == 0 || sig[len(sig)-1]&1 != 0 {\r\n```\r\n\r\nSince `&&` binds tighter than `||`, the final `sig[len(sig)-1]` expression could still be evaluated when `certSig.Bytes()` returned an error. In that case, `sig` may be nil or empty, causing a panic instead of allowing the existing builder error to propagate through the normal `Bytes()` path.\r\n\r\nThis change only inspects the signature bytes when `certSig.Bytes()` succeeds.\r\n\r\nA regression test covers the case with `UnusedBit: true` and a `crypto.Signer` whose `Sign` method always returns an error.\r\n\r\nTested:\r\n\r\n```text\r\ngo test -run TestCreateCertificateUnusedBitCosignerError -v\r\ngo test ./... -v\r\n```\r\n",
"body": "Any reason this changed the structure to pull the assignment out of the `if`?",
18495
+
"createdAt": "2026-06-27T23:12:39Z",
18496
+
"updatedAt": "2026-06-27T23:31:59Z"
18497
+
},
18498
+
{
18499
+
"originalPosition": 80,
18500
+
"body": "What's this for? Go's test framework will already catch panics.",
18501
+
"createdAt": "2026-06-27T23:13:03Z",
18502
+
"updatedAt": "2026-06-27T23:31:59Z"
18503
+
},
18504
+
{
18505
+
"originalPosition": 1,
18506
+
"body": "TBH I'm not sure how much it's worth bothering with a test here. It seems mostly scar tissue. At the least, let's put this in encode_test.go.",
18507
+
"createdAt": "2026-06-27T23:22:59Z",
18508
+
"updatedAt": "2026-06-27T23:31:59Z"
18509
+
},
18510
+
{
18511
+
"originalPosition": 27,
18512
+
"body": "Same ID as the issuer is fine. We expect to have at least one cosigner that's the same name as the issuer anyway.",
18513
+
"createdAt": "2026-06-27T23:26:16Z",
18514
+
"updatedAt": "2026-06-27T23:31:59Z"
18515
+
},
18516
+
{
18517
+
"originalPosition": 40,
18518
+
"body": "I suspect this test does not *actually* need you to put anything here, but if you put this in encode_test.go, it can at least share a constant.",
18519
+
"createdAt": "2026-06-27T23:26:45Z",
18520
+
"updatedAt": "2026-06-27T23:31:59Z"
18521
+
},
18522
+
{
18523
+
"originalPosition": 69,
18524
+
"body": "Hmm, I am not thrilled about breaking the `Cosigner` abstraction here. It's something we'll have to remember to update every time we update that struct.\n\nInstead of all this, I think you could get the same coverage by just passing an invalid subtree to CreateCertificate.",
0 commit comments