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

Inconsistency between cert: and cert_pem: parameters to ssl_bind #3172

Closed
copiousfreetime opened this issue Jun 2, 2023 · 5 comments · Fixed by #3174
Closed

Inconsistency between cert: and cert_pem: parameters to ssl_bind #3172

copiousfreetime opened this issue Jun 2, 2023 · 5 comments · Fixed by #3174
Labels

Comments

@copiousfreetime
Copy link
Contributor

Describe the bug

When using ssl_bind the documentation seems to indicate that the following are equivalent:

File based certs/keys

puma/lib/puma/dsl.rb

Lines 513 to 520 in 763d1a1

# @example
# ssl_bind '127.0.0.1', '9292', {
# cert: path_to_cert,
# key: path_to_key,
# ssl_cipher_filter: cipher_filter, # optional
# verify_mode: verify_mode, # default 'none'
# verification_flags: flags, # optional, not supported by JRuby
# reuse: true # optional

String based certs/keys

puma/lib/puma/dsl.rb

Lines 526 to 531 in 763d1a1

# @example Alternatively, you can provide +cert_pem+ and +key_pem+:
# ssl_bind '127.0.0.1', '9292', {
# cert_pem: File.read(path_to_cert),
# key_pem: File.read(path_to_key),
# reuse: {size: 2_000, timeout: 20} # optional
# }

This appears to be incorrect.

When cert: is passed to ssl_bind - inside MiniSSL this file is loaded from disk using SSL_CTX_use_certificate_chain_file()

SSL_CTX_use_certificate_chain_file() loads a certificate chain from file into ctx. The certificates must be in PEM format and must be sorted starting with the subject's certificate (actual client or server certificate), followed by intermediate CA certificates if applicable, and ending at the highest level (root) CA

if (!NIL_P(cert)) {
StringValue(cert);
if (SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(cert)) != 1) {
raise_file_error("SSL_CTX_use_certificate_chain_file", RSTRING_PTR(cert));
}
}

When cert_pem: is passed to ssl_bind - inside MiniSSL this file is loaded using a combination of PEM_read_bio_X509 and SSL_CTX_use_certificate

if (!NIL_P(cert_pem)) {
bio = BIO_new(BIO_s_mem());
BIO_puts(bio, RSTRING_PTR(cert_pem));
x509 = PEM_read_bio_X509(bio, NULL, NULL, NULL);
if (SSL_CTX_use_certificate(ctx, x509) != 1) {
BIO_free(bio);
raise_file_error("SSL_CTX_use_certificate", RSTRING_PTR(cert_pem));
}
X509_free(x509);
BIO_free(bio);
}

If the contents of the file at path_to_cert is a full chain PEM then:

  • passing cert: path_to_cert a fully resolve cert chain is served up by puma
  • passing in cert_pem: File.read(path_to_cert) results in only the certificate itelf being loaded, and not the full chain.

To Reproduce

Take the self contained gist puma_ssl_test.rb and run it with ruby:

curl -s -O https://gist.githubusercontent.com/copiousfreetime/8ad7784ea609b2c5f6e5a3ccf0c9a2d6/raw/4c0197fafbf24fe2bf10438e873752b60e44c4ed/puma_ssl_test.rb 
ruby puma_ssl_test.rb --help

There are 3 different tests

  1. ruby puma_ssl_test.rb --test files
  2. ruby puma_ssl_test.rb --test strings
  3. ruby puma_ssl_test.rb --test ca

Expected Behavior

I would expect that give the same contents to cert_pem: that was in the file that was given to cert: should result in the same level of ssl validation.

ruby puma_ssl_test.rb --test files

% ruby puma_ssl_test.rb --test files
Writing pem/cert.pem
Writing pem/key.pem
Writing pem/ca.pem
Writing pem/all.pem
Test puma with ssl_bind options:

  cert: pem/all.pem
  key: pem/key.pem

The cert file contains the certificate and the entire cert chain


Once Puma is running, you can test it with:

echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'

Puma starting in single mode...
* Puma version: 6.2.2 (ruby 3.2.2-p53) ("Speaking of Now")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 171
* Listening on ssl://[::]:9292?cert=pem%2Fall.pem&key=pem%2Fkey.pem&verify_mode=none
Use Ctrl-C to stop

And testing the ssl

% echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'
depth=2 O = anchordotdev, CN = development - AnchorCA
verify error:num=19:self signed certificate in certificate chain
verify return:0
DONE
 0 s:/O=anchordotdev/CN=anchor.lcl.host
   i:/O=anchordotdev/CN=anchor - SubCA
 1 s:/O=anchordotdev/CN=anchor - SubCA
   i:/O=anchordotdev/CN=development - AnchorCA
 2 s:/O=anchordotdev/CN=development - AnchorCA
   i:/O=anchordotdev/CN=development - AnchorCA
subject=/O=anchordotdev/CN=anchor.lcl.host

What I would expect -- full cert chain and notification that the self signed cert is there.

ruby puma_ssl_test.rb --test strings

% ruby puma_ssl_test.rb --test strings
Test puma with ssl_bind options:

  cert_pem: contents of pem/all.pem
  key_pem: contents of pem/key.pem

The cert_pem option contains the certificate and the entire cert chain


Once Puma is running, you can test it with:

echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'

Puma starting in single mode...
* Puma version: 6.2.2 (ruby 3.2.2-p53) ("Speaking of Now")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 263
* Listening on ssl://[::]:9292?cert=store%3A0&key=store%3A1&verify_mode=none
Use Ctrl-C to stop

And testing the ssl

% echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'
depth=0 O = anchordotdev, CN = anchor.lcl.host
verify error:num=20:unable to get local issuer certificate
verify return:1
depth=0 O = anchordotdev, CN = anchor.lcl.host
verify error:num=21:unable to verify the first certificate
verify return:1
DONE
 0 s:/O=anchordotdev/CN=anchor.lcl.host
   i:/O=anchordotdev/CN=anchor - SubCA
subject=/O=anchordotdev/CN=anchor.lcl.host
issuer=/O=anchordotdev/CN=anchor - SubCA

The cert chain is missing.

Observations

It appears that in order to use the cert_pem: option and get the same results as using cert: you must also use the ca: and verify_mode: options

ruby puma_ssl_test.rb --test ca

% ruby puma_ssl_test.rb --test ca
Test puma with ssl_bind options:

  cert_pem: contents of pem/cert.pem
  key_pem: contents of pem/key.pem
  ca: pem/ca.pem
  verify_mode: peer

The cert_pem option contains only the certificate.
The ca option is the path to the rest of the cert chain


Once Puma is running, you can test it with:

echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'

Puma starting in single mode...
* Puma version: 6.2.2 (ruby 3.2.2-p53) ("Speaking of Now")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 379
* Listening on ssl://[::]:9292?cert=store%3A0&key=store%3A1&verify_mode=peer&ca=pem%2Fca.pem
Use Ctrl-C to stop

And testing results in the same result as using cert: with the path to a file

% echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'
depth=2 O = anchordotdev, CN = development - AnchorCA
verify error:num=19:self signed certificate in certificate chain
verify return:0
DONE
 0 s:/O=anchordotdev/CN=anchor.lcl.host
   i:/O=anchordotdev/CN=anchor - SubCA
 1 s:/O=anchordotdev/CN=anchor - SubCA
   i:/O=anchordotdev/CN=development - AnchorCA
 2 s:/O=anchordotdev/CN=development - AnchorCA
   i:/O=anchordotdev/CN=development - AnchorCA
subject=/O=anchordotdev/CN=anchor.lcl.host
issuer=/O=anchordotdev/CN=anchor - SubCA

Questions

  • Is this the expected behavior when using cert_pem:
  • Or is it the cert: option that is actually out of spec, and they should both be using ca:
  • Should there also be a ca_pem: option for passing a string based ca option so none of them need to read from disk?
  • Should we submit a PR that attempts to resolve this and get consistent behavior, or is this something that you are readily familiar enough that would be trivial to resolve for you?

Desktop Environment (please complete the following information)

  • OS: macOS Monterey - 12.6.4
  • Puma Version Puma version: 6.2.2 (ruby 3.2.2-p53) ("Speaking of Now")
@MSP-Greg
Copy link
Member

MSP-Greg commented Jun 2, 2023

If I recall, there has never been parity between 'file based' and 'string based' functions.

Or, as you've mentioned, there are two 'file based' functions that will process multiple certs, but there are not equivalent string functions. I.think.

So, not sure what to do. The docs could be improved. I can't seriously look at this until this weekend...

@dentarg
Copy link
Member

dentarg commented Jun 2, 2023

I like consistency. There's probably not consistency here because the functionality has been contributed by different parties with different needs over time. Maybe you can dive into the git history/PRs and see what you find?

I know @stanhu of GitLab recently contributed a SSL related thing (#3133), maybe they have some thoughts on this.

@MSP-Greg
Copy link
Member

MSP-Greg commented Jun 2, 2023

SSL_CTX_use_certificate_chain_file was added with #801, 20-Oct-2015. Found that last night.

At the time, there wasn't an API for using strings. That was added 27-Oct-2021 with #2728.

#3133 added an option to pass a file thru a program/script, decrypting the file. The file doesn't contain useful information until it's decrypted.

@copiousfreetime
Copy link
Contributor Author

I took a look at the implementation of SSL_CTX_use_certificate_chain_file and it uses the rest of the public api to do its work. In fact the same way that cert_pem= uses PEM_read_bio_X509 and SSL_CTX_use_certificate is the first bit of SSL_CTX_use_certificate_chain_file implementation.

I'll give a go at an initial implementation to make cert= and cert_pem= be consistent.

@copiousfreetime
Copy link
Contributor Author

With the patch in the above PR - I now get the same results from the --test strings test as I do with the --test files test.

% ruby ssl_app.rb --test strings
Test puma with ssl_bind options:

  cert_pem: contents of pem/all.pem
  key_pem: contents of pem/key.pem

The cert_pem option contains the certificate and the entire cert chain


Once Puma is running, you can test it with:

echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'

Puma starting in single mode...
* Puma version: 6.3.0 (ruby 3.2.2-p53) ("Mugi No Toki Itaru")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 79379
* Listening on ssl://[::]:9292?cert=store%3A0&key=store%3A1&verify_mode=none
Use Ctrl-C to stop

And the ssl client test

% echo | openssl s_client -host anchor.lcl.host -port 9292 -debug -servername anchor.lcl.host | grep 'O=anchordotdev'
depth=2 O = anchordotdev, CN = development - AnchorCA
verify error:num=19:self signed certificate in certificate chain
verify return:0
DONE
 0 s:/O=anchordotdev/CN=anchor.lcl.host
   i:/O=anchordotdev/CN=anchor - SubCA
 1 s:/O=anchordotdev/CN=anchor - SubCA
   i:/O=anchordotdev/CN=development - AnchorCA
 2 s:/O=anchordotdev/CN=development - AnchorCA
   i:/O=anchordotdev/CN=development - AnchorCA
subject=/O=anchordotdev/CN=anchor.lcl.host
issuer=/O=anchordotdev/CN=anchor - SubCA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants