Repository navigation
Add unchecked DER certificate setters - #545
Merged
Merged
Conversation
`SslRef::set_certificate_der_unchecked` and `SslContextBuilder::set_certificate_der_unchecked` allow callers to preserve certificate DER that cannot be represented by a parsed `X509Ref`. The impl of `AsRef<Asn1StringRef>` for `Asn1TimeRef` lets callers inspect encoded time bytes without direct FFI. Documentation notes that the unchecked setters bypass full X.509 parsing, and tests cover both setters.
There was a problem hiding this comment.
✅ Approved — No issues found: the FFI wrappers match BoringSSL's argument orders, the time-to-string cast is sound, and the tests cover both setters.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
📋 Code Quality: ✅ LGTM
No code issues. Both FFI wrappers pass the DER pointer and length in the order BoringSSL expects, which is different for the two functions. The AsRef cast is sound because ASN1_TIME is a typedef of ASN1_STRING.
📝 Documentation: ✅ LGTM
The doc comments accurately describe the limited validation and steer callers toward X509::from_der plus set_certificate for normal use.
Reviewed commit: 50e59235
kornelski
approved these changes
Oct 30, 2026
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.
SslRef::set_certificate_der_uncheckedandSslContextBuilder::set_certificate_der_uncheckedwrapSSL_use_certificate_ASN1andSSL_CTX_use_certificate_ASN1. Their names make explicit that they do not fully parse X.509 fields, while BoringSSL copies the input DER and validates enough structure to extract a supported public key.This supports negative handshake tests that need to send certificate DER which
X509::from_derrejects.The impl of
AsRef<Asn1StringRef>forAsn1TimeReflets callers inspect encoded time bytes without directboring-syscalls.Tests cover both setters, malformed certificate DER, and the time-ref conversion.