Skip to content

Add unchecked DER certificate setters - #545

Merged
kornelski merged 1 commit into
masterfrom
nox/unchecked-certificate-der
Oct 30, 2026
Merged

kornelski merged 1 commit into
masterfrom
nox/unchecked-certificate-der

Conversation

@nox

@nox nox commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

SslRef::set_certificate_der_unchecked and SslContextBuilder::set_certificate_der_unchecked wrap SSL_use_certificate_ASN1 and SSL_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_der rejects.

The impl of AsRef<Asn1StringRef> for Asn1TimeRef lets callers inspect encoded time bytes without direct boring-sys calls.

Tests cover both setters, malformed certificate DER, and the time-ref conversion.

`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.

@clanky-the-clanker clanky-the-clanker Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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
kornelski merged commit fa5efdc into master Oct 30, 2026
29 checks passed
@nox
nox deleted the nox/unchecked-certificate-der branch October 30, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants