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

Use more external crates #72

Open
lukaslihotzki opened this issue Jan 23, 2023 · 3 comments
Open

Use more external crates #72

lukaslihotzki opened this issue Jan 23, 2023 · 3 comments

Comments

@lukaslihotzki
Copy link

ACMEd uses custom code for some things that are already provided by external crates.

  • There are multiple crates providing jws and jwk. josekit is one of those also using openssl. Can josekit replacing the existing jws and jwk code?
  • There are a lot of crates that provide config parsing. Can some of those (config, for example) replace the custom config parser (and layering) of ACMEd?
@breard-r
Copy link
Owner

I a general way, I don't see the point of adding dependencies just for the sake of doing so. I'm more inclined to reduce the number of dependencies instead of increasing it.

Can josekit replacing the existing jws and jwk code?

Sorry, but this one is a no-go. josekit uses a fixed version for openssl (currently 0.10.38 while 0.10.45 is out), and therefore it would either duplicate the openssl dependency or force ACMEd to use the exacte same openssl version. I don't find any of those choices acceptable.

Furthermore, using it would not permit to use an alternative crypto library as requested in issues #2 and #33. I'm seriously considering Botan + Ring as an optional alternative to OpenSSL.

There are a lot of crates that provide config parsing. Can some of those (config, for example) replace the custom config parser (and layering) of ACMEd?

The configuration parsing itself is done by the toml crate, not a custom parser. That said, I will have a look at config and see if it can improve ACMEd in any way.

@jcgruenhage
Copy link
Contributor

I a general way, I don't see the point of adding dependencies just for the sake of doing so. I'm more inclined to reduce the number of dependencies instead of increasing it.

That's not what we're saying though, we're saying that it makes sense to not build stuff yourself when there's solid libraries out there implementing what you're trying to do.

Sorry, but this one is a no-go. josekit uses a fixed version for openssl (currently 0.10.38 while 0.10.45 is out), and therefore it would either duplicate the openssl dependency or force ACMEd to use the exacte same openssl version. I don't find any of those choices acceptable.

That's not accurate. Specifying 0.10.38 in the Cargo.toml is the same as specifiying ^0.10.38, which means allowing all sem-ver compatible upgrades. 0.10.45 would be covered by that, meaning that your 0.10 and their 0.10.38 would when compiled without a lockfile right now both result in 0.10.45 being pulled in.

Furthermore, using it would not permit to use an alternative crypto library as requested in issues #2 and #33. I'm seriously considering Botan + Ring as an optional alternative to OpenSSL.

It would not prevent this, but instead shift where that work would need to happen. Either way, I'm not convinced that the maintenance burden for supporting multiple cryptographic backends in an ACME client is worth it. Sticking with one tried and tested variant would be a lot better IMO. It'd be nice if the libraries used under the hood would support multiple cryptographic backends, for example having a JOSE library that has feature toggles for openssl vs some pure rust impl, that'd be nice, but it's not really that big of a deal, at least not in my opinion.

There are a lot of crates that provide config parsing. Can some of those (config, for example) replace the custom config parser (and layering) of ACMEd?

The configuration parsing itself is done by the toml crate, not a custom parser. That said, I will have a look at config and see if it can improve ACMEd in any way.

Maybe parsing wasn't the right word here, but layering was explicitly mentioned as well, and layering is something that is provided in libraries like config as well. The code for handling the config in acmed is at ~750 lines, which is quite a lot, considering that it's basically only reading a few files and merging the results.

Just to make sure that it's not lost: I've posted a fairly extensive reply over at #71 (comment) right before I closed that issue. Have you seen that? It's going into details a bit what our intention is in opening these issues.

@breard-r
Copy link
Owner

it makes sense to not build stuff yourself when there's solid libraries out there implementing what you're trying to do

When I started ACMEd there was no crate that implemented JOSE in a way that I find acceptable for this use. josekit started a year later.

That's not accurate.

My bad. It seems like I need to re-read the cargo book, I'm a little bit rusty on that point.

Just to make sure that it's not lost: I've posted a fairly extensive reply over at #71 (comment) right before I closed that issue. Have you seen that? It's going into details a bit what our intention is in opening these issues.

Don't worry, I've seen it. I just need some time to reply 😉

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

No branches or pull requests

3 participants