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

Deterministic certificates #173

Open
thomaseizinger opened this issue Oct 4, 2023 · 20 comments
Open

Deterministic certificates #173

thomaseizinger opened this issue Oct 4, 2023 · 20 comments

Comments

@thomaseizinger
Copy link
Contributor

I'd like to have a way to create a fully deterministic certificate from a given seed. We use WebRTC to establish p2p connections and want to embed the certificate hash in the address information that we are passing around. Each node already has a cryptographic identity and I'd like to generate a deterministic certificate from that via a HKDF.

In an ideal world, rcgen exposes a Certificate::from_seed function and uses the provided seed to bootstrap all randomness inside. That would keep implementation details like ring contained within the library.

Alternatively, we can expose the randomness and let users deal with passing a seeded, deterministic RNG.

Thoughts?

@djc
Copy link
Member

djc commented Oct 4, 2023

It seems reasonable to offer some API that lets you pass a source of randomness similar to the ring API. As @est31 proposed, we'll probably need some kind of trait proxy for that if we want to keep the use of ring from leaking into the public API.

I'm not convinced rcgen should be in the business of narrowing the ring API (of passing in something that satisfies a very narrow random byte generation API) to something that converts seed numbers, but maybe if that avoids a bunch of complexity it's an option to consider?

@cpu
Copy link
Member

cpu commented Oct 4, 2023

It seems reasonable to offer some API that lets you pass a source of randomness similar to the ring API

Agreed, there's also precedent with the Golang X.509 package CreateCertificate func taking an io.Reader, or Botan's X509CA constructor taking a RandomNumberGenerator.

@est31
Copy link
Member

est31 commented Oct 4, 2023

I agee with @djc that we should not narrow the amount of flexibility provided by ring by only providing a seed based API. But we could make the rng trait implemented for a SeededRng or such, noting that the security wholly depends on the passed seed. Maybe behind an default-off cargo feature? IDK IIRC rustls only exposes custom TLS cert verification if you enable a cargo feature.

I think it's important to distinguish the possible types of determinism:

  • Determinism over multiple runs of the same binary is easy to provide, and something I'm open to support.
  • Determinism over rcgen version upgrades, ring version upgrades, etc, that is a) outside of our control due to ring probably changing the pattern it invokes the random generator with, and b) not sure something I want to support for rcgen either, as subtle changes can change the way you sign data. I know it is annoying, because sometimes I also have relied on this type of determinism and it was broken

People who want the second type of determinism often also don't want this to ever change, even with semver breaking changes, because the breakage is in the domain of data (migrating which is harder) and not of APIs (that you can adjust to changes for).

Currently it is not a breaking change for rcgen for example to change the default set of certificate fields. I don't think we should promise this to never change.

See also my earlier comment on the suggestion of determinism: #98 (comment)

@cpu
Copy link
Member

cpu commented Oct 4, 2023

IDK IIRC rustls only exposes custom TLS cert verification if you enable a cargo feature.

For what it's worth, that's true for the released crates but the next release will drop the dangerous_configuration feature and expose setting a custom certificate verifier without needing to set any feature flags.

@thomaseizinger
Copy link
Contributor Author

I think it's important to distinguish the possible types of determinism:

* Determinism over multiple runs of the same binary is easy to provide, and something I'm open to support.

* Determinism over rcgen version upgrades, ring version upgrades, etc, that is a) outside of our control due to ring probably changing the pattern it invokes the random generator with, and b) not sure something I want to support for rcgen either, as subtle changes can change the way you sign data. I know it is annoying, because sometimes I also have relied on this type of determinism and it was broken

Those are good points. I am after the first one. The 2nd kind can easily be controlled using a Cargo.lock file and verified by a test that checks against a generated certificate. If it breaks, one has to do a two-stage roll-out of some kind.

I'm not convinced rcgen should be in the business of narrowing the ring API (of passing in something that satisfies a very narrow random byte generation API) to something that converts seed numbers, but maybe if that avoids a bunch of complexity it's an option to consider?

It would be much easier API to support though because it is smaller. I am not too fussed which way we decide. I do want to say though that deterministic certificates will get more important as WebTransport gains traction. WebTransport offers the ability to connect to servers with self-signed certificates if you know the certificate hash in advance which is tightly coupled to deterministically generating certificates.

