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

Series of changes from wire.com #62

Merged
merged 23 commits into from
Sep 12, 2024

Conversation

augustocdias
Copy link

@augustocdias augustocdias commented Aug 28, 2024

Closes #61

Here's a rebased version of our changes. There were conflicts in almost all conflicts. I hope I resolved them correctly.

use lazy_static::lazy_static;

#[cfg(feature = "std")]
Copy link
Owner

Choose a reason for hiding this comment

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

I am not tracking this change and similar in this file. When I build with cargo hack build --feature-powerset it fails straightaway when building cargo build --no-default-features. I did not look at regex source code, but nothing in the docs indicates no-std support presently. How did you envision removing the feature gate here working?

@carl-wallace
Copy link
Owner

Aside from not seeing how the removal of the std feature gate for regex and url usage should work, this all looks great. Thank you for doing the work and for sharing. I plan to merge as-is tomorrow but you can expect a few changes over the next days/weeks, including:

  • Restore the std feature gate around regex and url usage. Any additional info on what you were aiming for here would be helpful.
  • Add an rsa feature gate to tests that require rsa, which is most of them, and, longer-term, replace or augment those tests with artifacts that don't use RSA.
  • Update documentation to reflect changes to features, algorithms, etc.
  • Update ml-dsa and slh-dsa support to FIPS standard versions once pqcrypto is updated (and/or move to RustCrypto implementations).
  • Align with changes to RustCrypto/formats. These changes will likely impact your code (though probably mostly mechanical changes). Many of the changes emanate from x509-cert: Only allow Certificates to be constructed via either the builder or deserialization RustCrypto/formats#1486.

Once updates for the crates related to the last item are released, I'll likely merge this branch to main and prepare a release of certval for crates.io.

@augustocdias
Copy link
Author

Hi. Thank you very much. I don't think I'll be able to answer why that was done. Our colleagues that made those changes are not in the company anymore. I'm currently very busy with other tasks and unable to investigate. I'd say go for it and if we see something breaks when we upgrade we come back and discuss what happened.

@carl-wallace carl-wallace merged commit 002cf40 into carl-wallace:cert_limbo Sep 12, 2024
0 of 5 checks passed
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.

4 participants