Repository navigation
const-oid: accept the RFC 5280 Appendix B worst case in the default capacity - #2497
Closed
choksi2212 wants to merge 1 commit into
Closed
choksi2212 wants to merge 1 commit into
choksi2212 wants to merge 1 commit into
Conversation
…apacity DEFAULT_MAX_SIZE was 39 so the struct came out a round 40 bytes total, but RFC 5280 Appendix B requires implementations to accept OIDs with at most 20 arcs, every arc below 2^28, whose dotted-decimal string is at most 100 bytes. The 100-byte string is the binding limit: after the leading two arcs (one octet), each 4-octet arc costs at least 8 string bytes to spell, so a 100-byte string needs at most 1 + 12 * 4 = 49 contents octets - reached by twelve arcs of 9999999 below 1.2, which is exactly what the new worst-case test encodes. Raise the default to 49 so every Appendix B-conformant OID fits, fix the validity docs (the encoding may be exactly MAX_SIZE long, not shorter), and pin the reported 100-byte/40-octet OID, the 99-byte/49-octet ceiling, and a custom 39 capacity still rejecting what it cannot hold. der and x509-cert take the capacity from the default, so certificates carrying such OIDs as extension ids decode again (RustCrypto#2485). Note: ObjectIdentifier grows from 40 to 50 bytes total, so the release will need a minor version bump rather than a patch. Signed-off-by: choksi2212 <choksi2212@users.noreply.github.com>
Member
|
This is a breaking change we can't make until the next breaking release of |
Contributor
Author
|
Fair enough — bumping If it is useful I can send the two doc-only bits as a standalone non-breaking PR (the "shorter than" -> "no longer than" correction on the validity note, plus the RFC 5280 Appendix B derivation as a comment on |
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.
Closes #2485.
Root cause
DEFAULT_MAX_SIZE = 39was chosen soObjectIdentifiercomes out a round 40 bytes total (the comment it replaced said exactly that), but RFC 5280 Appendix B requires implementations to accept OIDs with at most 20 arcs, each arc below 2^28, whose dotted-decimal string is at most 100 bytes. OIDs inside those limits can need more than 39 contents octets, sonewandfrom_bytesreject them withError::Length, and sincederdecodesOBJECT IDENTIFIERinto the default-sized buffer, a certificate whose extension carries such an OID failsfrom_dereven when the extension is non-critical.Why the ceiling is 49, not 40
The 40-octet OID in #2485 is not the largest encoding the RFC forces; it is the largest for nine 9-digit arcs. A 100-byte dotted string can spend its budget on cheaper arcs: after the leading two arcs (always a single octet, since the second arc is 0..=39), a 4-octet arc costs at least 8 string bytes to spell (4-octet arcs start at 2097152, seven digits), so the 96 remaining bytes buy at most twelve of them: 1 + 12 * 4 = 49 contents octets, reached by twelve arcs of 9999999 below 1.2, a 99-byte string. Anything longer needs a dotted string over 100 bytes, so 49 is the ceiling, and that is what the default becomes.
Change
const-oid:DEFAULT_MAX_SIZE39 -> 49, with the derivation in the doc comment.ObjectIdentifiergrows from 40 to 50 bytes total, so the release wants a minor version bump; I left versioning to the release process.ObjectIdentifier::MAX_SIZE, but the encoder accepts exactlyMAX_SIZEoctets (which the new 49-octet worst case now exercises), so the bullet now says no longer than.derandx509-certneed no changes of their own: they take the capacity from theObjectIdentifierdefault.Tests (red on 39, green on 49)
rfc5280_appendix_b_worst_case_fits: the 99-byte/49-octet ceiling throughnew,from_bytes, and back.rfc5280_appendix_b_100_byte_40_octet_oid_fits: the exact OID from x509-cert/const-oid: a 100-character OID (RFC 5280 Appendix B) needs 40 octets, so a certificate carrying one as a non-critical extension id does not decode #2485, a 100-byte string encoding to 40 octets.explicit_smaller_capacity_still_rejects: the same 49 bytes parse by default, butObjectIdentifier::<39>::from_bytes_sizedstill rejects them, so custom capacities keep their meaning.Verification
cargo test -p const-oid --all-features: 22/22 integration (including the 3 new), 8 unit, 2 oid_ref, 1 proptest, 2 doctests, all green.src/lib.rsreverted to the 39-octet capacity, both Appendix B tests panic atnew(...).unwrap()with theErr(Length)from the report; restored, everything is green.cargo test -p der --all-features: green (this is the decoder path certificates go through).cargo test -p x509-cert --all-features: identical results with and without this change; the 5 builder tests that fail need the externalzlintbinary this environment lacks and fail the same way on pristine master.cargo check --workspace --all-features: clean.cargo clippy -p const-oid -p der -p x509-cert --all-features --all-targets -- -D warnings: clean;cargo fmt --all --check: clean.