Skip to content

Fix getting leaf certificate#121

Merged
DarkaMaul merged 9 commits into
trailofbits:mainfrom
pjrobertson:fix_leaf_certificate
Mar 14, 2025
Merged

Fix getting leaf certificate#121
DarkaMaul merged 9 commits into
trailofbits:mainfrom
pjrobertson:fix_leaf_certificate

Conversation

@pjrobertson

@pjrobertson pjrobertson commented Mar 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

As per the discussion in #104 - this correctly re-orders the certificate chain to get the correct leaf cert.

The logic here is: the leaf certificate is the one that isn't an issuer of any other cert.

Requesting to be merged into @DarkaMaul 's current fix-eku branch

@pjrobertson pjrobertson changed the base branch from main to dm/fix-eku March 11, 2025 07:53
@pjrobertson pjrobertson mentioned this pull request Mar 11, 2025
Closed
@pjrobertson pjrobertson mentioned this pull request Mar 11, 2025
Closed
@DarkaMaul

DarkaMaul commented Mar 11, 2025

Copy link
Copy Markdown
Collaborator

Thanks a lot; the fix is excellent.

I took the liberty to add tests to your PR - I hope you won't mind.

@pjrobertson

pjrobertson commented Mar 11, 2025

Copy link
Copy Markdown
Contributor Author

I took the liberty to add tests to your PR - I hope you won't mind.

Absolutely, of course not. Great to have those tests in there :)

@pjrobertson pjrobertson mentioned this pull request Mar 11, 2025
Merged
@DarkaMaul DarkaMaul deleted the branch trailofbits:main March 13, 2025 14:20
@DarkaMaul DarkaMaul closed this Mar 13, 2025
@DarkaMaul DarkaMaul reopened this Mar 13, 2025
@DarkaMaul DarkaMaul changed the base branch from dm/fix-eku to main March 13, 2025 14:26
@pjrobertson pjrobertson force-pushed the fix_leaf_certificate branch from d09a10b to 05819d9 Compare March 13, 2025 15:01
@pjrobertson

pjrobertson commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

I've just rebased and pushed to fix the merge conflicts now that #120 is merged. @DarkaMaul can you please double check and review? Thanks!

DarkaMaul
DarkaMaul reviewed Mar 13, 2025
View reviewed changes
Comment thread test/test_verify.py Outdated
DarkaMaul
DarkaMaul reviewed Mar 13, 2025
View reviewed changes
Comment thread src/rfc3161_client/verify.py Outdated
@DarkaMaul

DarkaMaul commented Mar 13, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

I've just rebased and pushed to fix the merge conflicts now that #120 is merged. @DarkaMaul can you please double check and review? Thanks!

Everything looks good minor two nitpicks (a test that was never merged, and a weirdly placed comments).
I've added a CHANGELOG entry and we're good to merge.

Thanks a lot for your help into identifying and fixing this issue - that's really appreciated!

@pjrobertson

pjrobertson commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for being so quick to respond! I've pushed those tweaks :)

@DarkaMaul DarkaMaul merged commit 7e9882f into trailofbits:main Mar 14, 2025
@pjrobertson pjrobertson deleted the fix_leaf_certificate branch March 14, 2025 14:24
@pjrobertson

pjrobertson commented Mar 14, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the merge! Looking forward to a new release 🎉

We will start using the client in auto-archiver once this is released :)

@DarkaMaul DarkaMaul mentioned this pull request Mar 31, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DarkaMaul DarkaMaul DarkaMaul approved these changes

@facutuesca facutuesca Awaiting requested review from facutuesca facutuesca is a code owner

@woodruffw woodruffw Awaiting requested review from woodruffw

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pjrobertson @DarkaMaul