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

Expose private key format options #137

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bdj
Copy link

@bdj bdj commented Feb 7, 2025

I want to use a LetsEncrypt key with the Racket web server, but the default private key is now an EC key. Exposing these parameters enables the web server to run with one of these keys (by setting rsa? to #f and asn1? to #t in the underlying call to ssl-load-private-key!). I also tried it with a key generated by the https://nixos.wiki/wiki/ACME service (which requires rsa? #f and asn1? #f).

I wanted to try to determine these parameters automatically from the key file, but I don't think there is a robust way to do that. If anyone has ideas, I can make that change instead.

I am unsure of how to write tests for this as it is just passing parameters through to the underlying openssl call. If anyone has ideas for how to write tests for this, I will add tests as well. I also don't know if I updated the documentation correctly.

You can see this working at https://bdj.dev currently (no uptime guarantees).

@LiberalArtist
Copy link
Contributor

I'm very glad you found this problem! (I was I little surprised that I didn't have the right combination of conditions to have seen this myself.)

I wanted to try to determine these parameters automatically from the key file, but I don't think there is a robust way to do that. If anyone has ideas, I can make that change instead.

I agree that this would be much, much nicer. When I configure Apache, I definitely don't need to specify (or know) the details of the file's contents. I just have to write:

SSLEngine on
SSLCertificateFile "/path/to/www.example.com.cert"
SSLCertificateKeyFile "/path/to/www.example.com.key"

But it is not immediately obvious to me, either, how to do that. I think @rmculpepper knows more about OpenSSL than I do.

@samth
Copy link
Member

samth commented Feb 7, 2025

The functions here: https://docs.openssl.org/3.1/man3/d2i_PrivateKey/#copyright seem to autodetect the key format, but that would require upgrading to v3.

@LiberalArtist
Copy link
Contributor

We already use v3 in some circumstances, and we should update what we ship anyway: racket/racket#5201

@rmculpepper
Copy link
Contributor

As I understand it, here's the current effective behavior of the optional arguments to ssl-load-private-key! (rsa?, der?):

#f #f -- accepts (text) PEM-encoded PKCS#8 or PKCS#1 key
#t #f -- accepts (text) PEM-encoded PKCS#1 key only
#f #t -- accepts (binary) DER-encoded PKCS#8 key only
#t #t -- accepts (binary) DER-encoded PKCS#1 key only

PKCS#1 defines an encoding for RSA keys only, and PKCS#8 defines an encoding that stores a key type along with the key data. But the PEM encoding has its own header tags that distinguish PKCS#1 from PKCS#8 data ("BEGIN RSA PRIVATE KEY" vs "BEGIN PRIVATE KEY"), and I think the SSL_CTX_use_PrivateKey_file function uses that to accept both formats. (This needs more testing.)

The current defaults (#t #f) are obsolete, because PKCS#8 is preferred over PKCS#1. The question is how to proceed in a backwards-compatible manner for Racket.

One choice would be to ignore the rsa? argument unless der? is true. Then it would use the PEM function that accepts both formats (I think). That's not completely backward compatible, because the old behavior is to skip to the first PKCS#1 key, so if a file contained a PKCS#8 key followed by a PKCS#1 key, this change would result is a different key selected, I believe. I think that situation should not be common.

Meanwhile, the web server may be able to get away with just using #f #f mode rather than adding options.

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