Skip to content
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.

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

Create the various helper functions to enable complete Revocationlist support #354

Merged
merged 4 commits into from
Mar 10, 2023

Conversation

aaomidi
Copy link
Contributor

@aaomidi aaomidi commented Feb 28, 2023

Most of the contents here are taken from Go's x509 library and Let's Encrypts changes to enable CRL linting: https://github.com/letsencrypt/boulder/tree/main/crl

The tests are directly taken from Let's Encrypt. Testing locally they all passed.

I've allowed edits by maintainers here as well - so feel free to make commits directly on top of this. Especially for providing any attribution to Let's Encrypt for their changes to the standard library here.

@aaomidi
Copy link
Contributor Author

aaomidi commented Mar 6, 2023

@christopher-henderson just to make sure I understand, are you waiting on me to create a new PR introducing a CRL lint using this change, or are you planning on reviewing this PR first?

@dadrian
Copy link
Member

dadrian commented Mar 6, 2023

I'm fairly certain we can't take MPL code in this project, at least not in this directory.

@dadrian
Copy link
Member

dadrian commented Mar 6, 2023

If it's just the tests that are from Boulder, we should be able to:

  • Convert them to table-driven tests
  • Add the table (data) to a subfolder of testdata and slap MPL on just that folder

Can you clarify what part is MPL, and either remove it or provide an alternate implementation?

@aaomidi
Copy link
Contributor Author

aaomidi commented Mar 7, 2023

Can you clarify what part is MPL, and either remove it or provide an alternate implementation?

I think the main problem here is that we (zcrypto) doesn't have a pathway to merge from Go upstream and maintain the various custom changes that have been made.

I believe I've made a note of everywhere this change differs from upstream. I'm open to suggestions on how to handle the few places the code differs. I think we can arguably get rid of it and see if we actually end up needing them.

@dadrian
Copy link
Member

dadrian commented Mar 10, 2023

Since it's been upstreamed, I think we're good to merge once all the CI passes.

@dadrian dadrian merged commit 31dfd55 into zmap:master Mar 10, 2023
@dadrian
Copy link
Member

dadrian commented Mar 10, 2023

Thanks!

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