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

improve and refactor the Crypto story in rama #391

Open
GlenDC opened this issue Jan 11, 2025 · 4 comments
Open

improve and refactor the Crypto story in rama #391

GlenDC opened this issue Jan 11, 2025 · 4 comments
Assignees
Labels
Crypto enhancement New feature or request TLS
Milestone

Comments

@GlenDC
Copy link
Member

GlenDC commented Jan 11, 2025

This story would be about refactoring the code related to crypto and prepare the path for the crypto future of rama. Follow up stories can continue the work started here. There are two aspects to this:

  1. Regards to tls, we drop the attempt of finding one unified API to rule them all and instead respect that the rustls integration has a different purpose than that of boring. For all the standard common use cases Rustls is fine. And so it wil have a nice working client connector and a nice transport layer-service to accept streams on server sides. But that's pretty much where that one ends. Boring will be used in case you need a tight grip on the controls, have all kind of special needs, want to do advanced stuff etc. In this story we'll still work with the cloudflare bindings but in a future we'll want to provide our own rama-tls-boring crate. As part of this task you can already create this crate (dummy-style, see rama-socks5 for an example) to already reserve the name next time we publish an alpha version.
  2. Secondly, we affirm that crypto is more than just tls. So far we didn't have the luxary to think about that, but now with the http(s) stack shaping up we can think about that a bit more. Data verification, encryption and hashing is a thing and used for a lot more than just tls. E.g. JWT support is an example for that. As such it would make sense to perhaps have a rama-crypto crate where we would also house crypto primitives and expose crypto backends as well. Or perhaps we do not need such a crate and we could just expose jwt in future directly from within rama master crate somewhere.
  • we would need to think on how this relates to tls though. It's confusing in the crypto space in general. E.g. libs like boringssl and openssl are really focussed on SSL/TLS, but at the same time they also come with many related cryptographical algorithms and tools. This makes me not entirely certain what to do with crypto backends if we also have a rama-crypto crate. E.g. do we also expose boring there? Or instead use rama-tls-boring if the boring feature is enabled? or perhaps only have it in rama-crypto and have rama-tls-backend use that? All choices feel weird in their own way.
  • what is however more certain that it does make sense that we should have:
    • easy support using our backends already available for our tls support for: encryption, signing, hashing and random data
      (e.g. boring in case that's anyway available)
    • jwt support as it's pretty common for a lot of projects to need that, so it might make sense to expose that. That's not for this story. But I did start work on https://github.com/Keats/jsonwebtoken to have that crate be able to work with other crypto backends, which would allow us to embed that crate and re-export it, but reusing the same crypto backend (depending on the feature) that we already use for tls. Keeping the dep tree of the rama user small.

(2) will have less focus in this story then (1) but I would like that the path regarding this all becomes a lot more clear after we resolve this issue. (1) is however where the focus should be. It will result in some code removal and simplifying certain bits. And all the advanced / low-level controls will move to boring instead of something generic.

Not sure how this will impact rama::net::tls and rama::tls... that will ahve to be figured out. In one hand I think it wil make it possible to keep things a lot easier and just use the crypto backend API's directly instead of abstracting it away. At the same time I do like how easy to use the abstraction layer in the middle is to setup a lot of common stuff... So there's some thought that need to be put here. E.g. there's something to be said with how simple stuff like

let tls_server_config = ServerConfig::new(ServerAuth::SelfSigned(SelfSignedData::default()));
works compared to the more complicated matters that it controls under the hood...

@GlenDC GlenDC added this to the v0.2 milestone Jan 11, 2025
@GlenDC
Copy link
Member Author

GlenDC commented Jan 13, 2025

Had an offline discussion about this with @soundofspace . Here's what came out of that:

  • rustls will remain supported, but will work directly with the rustls types (E.g. rustls configs). It will still have a Layer for the server and a connector for the Client, but again just work directly with Rustls types there, no special support;
  • common APIs:
    • Anything which is meant for Context usage (e.g SecureTransport, NegotiatedTlsParameters, ...) will remain in their location and as-is. These are meant to communicate high level properties of the incoming request and are thus required to stay common, even for those people that implement their own layers/services/connectors.
    • tls APIs which are not meant for Context usage will no longer be in a common location. This means that the stuff like ClientAuth and ServerAuth, as well as all its used types, will no longer be in a generic location. Instead this will move in one form or another to rama-boring

rama-boring

rama-boring will be a new crate, which will be the only crate depending and exposing the boring crate. Utilities and convenient wrappers will all move to here. Rustls will not get any of that. "Advanced" features such as dynamic cert issuing etc will also move to here.

rama-tls/src/boring will rely on this crate.

rama-crypto

Will have the following root structure:

src
   - boring (== `pub use ::rama_boring as boring`?)
   - ring
   - aws-lc-rs
   - jwt (dummy for now)

The current goal is to not have to fork boring crate unless we really need to do so due to one or multiple blockers in future. It's mostly to have 1 common location for now where we expose it through and where such high level features are possible.

This can already be developed as well and also be exposed as rama::crypto.

The different modules have each their own API definitions but we do want to have some kind of harmony. E.g. while they each might have their different types and function signatures, we must try to align them where it is possible without extra work. E.g. keep symbol names and definitions the same where we anyway have the choice.

Examples of what they'll expose:

  • common crypto hashers/signers
  • similar rand functionality
  • in near future once jsonwebtoken work is done by @GlenDC : Crypto backend for jsonwebtoken

@soundofspace will pick up this work and it can be done in several steps.

@GlenDC GlenDC added enhancement New feature or request TLS Crypto and removed needs input labels Jan 13, 2025
@GlenDC
Copy link
Member Author

GlenDC commented Jan 20, 2025

@soundofspace github now has support for sub tasks, so in case you feel this story is too big as-is, do feel free to split it up where you see fit, in case you feel that structure in github issues is helpful for you.

@soundofspace
Copy link
Collaborator

Will start work on this now and I'm expecting this to take a very long time until it is completely finished.
As such, I will try to split this up as much as possible into smaller task, and will prioritize tasks that improve developer experience a lot vs amount of work needed to make it happen.

Also let me know if any other work is blocked on something here, or could be simplified a lot by changes here. Having a list of everything which currently sucks / is hard to do will make this much easier.

E.g. on the side I have been working on an acme client and some other stuff, huge pain points there are :

  • Implementing JWK and supporting all crypto backends blows up the scope of an acme client by a lot
  • Generating a certificate with a custom critical extension (acme oid) for tls-alpn-01 is currently not really possible with boringssl bindings
  • Async certificate callbacks in boring rust are of type Fn.
    • Can we make these FnOnce?
    • Can we expose these in an easier format?
    • Can we skip callbacks all together and directly issue async certs while having access to client hello, by use custom extension? Other code online seems to suggest we can (can't find link right now)

But please report any other issue you are having, or thing that might simplify developer experience a lot so I can see the bigger picture and know what to prioritize.

@GlenDC
Copy link
Member Author

GlenDC commented Feb 4, 2025

I think moving the tls backens to rama-boring and rama-rustls, splitting these up, is probably a good first step.

Is I think also the biggest hurddle currently as these API's are too much tangled up with each other.
Not sure what best next step for you after that could be. Depends a lot on how much time you have at hand.

What you list is pretty much anything of UX I can think of as well. Other than that we are currently limited by the bindings of cloudflare, so that probably would be for me another item on that list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crypto enhancement New feature or request TLS
Projects
None yet
Development

No branches or pull requests

2 participants