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

1.9.0 release notes #59

Closed
ctz opened this issue Sep 27, 2024 · 10 comments
Closed

1.9.0 release notes #59

ctz opened this issue Sep 27, 2024 · 10 comments

Comments

@ctz
Copy link
Member

ctz commented Sep 27, 2024

Sorry, not doing this in the right order. For review:

This crate now contains a stand-alone PEM decoder. This removes a dependency on the base64 crate, and also means PEM decoding of private key data is now done in constant time and avoids secret-dependent branches and memory accesses (at some performance cost).

The function of the rustls-pemfile crate is incorporated into this one. We will shortly make an apex release of rustls-pemfile which maintains the same API as before, using the new API in this crate.

@cpu
Copy link
Member

cpu commented Sep 27, 2024

LGTM

This removes a dependency on the base64 crate, and also means PEM decoding of private key data avoids secret-dependent branches and memory accesses.

My only suggestion is to throw in the term "constant time" since I think that's maybe more approachable/familiar to a casual reader than describing the secret-dependent branches and memory accesses that constitute whether something is/isn't constant time. Perhaps something like:

This removes a dependency on the base64 crate, and also means PEM decoding of private key data is now done in constant time and avoids secret-dependent branches and memory accesses.

@djc
Copy link
Member

djc commented Sep 27, 2024

Should we add a note on slight perf degradation? Why do we think this is better than the previous setup, is it just the constant time impl?

@ctz
Copy link
Member Author

ctz commented Sep 27, 2024

Should we add a note on slight perf degradation?

I don't object to mentioning it, but I kinda struggle to come up with a scenario where parsing PEM is performance sensitive, or hinges on the difference between parsing 1.6m certificates per second vs 1.0m certificates per second.

Edit: actually, added a mention specifically of key parsing costs; these are going to be significant (now measured: its about 3x slower).

@djc
Copy link
Member

djc commented Sep 27, 2024

What's the aggregate effect on say, running rustls-native-certs on a Debian system?

@ctz
Copy link
Member Author

ctz commented Sep 27, 2024

Good question! Looking...

@ctz
Copy link
Member Author

ctz commented Sep 27, 2024

before:

test benchmark ... bench:   1,834,606 ns/iter (+/- 94,466)

after:

test benchmark ... bench:   2,106,983 ns/iter (+/- 107,236)

so, 1.8ms vs 2.1ms

with just SSL_CERT_FILE=/usr/lib/ssl/cert.pem (that file has 146 certs, is 219KB):

before:

test benchmark ... bench:     226,172 ns/iter (+/- 15,505)

after:

test benchmark ... bench:     233,592 ns/iter (+/- 13,963)

so, conclusion: pretty minor difference caused by this, but actually the change to enumerate directories & chase symlinks had a pretty big cost.

@djc
Copy link
Member

djc commented Sep 27, 2024

Okay, good numbers!

@djc
Copy link
Member

djc commented Sep 27, 2024

Should we also mention #54?

@ctz
Copy link
Member Author

ctz commented Sep 27, 2024

Should we also mention #54?

I think this is OK in the non-headlines section; it is only is useful for testing, or ECH servers, or maybe DNS automation.

@cpu
Copy link
Member

cpu commented Sep 27, 2024

Agreed: it's also based on a pretty obscure draft that AFAIK hasn't been adopted by any working groups.

@ctz ctz closed this as completed Sep 27, 2024
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