@thomaseizinger
Copy link
Contributor Author

Currently it is not a breaking change for rcgen for example to change the default set of certificate fields. I don't think we should promise this to never change.

That is fair and I wouldn't expect it to be communicated as breaking change.

@est31
Copy link
Member

est31 commented Oct 5, 2023

If it breaks, one has to do a two-stage roll-out of some kind.

Okay, that's the important part, that you keep it in mind when designing the system that builds on rcgen, because I don't want you to be extremely disrupted in the future by what should be a simple upgrade of ring, rcgen, or similar.

Any implementation in rcgen to support seeds should also make this very clear in the docs that you should not rely on the output to never ever change.

How far we should go with the guarantee, idk. Maybe it might differ between linux and windows. Maybe it might differ based on which version of a system crypto library is used. rcgen uses ring right now but it might gain multiple backends in the future where one backend consults an OS's system crypto API to do the operations... ring already has different asm implementations for different ISAs, like x86_64 vs arm. They might do the same operations in the end, but are they also guaranteed to be the same in how they use the secure RNG? Maybe? Is this guaranteed? Maybe it shells out to C which has some internal word sized parameters that it uses for managing the data, or does some operations that end up endianness dependent. Still perfectly preserving randomness but not having the same output on different endian architectures.

@thomaseizinger
Copy link
Contributor Author

Yeah that totally makes sense! This makes me think that a "best-effort deterministic certificate" is a better API than exposing randomness.

For one, it is closer to the actual usecase being solved, making life easier for users. I do wonder whether there are other reasons for controlling the rng?

Two, it gives us a place to add documentation that this is best effort and what might still affect the output etc.

Three, it would be possible to pro-actively control more parameters other than randomness. For example, if there are any hashmaps with random insertion order involved etc. Again, I think doing it at on a best effort basis is good enough here.

@djc
Copy link
Member

djc commented Oct 6, 2023

This still sounds like you want to do a more elaborate rcgen API on top of the basic ring API. I (still) think that's a bad idea -- the rcgen API should be highly similar to the underlying ring API. If you want to do something more deterministic that should live in your library/application.

WebTransport offers the ability to connect to servers with self-signed certificates if you know the certificate hash in advance which is tightly coupled to deterministically generating certificates.

I'm curious where/how this is documented.

@thomaseizinger
Copy link
Contributor Author

WebTransport offers the ability to connect to servers with self-signed certificates if you know the certificate hash in advance which is tightly coupled to deterministically generating certificates.

I'm curious where/how this is documented.

See https://www.w3.org/TR/webtransport/#dom-webtransportoptions-servercertificatehashes.

Chrome has already implemented it: https://chromestatus.com/feature/5690646332440576
Firefox is likely getting it too: mozilla/standards-positions#167 (comment)

@thomaseizinger
Copy link
Contributor Author

This still sounds like you want to do a more elaborate rcgen API on top of the basic ring API. I (still) think that's a bad idea -- the rcgen API should be highly similar to the underlying ring API. If you want to do something more deterministic that should live in your library/application.

Can you elaborate why you think it is a bad idea?

@est31
Copy link
Member

est31 commented Oct 7, 2023

I think the best reason to pass stateful random generators rather than seeds is that the seeds could otherwise easily be reused. it's obviously bad when one has programs using shared seeds in two places.

second, we'd create an entropy bottleneck. maybe some post quantum crypto algorithm needs higher levels of entropy.

third, crates.io easily allows for deep hierarchies, and it would be quite weird if at each spot one would have a different paradigm used for the random number generation. then you'd have the uppermost level fill a seed, then pass it to an API that takes a trait that allows one to fill buffers. the next level creates a seed again, and so on and so forth. ideally one would just have one of the two and if ring already directs us towards traits, we should use those.

That's why I think we should just simply copy paste the SecureRandom trait and impl it for some SeededRng struct.

@thomaseizinger
Copy link
Contributor Author

That all makes sense but it is not going to work because ring's SecureRandom trait is sealed, meaning you can't implement it for your own struct. See https://docs.rs/ring/0.17.2/src/ring/rand.rs.html#23.

