Skip to content

Commit

Permalink
Fail hard if SSL certs or keys are invalid (#2848)
Browse files Browse the repository at this point in the history
* Fail hard if OpenSSL cannot load cert or key

Previously if an invalid SSL cert or key (e.g. blank file, mismatching
private and public key, etc.) were specified, Puma would quietly
accept them, bind to the port, and mysteriously fail with an error
only after a client initiates a connection:

```
SSL error, peer: ::1, peer cert: : #<Puma::MiniSSL::SSLError: OpenSSL error: error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher - 193>
```

We now check the OpenSSL return values and raise an exception if the
cert or key cannot be loaded.

* Add SSL invalid tests

Co-authored-by: MSP-Greg <Greg.mpls@gmail.com>
  • Loading branch information
2 people authored and nateberkopec committed Aug 22, 2022
1 parent 416c1e2 commit c7dc812
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 5 deletions.
28 changes: 23 additions & 5 deletions ext/puma_http11/mini_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ typedef struct {

VALUE eError;

NORETURN(void raise_file_error(const char* caller, const char *filename));

void raise_file_error(const char* caller, const char *filename) {
rb_raise(eError, "%s: error in file '%s': %s", caller, filename, ERR_error_string(ERR_get_error(), NULL));
}

void engine_free(void *ptr) {
ms_conn *conn = ptr;
ms_cert_buf* cert_buf = (ms_cert_buf*)SSL_get_app_data(conn->ssl);
Expand Down Expand Up @@ -244,28 +250,38 @@ sslctx_initialize(VALUE self, VALUE mini_ssl_ctx) {

if (!NIL_P(cert)) {
StringValue(cert);
SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(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));
}
}

if (!NIL_P(key)) {
StringValue(key);
SSL_CTX_use_PrivateKey_file(ctx, RSTRING_PTR(key), SSL_FILETYPE_PEM);

if (SSL_CTX_use_PrivateKey_file(ctx, RSTRING_PTR(key), SSL_FILETYPE_PEM) != 1) {
raise_file_error("SSL_CTX_use_PrivateKey_file", RSTRING_PTR(key));
}
}

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);

SSL_CTX_use_certificate(ctx, x509);
if (SSL_CTX_use_certificate(ctx, x509) != 1) {
raise_file_error("SSL_CTX_use_certificate", RSTRING_PTR(cert_pem));
}
}

if (!NIL_P(key_pem)) {
bio = BIO_new(BIO_s_mem());
BIO_puts(bio, RSTRING_PTR(key_pem));
pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);

SSL_CTX_use_PrivateKey(ctx, pkey);
if (SSL_CTX_use_PrivateKey(ctx, pkey) != 1) {
raise_file_error("SSL_CTX_use_PrivateKey", RSTRING_PTR(key_pem));
}
}

verification_flags = rb_funcall(mini_ssl_ctx, rb_intern_const("verification_flags"), 0);
Expand All @@ -278,7 +294,9 @@ sslctx_initialize(VALUE self, VALUE mini_ssl_ctx) {

if (!NIL_P(ca)) {
StringValue(ca);
SSL_CTX_load_verify_locations(ctx, RSTRING_PTR(ca), NULL);
if (SSL_CTX_load_verify_locations(ctx, RSTRING_PTR(ca), NULL) != 1) {
raise_file_error("SSL_CTX_load_verify_locations", RSTRING_PTR(ca));
}
}

ssl_options = SSL_OP_CIPHER_SERVER_PREFERENCE | SSL_OP_SINGLE_ECDH_USE | SSL_OP_NO_COMPRESSION;
Expand Down
40 changes: 40 additions & 0 deletions test/test_puma_server_ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,46 @@ def test_http_rejection

assert busy_threads.zero?, "Our connection is wasn't dropped"
end

unless Puma.jruby?
def test_invalid_cert
assert_raises(Puma::MiniSSL::SSLError) do
start_server { |ctx| ctx.cert = __FILE__ }
end
end

def test_invalid_key
assert_raises(Puma::MiniSSL::SSLError) do
start_server { |ctx| ctx.key = __FILE__ }
end
end

def test_invalid_cert_pem
assert_raises(Puma::MiniSSL::SSLError) do
start_server { |ctx|
ctx.instance_variable_set(:@cert, nil)
ctx.cert_pem = 'Not a valid pem'
}
end
end

def test_invalid_key_pem
assert_raises(Puma::MiniSSL::SSLError) do
start_server { |ctx|
ctx.instance_variable_set(:@key, nil)
ctx.key_pem = 'Not a valid pem'
}
end
end

def test_invalid_ca
assert_raises(Puma::MiniSSL::SSLError) do
start_server { |ctx|
ctx.ca = __FILE__
}
end
end
end
end if ::Puma::HAS_SSL

# client-side TLS authentication tests
Expand Down

0 comments on commit c7dc812

Please sign in to comment.