Skip to content

Upgrade x509-parser to 0.16 #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 1, 2024
Merged

Upgrade x509-parser to 0.16 #231

merged 3 commits into from
Mar 1, 2024
+52 −146

Conversation

djc
Copy link
Member

@djc djc commented Feb 29, 2024

No description provided.

@djc
Upgrade x509-parser to 0.16

Verified

This commit was signed with the committer’s verified signature.
Josh-Cena Joshua Chen
GPG key ID: C37145B818BDB68F
Verified
Learn about vigilant mode
b1f1398
@djc djc requested review from cpu and est31 February 29, 2024 12:17
@Rigidity
Copy link

Nice, this should fix support for compiling on ARM Windows I believe - thanks!

cpu
cpu approved these changes Feb 29, 2024
View reviewed changes
Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

There are a couple of TODO's in unit tests we could knock out now, but I think it would be reasonable to do that in a separate PR. I can take a look later:

rcgen/rcgen/tests/generic.rs

Lines 174 to 176 in 3c3e984

// TODO: Waiting on x509-parser 0.15.1 to be released.
// let crl_number = BigUint::from_bytes_be(crl.get_params().crl_number.as_ref());
// assert_eq!(x509_crl.crl_number().unwrap(), &crl_number);

rcgen/rcgen/tests/generic.rs

Line 196 in 3c3e984

// TODO: x509-parser does not yet parse the CRL issuing DP extension for further examination.

@cpu
tests: assert correct x509-parser CRL number

Verified

This commit was signed with the committer’s verified signature.
Josh-Cena Joshua Chen
GPG key ID: C37145B818BDB68F
Verified
Learn about vigilant mode
baa0975 
An upstream bug in x509-parser that prevented us from asserting the
rcgen generated CRL had the correct CRL number has been fixed, so we can
now update our unit test to assert the expected value.
@cpu
Copy link
Member

cpu commented Feb 29, 2024

@djc I have commits fixing up those two test TODOs. Are you OK with me pushing those into this branch or would you prefer I do it separately after this merges?

@cpu
tests: check x509-parser CRL IDP ext matches expected
85b69db 
The upstream x509-parser crate now has support for representing CRL
issuing distribution point extensions. This commit extends the existing
x509-parser CRL unit test to assert various properties of the parsed CRL
IDP extension match expectations.
@djc
Copy link
Member Author

djc commented Feb 29, 2024

Sure, you have the baton!

@cpu
Copy link
Member

cpu commented Feb 29, 2024

Sure, you have the baton!

Pushed my changes 🚀

@djc
Copy link
Member Author

djc commented Feb 29, 2024

✅ Test changes LGTM.

est31
est31 approved these changes Mar 1, 2024
View reviewed changes
Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

great work

@est31 est31 added this pull request to the merge queue Mar 1, 2024
Merged via the queue into main with commit a1b9cc2 Mar 1, 2024
@est31 est31 deleted the x509-parser-0.16 branch March 1, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpu cpu

@est31 est31

@Rigidity Rigidity

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@djc @Rigidity @cpu @est31