So I think we may be stuck with exposing ring's randomness traits.

@djc
Copy link
Member

djc commented Oct 9, 2023

AFAICT since ring's EcdsaKeyPair constructors now all require a &dyn SecureRandom and given that that trait is sealed, it basically means that ring itself doesn't support the feature you're looking for of building deterministic certificates. So I guess you'll have to go convince Brian that it's worth supporting this use case in ring or rcgen's support for deterministic certificate generation has to be built on a different crypto library.

@est31
Copy link
Member

est31 commented Oct 10, 2023

Hmm yeah good point about it being sealed. I guess we'll have to either get Brian to un-seal the trait, or we address #74. Until then we can't do much about this issue.

Open to merging a PR that adds a cfg controlled API (preferably without a cargo feature) that exposes the ring API as suggested in #166 originally. Then you will only have to fork ring locally not also rcgen (still having to set the cfg somehow though).

@thomaseizinger
Copy link
Contributor Author

AFAICT since ring's EcdsaKeyPair constructors now all require a &dyn SecureRandom and given that that trait is sealed, it basically means that ring itself doesn't support the feature you're looking for of building deterministic certificates. So I guess you'll have to go convince Brian that it's worth supporting this use case in ring or rcgen's support for deterministic certificate generation has to be built on a different crypto library.

There is a dedicated module for deterministic randomness in ring: https://github.com/briansmith/ring/blob/2e8363b433fa3b3962c877d9ed2e9145612f3160/src/test.rs#L468-L542

@thomaseizinger
Copy link
Contributor Author

AFAICT since ring's EcdsaKeyPair constructors now all require a &dyn SecureRandom and given that that trait is sealed, it basically means that ring itself doesn't support the feature you're looking for of building deterministic certificates. So I guess you'll have to go convince Brian that it's worth supporting this use case in ring or rcgen's support for deterministic certificate generation has to be built on a different crypto library.

There is a dedicated module for deterministic randomness in ring: briansmith/ring@2e8363b/src/test.rs#L468-L542

Relying on that for a feature in rcgen is probably not good though.

Open to merging a PR that adds a cfg controlled API (preferably without a cargo feature) that exposes the ring API as suggested in #166 originally.

The ring API is already exposed here

rcgen/src/error.rs

Lines 101 to 111 in 948c3b5

impl From<ring::error::Unspecified> for Error {
fn from(_unspecified: ring::error::Unspecified) -> Self {
Error::RingUnspecified
}
}
impl From<ring::error::KeyRejected> for Error {
fn from(err: ring::error::KeyRejected) -> Self {
Error::RingKeyRejected(err.to_string())
}
}
but I am guessing that was more by mistake rather than on purpose :)

Am I understanding you correctly that you'd like a plain cfg flag that enables a particular API? What is wrong with using a regular feature-flag for that? I'd like to embed deterministic certificates into rust-libp2p so I only have access to regular cargo features.

@djc
Copy link
Member

djc commented Oct 10, 2023

Open to merging a PR that adds a cfg controlled API (preferably without a cargo feature) that exposes the ring API as suggested in #166 originally. Then you will only have to fork ring locally not also rcgen (still having to set the cfg somehow though).

That seems like a very complicated, user-unfriendly solution. IMO it probably isn't worth the complexity. Might be better off (a) advocating for a deterministic key generation API in ring or (b) investigating other ECDSA key generation methods (maybe the RustCrypto project has something that exposes this capability?).

@est31
Copy link
Member

est31 commented Oct 10, 2023

That seems like a very complicated, user-unfriendly solution. IMO it probably isn't worth the complexity. Might be better off

Agree that these two solutions you list as a and b are better than the cfg controlled API. I basically described them in my post too, before the paragraph you quote. That one I only meant that as a way to make the life of our users easier until we either have a or b implemented, and done in a way that we can remove it without a semver breaking change. Am especially doubtful of a's success so it will probably mean we'll have to do/wait for b :).

@djc
Copy link
Member

djc commented Oct 11, 2023

I find it hard to predict what Brian is open too, at least he seems to be a lot more responsive recently so I think it's worth opening an issue to get his feedback.

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

4 participants