Repository navigation
const-oid: fix (and simplify) base 128 encoder - #1600
Merged
Merged
Conversation
This changes the base 128 decoder to calculate the length of a base 128-encoded arc and then iterates over each byte, computing the value for that byte, without any mutable state other than the position. It also refactors the unit tests and adds an example extracted from proptest failures. The new implementation passes that test.
tarcieri
force-pushed
the
const-oid/fix-base128-encoder
branch
from
November 2, 2024 23:13
94144bf to
11a2bbd
Compare
Merged
This was referenced Oct 25, 2026
paolo-de-rosa
added a commit
to cyberverso-net/sigillum
that referenced
this pull request
Oct 25, 2026
## Summary Adds ADR-0024, "Bounded decoding of untrusted input", with rules for the library crates before their first parser is written. - **Size limits and fuzzing.** Every public decoding entry point has a documented, caller-configurable input size limit and a fuzz target. - **Length walk.** An allocation-free walk, written from ITU-T X.690 (02/2021) only, checks every declared length before any allocating decode, including DER encapsulated in OCTET STRING and BIT STRING. The existing `check_der` of `sigillum-testpki` becomes a small public module of `sigillum-pki` with the first library parser. A separate published crate was rejected: another name to protect (threat S5), release coordination, and another public API to keep stable. - **Object identifiers.** An untrusted object identifier, in DER or in dotted text, gives INDETERMINATE with "not supported" when it fails the canonical syntax, the limit of 2^28 on arcs, parsing by `const-oid` or a round-trip check. Dotted text is encoded to DER by an encoder of our own, so the textual parser of `const-oid` 0.9.6 is never used on untrusted input. The restriction is recorded as divergence DIV-2 (class 3 under ADR-0019). - **Links.** Threat P2, ADR-0011, the design overview in `architecture.md` and review checklist item 3 refer to the ADR. The defects behind the rules are those of `der` 0.7.10 (allocation of a declared length before checking it) and `const-oid` 0.9.6 (object identifier arcs), all public upstream: [RustCrypto/formats#1520](RustCrypto/formats#1520), [#1585](RustCrypto/formats#1585), [#1522](RustCrypto/formats#1522), [#1592](RustCrypto/formats#1592), [#1600](RustCrypto/formats#1600), [#1753](RustCrypto/formats#1753), [#2412](RustCrypto/formats#2412). The migration to the `der` 0.8 generation (#21) checks them. **ADR status.** ADR-0024 is accepted directly, without a *Proposed* stage: the maintainer decided its rules in #78 and in the review of this change. The sentence added to accepted ADR-0011 is a cross-reference only, not a change of substance. **Numbering.** ADR-0023 is added by #75, which merges after this pull request. Closes #78 ## Definition of Done - [x] Every acceptance criterion is met and covered by a test (documentation only) - [x] Quality gates pass, including coverage, mutation and fuzzing thresholds where they apply ([quality gates](https://github.com/cyberverso-net/sigillum/blob/main/docs/development/quality-gates.md)) - [x] Reviewed against the review checklist - [x] Implemented from the specifications; no code or test data under incompatible licences, as required by the provenance rule in [CONTRIBUTING.md](https://github.com/cyberverso-net/sigillum/blob/main/CONTRIBUTING.md) - [x] Documentation and rustdoc updated - [x] ADR added for any significant decision - [x] Intentional divergences from DSS recorded in `docs/design/divergences.md` - [x] Changelog fragment added under `changelog.d/`, or not needed (reason given above) - [x] Commits signed off - [x] Maven dependency licences checked against `tools/dss-oracle/README.md`, or `tools/dss-oracle/pom.xml` unchanged ## Quality report Documentation only. All 143 links in the changed files resolve. Clauses checked against ITU-T X.660 (07/2011), X.680 (02/2021) and X.690 (02/2021) from itu.int, and RFC 5280 from rfc-editor.org. Line references checked against the sources of `der` 0.7.10 and `const-oid` 0.9.6. Signed-off-by: Paolo De Rosa <paolo.de.rosa@linux.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changes the base 128 decoder to calculate the length of a base 128-encoded arc and then iterates over each byte, computing the value for that byte, without any mutable state other than the position.
It also refactors the unit tests and adds an example extracted from proptest failures. The new implementation passes that test.
Closes #1585