From d8033be991487f2bc80b5ae26b175014f66e9d3d Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 19 Dec 2025 12:02:24 +0100 Subject: [PATCH v14 2/2] Review comments --- doc/src/sgml/runtime.sgml | 19 +- src/backend/libpq/be-secure-common.c | 47 ++- src/backend/libpq/be-secure-openssl.c | 276 ++++++++++++------ src/backend/libpq/be-secure.c | 3 + src/backend/utils/misc/guc_parameters.dat | 7 + src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/libpq/hba.h | 6 +- src/include/libpq/libpq.h | 1 + src/test/ssl/t/001_ssltests.pl | 41 ++- src/test/ssl/t/004_sni.pl | 164 +++++++---- 10 files changed, 397 insertions(+), 168 deletions(-) diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index ca0a114da76..0705b72ca4e 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -2626,8 +2626,8 @@ openssl x509 -req -in server.csr -text -days 365 \ hostname should either be set to the literal - hostname for the connection, no_sni or *. - contain details on how these values are + hostname for the connection, /no_sni/ or *. + contains details on how these values are used. Hostname setting values @@ -2644,14 +2644,17 @@ openssl x509 -req -in server.csr -text -days 365 \ * Not required - Default host, matches all connections + + Default host, matches all connections. + - no_sni + /no_sni/ Not allowed - Certificate and key to use for connection with no sslsni defined. + Certificate and key to use for connections with no + sslsni defined. @@ -2659,8 +2662,10 @@ openssl x509 -req -in server.csr -text -days 365 \ hostnameRequired - Certificate and key to use for connections to the host specified in the - connection. + Certificate and key to use for connections to the host specified in + the connection. Multiple hostnames can be defined by using a comma + separated list. The certificate and key will be used for connections + to all hosts in the list. diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index 78430aad825..251100c27b8 100644 --- a/src/backend/libpq/be-secure-common.c +++ b/src/backend/libpq/be-secure-common.c @@ -204,6 +204,7 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) parsedline->sourcefile = pstrdup(tok_line->file_name); parsedline->linenumber = tok_line->line_num; parsedline->rawline = pstrdup(tok_line->raw_line); + parsedline->hostnames = NIL; /* Initialize optional fields */ parsedline->ssl_passphrase_cmd = NULL; @@ -212,8 +213,21 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) /* Hostname */ field = list_head(tok_line->fields); tokens = lfirst(field); - token = linitial(tokens); - parsedline->hostname = pstrdup(token->string); + foreach_ptr(AuthToken, hostname, tokens) + { + if ((tokens->length > 1) && + (strcmp(hostname->string, "*") == 0 || strcmp(hostname->string, "/no_sni/") == 0)) + { + ereport(elevel, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("default and non-SNI entries cannot be mixed with other entries"), + errcontext("line %d of configuration file \"%s\"", + tok_line->line_num, tok_line->file_name)); + return NULL; + } + + parsedline->hostnames = lappend(parsedline->hostnames, pstrdup(hostname->string)); + } /* SSL Certificate (Required) */ field = lnext(tok_line->fields, field); @@ -227,6 +241,15 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) return NULL; } tokens = lfirst(field); + if (tokens->length > 1) + { + ereport(elevel, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("multiple values specified for SSL certificate"), + errcontext("line %d of configuration file \"%s\"", + tok_line->line_num, tok_line->file_name)); + return NULL; + } token = linitial(tokens); parsedline->ssl_cert = pstrdup(token->string); @@ -242,6 +265,15 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) return NULL; } tokens = lfirst(field); + if (tokens->length > 1) + { + ereport(elevel, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("multiple values specified for SSL key"), + errcontext("line %d of configuration file \"%s\"", + tok_line->line_num, tok_line->file_name)); + return NULL; + } token = linitial(tokens); parsedline->ssl_key = pstrdup(token->string); @@ -250,6 +282,15 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) if (!field) return parsedline; tokens = lfirst(field); + if (tokens->length > 1) + { + ereport(elevel, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("multiple values specified for SSL CA"), + errcontext("line %d of configuration file \"%s\"", + tok_line->line_num, tok_line->file_name)); + return NULL; + } token = linitial(tokens); parsedline->ssl_ca = pstrdup(token->string); @@ -301,7 +342,7 @@ parse_hosts_line(TokenizedAuthLine *tok_line, int elevel) * load_hosts * * Reads and parses the pg_hosts.conf configuration file and passes back a List - * of HostLine elements containing the parsed lines, or NIL in case of an empty + * of HostsLine elements containing the parsed lines, or NIL in case of an empty * file. The list is returned in the hosts_lines parameter. If loading the * file was successful, true is returned, else false. This function is * intended to be executed within a temporary memory context which can be diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index a540cc93163..c60bc2209b5 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -53,7 +53,7 @@ typedef struct HostContext { - const char *hostname; + List *hostnames; SSL_CTX *context; bool ssl_loaded_verify_locations; } HostContext; @@ -79,7 +79,7 @@ static int alpn_cb(SSL *ssl, const unsigned char *in, unsigned int inlen, void *userdata); -static int sni_servername_cb(SSL *ssl, int *al, void *arg); +static int sni_clienthello_cb(SSL *ssl, int *al, void *arg); static bool initialize_dh(SSL_CTX *context, bool isServerStart); static bool initialize_ecdh(SSL_CTX *context, bool isServerStart); static const char *SSLerrmessageExt(unsigned long ecode, const char *replacement); @@ -130,33 +130,31 @@ be_tls_init(bool isServerStart) MemoryContext host_memcxt; char *err_msg; int res; + List *new_sni_contexts = NIL; + HostContext *new_default_context = NULL; + HostContext *new_no_sni_context = NULL; + HostContext *new_Host_context = NULL; /* - * If there are contexts loaded when we init they must be released. This - * should only be possible during configuration reloads and not when the - * server is starting up. + * If ssl_sni is enabled, attempt to load and parse TLS configuration from + * the pg_hosts.conf file with the set of hosts returned as a list. If + * there are hosts configured they take precedence over the + * postgresql.conf config. Make sure to allocate the parsed rows in a + * temporary memory context so that we can avoid memory leaks from the + * parsing process. If ssl_sni is disabled then set the state accordingly + * to make sure we instead parse the config from postgresql.conf. */ - if (sni_contexts != NIL || default_context || no_sni_context) + if (ssl_sni) { - Assert(!isServerStart); - free_contexts(); - Host_context = NULL; - SSL_context = NULL; + host_memcxt = AllocSetContextCreate(CurrentMemoryContext, + "hosts file parser context", + ALLOCSET_SMALL_SIZES); + oldcxt = MemoryContextSwitchTo(host_memcxt); + res = load_hosts(&pg_hosts, &err_msg); + MemoryContextSwitchTo(oldcxt); } - - /* - * Attempt to load, and parse, TLS configuration from the pg_hosts.conf - * file with the set of hosts returned as a list. If there are hosts - * configured there they take precedence over the postgresql.conf config. - * Make sure to allocate the parsed rows in a temporary memory context so - * that we can avoid memory leaks from the parsing process. - */ - host_memcxt = AllocSetContextCreate(CurrentMemoryContext, - "hosts file parser context", - ALLOCSET_SMALL_SIZES); - oldcxt = MemoryContextSwitchTo(host_memcxt); - res = load_hosts(&pg_hosts, &err_msg); - MemoryContextSwitchTo(oldcxt); + else + res = HOSTSFILE_DISABLED; /* * pg_hosts.conf is not required to contain configuration, but if it does @@ -194,12 +192,13 @@ be_tls_init(bool isServerStart) errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("unable to load SSL config from \"%s\" line %i", host->sourcefile, host->linenumber)); - free_contexts(); MemoryContextDelete(host_memcxt); return -1; } - host_context = palloc0(sizeof(HostContext)); + host_context = palloc(sizeof(HostContext)); + host_context->hostnames = NIL; + host_context->ssl_loaded_verify_locations = false; host_context->context = tmp_context; /* Set flag to remember whether CA store has been loaded */ @@ -208,16 +207,21 @@ be_tls_init(bool isServerStart) /* * The hostname in the context is NULL in case it is the default - * host, or a context to use for non-SNI connections. + * host, or a context to use for non-SNI connections. We already + * know that default and non-SNI configurations are not mixed with + * hostnames so in those cases we can just take the head of the + * list. */ - if (strcmp(host->hostname, "*") == 0) - default_context = host_context; - else if (strcmp(host->hostname, "no_sni") == 0) - no_sni_context = host_context; + if (strcmp(linitial(host->hostnames), "*") == 0) + new_default_context = host_context; + else if (strcmp(linitial(host->hostnames), "/no_sni/") == 0) + new_no_sni_context = host_context; else { - host_context->hostname = pstrdup(host->hostname); - sni_contexts = lappend(sni_contexts, host_context); + foreach_ptr(char, hostname, host->hostnames) + host_context->hostnames = lappend(host_context->hostnames, + pstrdup(hostname)); + new_sni_contexts = lappend(new_sni_contexts, host_context); } /* @@ -225,8 +229,8 @@ be_tls_init(bool isServerStart) * until the SNI callback switches over to the expected one, for * now just set it to the first one we see. */ - if (!Host_context) - Host_context = host_context; + if (!new_Host_context) + new_Host_context = host_context; } MemoryContextDelete(host_memcxt); @@ -236,7 +240,7 @@ be_tls_init(bool isServerStart) * If the pg_hosts.conf file doesn't exist, or is empty, then load the * config from postgresql.conf. */ - else if (res == HOSTSFILE_EMPTY || res == HOSTSFILE_MISSING) + else if (res == HOSTSFILE_DISABLED || res == HOSTSFILE_EMPTY || res == HOSTSFILE_MISSING) { HostsLine pgconf; SSL_CTX *tmp_context = NULL; @@ -264,17 +268,18 @@ be_tls_init(bool isServerStart) * can also set it as the Host_context since it will be used for all * connections. */ - default_context = palloc0(sizeof(HostContext)); - default_context->context = tmp_context; - Host_context = default_context; + new_default_context = palloc(sizeof(HostContext)); + new_default_context->context = tmp_context; + new_default_context->ssl_loaded_verify_locations = false; + new_Host_context = new_default_context; /* Set flag to remember whether CA store has been loaded */ if (ssl_ca_file[0]) - default_context->ssl_loaded_verify_locations = true; + new_default_context->ssl_loaded_verify_locations = true; } /* Make sure we have at least one certificate loaded */ - if (sni_contexts == NIL && !default_context && !no_sni_context) + if (new_sni_contexts == NIL && !new_default_context && !new_no_sni_context) { ereport(isServerStart ? FATAL : LOG, errcode(ERRCODE_CONFIG_FILE_ERROR), @@ -282,6 +287,24 @@ be_tls_init(bool isServerStart) return -1; } + /* + * If there are contexts loaded when we init they must be released. This + * should only be possible during configuration reloads and not when the + * server is starting up. + */ + if (sni_contexts != NIL || default_context || no_sni_context) + { + Assert(!isServerStart); + free_contexts(); + Host_context = NULL; + SSL_context = NULL; + } + + sni_contexts = new_sni_contexts; + no_sni_context = new_no_sni_context; + default_context = new_default_context; + Host_context = new_Host_context; + SSL_context = Host_context->context; return 0; @@ -323,12 +346,6 @@ ssl_init_context(bool isServerStart, HostsLine *host_line) */ SSL_CTX_set_mode(context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); - /* - * Install SNI TLS extension callback in order to validate hostnames in - * case we have at least one context configured with a host name. - */ - SSL_CTX_set_tlsext_servername_callback(context, sni_servername_cb); - /* * Call init hook (usually to set password callback) */ @@ -544,16 +561,23 @@ ssl_init_context(bool isServerStart, HostsLine *host_line) * free it when no longer needed. */ SSL_CTX_set_client_CA_list(context, root_cert_list); + + /* + * Always ask for SSL client cert, but don't fail if it's not + * presented. We might fail such connections later, depending on what + * we find in pg_hba.conf. + */ + SSL_CTX_set_verify(context, + (SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE), + verify_cb); } /* - * Always ask for SSL client cert, but don't fail if it's not presented. - * We might fail such connections later, depending on what we find in - * pg_hba.conf. + * Install SNI TLS extension callback in order to validate hostnames in + * case ssl_sni has been enabled. */ - SSL_CTX_set_verify(context, - (SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE), - verify_cb); + if (ssl_sni) + SSL_CTX_set_client_hello_cb(context, sni_clienthello_cb, NULL); /*---------- * Load the Certificate Revocation List (CRL). @@ -645,6 +669,10 @@ be_tls_open_server(Port *port) /* enable ALPN */ SSL_CTX_set_alpn_select_cb(SSL_context, alpn_cb, port); + SSL_CTX_set_verify(SSL_context, + (SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE), + verify_cb); + if (!(port->ssl = SSL_new(SSL_context))) { ereport(COMMERROR, @@ -1571,61 +1599,83 @@ alpn_cb(SSL *ssl, } /* - * sni_servername_cb + * sni_clienthello_cb * - * Callback executed by OpenSSL during handshake in case the server has been - * configured to validate hostnames. Returning SSL_TLSEXT_ERR_ALERT_FATAL to - * OpenSSL will immediately terminate the handshake. + * Callback for extracting the servername extension from the TLS handshake + * during ClientHello. There is a callback in OpenSSL for the servername + * specifically but OpenSSL themselves advice against using it as it is more + * dependent on ordering for execution. */ static int -sni_servername_cb(SSL *ssl, int *al, void *arg) +sni_clienthello_cb(SSL *ssl, int *al, void *arg) { const char *tlsext_hostname; + const unsigned char *tlsext; + size_t left, + len; HostContext *install_context = NULL; - tlsext_hostname = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); + if (!ssl_sni) + return SSL_CLIENT_HELLO_SUCCESS; - /* - * If there is no hostname set in the TLS extension, we have two options: - * i) there is a HostContext defined for non-SNI connections, in that case - * we switch to that; ii) there is no non-SNI config and we error out as - * there is no context to switch to. - */ - if (!tlsext_hostname) + if (SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_server_name, &tlsext, &left)) { - if (no_sni_context) - install_context = no_sni_context; - else if (default_context) - install_context = default_context; - else + if (left <= 2) + { + *al = SSL_AD_MISSING_EXTENSION; + return 0; + } + len = (*(tlsext++) << 8); + len += *(tlsext)++; + if (len + 2 != left) { - /* - * The error message for a missing server_name should, according - * to RFC 8446, be missing_extension. This isn't entirely ideal - * since the user won't be able to tell which extension the server - * considered missing. Sending unrecognized_name would be a more - * helpful error, but for now we stick to the RFC. - */ *al = SSL_AD_MISSING_EXTENSION; + return 0; + } - ereport(COMMERROR, - (errcode(ERRCODE_PROTOCOL_VIOLATION), - errmsg("no hostname provided in callback"))); - return SSL_TLSEXT_ERR_ALERT_FATAL; + left = len; + + if (left == 0 || *tlsext++ != TLSEXT_NAMETYPE_host_name) + { + *al = SSL_AD_MISSING_EXTENSION; + return 0; } - } - else - { + + left--; + + /* + * Now we can finally pull out the byte array with the actual + * hostname. + */ + if (left <= 2) + { + *al = SSL_AD_MISSING_EXTENSION; + return 0; + } + len = (*(tlsext++) << 8); + len += *(tlsext++); + if (len + 2 > left) + { + *al = SSL_AD_MISSING_EXTENSION; + return 0; + } + left = len; + tlsext_hostname = (const char *) tlsext; + /* * We have a requested hostname from the client, match against all * entries in the pg_hosts configuration and attempt to find a match. + * Matching is done case insensitive as per RFC 952 and RFC 921. */ foreach_ptr(HostContext, host, sni_contexts) { - if (strcmp(host->hostname, tlsext_hostname) == 0) + foreach_ptr(char, hostname, host->hostnames) { - install_context = host; - break; + if (pg_strncasecmp(hostname, tlsext_hostname, len) == 0) + { + install_context = host; + goto found; + } } } @@ -1636,14 +1686,44 @@ sni_servername_cb(SSL *ssl, int *al, void *arg) if (!install_context && default_context) install_context = default_context; } + else + { + if (no_sni_context) + install_context = no_sni_context; + else if (default_context) + install_context = default_context; + else + { + /* + * The error message for a missing server_name should, according + * to RFC 8446, be missing_extension. This isn't entirely ideal + * since the user won't be able to tell which extension the server + * considered missing. Sending unrecognized_name would be a more + * helpful error, but for now we stick to the RFC. + */ + *al = SSL_AD_MISSING_EXTENSION; + + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("no hostname provided in callback, and no fallback configured"))); + return SSL_CLIENT_HELLO_ERROR; + } + } /* * If we reach here without a context chosen as the session context then * fail the handshake and terminate the connection. */ if (install_context == NULL) - return SSL_TLSEXT_ERR_ALERT_FATAL; + { + if (tlsext_hostname) + *al = SSL_AD_UNRECOGNIZED_NAME; + else + *al = SSL_AD_MISSING_EXTENSION; + return SSL_CLIENT_HELLO_ERROR; + } +found: Host_context = install_context; SSL_context = install_context->context; if (SSL_set_SSL_CTX(ssl, SSL_context) == NULL) @@ -1651,10 +1731,14 @@ sni_servername_cb(SSL *ssl, int *al, void *arg) ereport(COMMERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("failed to switch to SSL context for host")); - return SSL_TLSEXT_ERR_ALERT_FATAL; + return SSL_CLIENT_HELLO_ERROR; } - return SSL_TLSEXT_ERR_OK; + /* Copy over context settings */ + SSL_clear_options(ssl, 0xFFFFFFFFL); + SSL_set_options(ssl, SSL_CTX_get_options(SSL_context)); + + return SSL_CLIENT_HELLO_SUCCESS; } /* @@ -2106,8 +2190,12 @@ free_contexts(void) { foreach_ptr(HostContext, host, sni_contexts) { - if (host->hostname) - pfree(unconstify(char *, host->hostname)); + if (host->hostnames != NIL) + { + foreach_ptr(char, hostname, host->hostnames) + pfree(hostname); + list_free(host->hostnames); + } SSL_CTX_free(host->context); } @@ -2116,7 +2204,7 @@ free_contexts(void) } /* - * The hostname need not be freed for the no_sni and default contexts + * The hostname list need not be freed for the no_sni and default contexts * since they by definition are not connected to a hostname and thus have * none allocated. */ diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 6dcb673843a..542aaaa2b26 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -56,6 +56,9 @@ bool SSLPreferServerCiphers; int ssl_min_protocol_version = PG_TLS1_2_VERSION; int ssl_max_protocol_version = PG_TLS_ANY; +/* GUC variable: if false, discards hostname extensions in handshake */ +bool ssl_sni = false; + /* ------------------------------------------------------------ */ /* Procedures common to all secure sessions */ /* ------------------------------------------------------------ */ diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index b95c373fb41..2765e6bbf4a 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -2758,6 +2758,13 @@ max => '0', }, +{ name => 'ssl_sni', type => 'bool', context => 'PGC_SIGHUP', group => 'CONN_AUTH_SSL', + short_desc => 'Sets whether to interpret SNI extensions in SSL connections.', + flags => 'GUC_SUPERUSER_ONLY', + variable => 'ssl_sni', + boot_val => 'false', +}, + { name => 'ssl_tls13_ciphers', type => 'string', context => 'PGC_SIGHUP', group => 'CONN_AUTH_SSL', short_desc => 'Sets the list of allowed TLSv1.3 cipher suites.', long_desc => 'An empty string means use the default cipher suites.', diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 1f360110564..404621df9bf 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -123,6 +123,7 @@ #ssl_dh_params_file = '' #ssl_passphrase_command = '' #ssl_passphrase_command_supports_reload = off +#ssl_sni = off #------------------------------------------------------------------------------ diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 38713381255..9d35ea1ba63 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -159,13 +159,12 @@ typedef struct HostsLine char *rawline; /* Required fields */ - bool default_host; - char *hostname; + List *hostnames; char *ssl_key; char *ssl_cert; - char *ssl_ca; /* Optional fields */ + char *ssl_ca; char *ssl_passphrase_cmd; bool ssl_passphrase_reload; } HostsLine; @@ -176,6 +175,7 @@ typedef enum HostsFileLoad HOSTSFILE_LOAD_FAILED, HOSTSFILE_EMPTY, HOSTSFILE_MISSING, + HOSTSFILE_DISABLED, } HostsFileLoadResult; /* diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h index 3d734266172..47009ead442 100644 --- a/src/include/libpq/libpq.h +++ b/src/include/libpq/libpq.h @@ -110,6 +110,7 @@ extern PGDLLIMPORT int ssl_max_protocol_version; extern PGDLLIMPORT char *ssl_passphrase_command; extern PGDLLIMPORT bool ssl_passphrase_command_supports_reload; extern PGDLLIMPORT char *ssl_dh_params_file; +extern PGDLLIMPORT bool ssl_sni; extern PGDLLIMPORT char *SSLCipherSuites; extern PGDLLIMPORT char *SSLCipherList; extern PGDLLIMPORT char *SSLECDHCurve; diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index c0104f6aa81..6a433623d1f 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -201,7 +201,7 @@ $result = $node->restart(fail_ok => 1); is($result, 0, 'restart fails with incorrect groups'); ok($node->log_contains(qr/no SSL error reported/) == 0, 'error message translated'); -$node->append_conf('ssl_config.conf', qq{ssl_groups='prime256v1'}); +$node->append_conf('sslconfig.conf', qq{ssl_groups='prime256v1'}); $result = $node->restart(fail_ok => 1); ### Run client-side tests. @@ -1004,4 +1004,43 @@ $node->connect_fails( qr{Failed certificate data \(unverified\): subject "/CN=\\xce\\x9f\\xce\\xb4\\xcf\\x85\\xcf\\x83\\xcf\\x83\\xce\\xad\\xce\\xb1\\xcf\\x82", serial number \d+, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"}, ]); +# Test client CAs +my $connstr = + "user=ssltestuser dbname=certdb hostaddr=$SERVERHOSTADDR sslmode=require sslsni=1"; + +switch_server_cert($node, certfile => 'server-cn-only', cafile => ''); +# example.org is unconfigured and should fail. +$node->connect_fails( + "$connstr host=example.org sslcertmode=require sslcert=ssl/client.crt" + . sslkey('client.key'), + "host: 'example.org', ca: '': connect with sslcert, no client CA configured", + expected_stderr => qr/client certificates can only be checked if a root certificate store is available/); + +# example.com uses the client CA. +switch_server_cert($node, certfile => 'server-cn-only', cafile => 'root+client_ca'); +# example.com is configured and should require a valid client cert. +$node->connect_fails( + "$connstr host=example.com sslcertmode=disable", + "host: 'example.com', ca: 'root+client_ca.crt': connect fails if no client certificate sent", + expected_stderr => qr/connection requires a valid client certificate/); +$node->connect_ok( + "$connstr host=example.com sslcertmode=require sslcert=ssl/client.crt " . sslkey('client.key'), + "host: 'example.com', ca: 'root+client_ca.crt': connect with sslcert, client certificate sent" +); + +# example.net uses the server CA (which is wrong). +switch_server_cert($node, certfile => 'server-cn-only', cafile => 'root+server_ca'); +# example.net is configured and should require a client cert, but will +# always fail verification. +$node->connect_fails( + "$connstr host=example.net sslcertmode=disable", + "host: 'example.net', ca: 'root+server_ca.crt': connect fails if no client certificate sent", + expected_stderr => qr/connection requires a valid client certificate/); + +$node->connect_fails( + "$connstr host=example.net sslcertmode=require sslcert=ssl/client.crt " + . sslkey('client.key'), + "host: 'example.net', ca: 'root+server_ca.crt': connect with sslcert, client certificate sent", + expected_stderr => qr/unknown ca/); + done_testing(); diff --git a/src/test/ssl/t/004_sni.pl b/src/test/ssl/t/004_sni.pl index 2dd70e7afee..348255e1602 100644 --- a/src/test/ssl/t/004_sni.pl +++ b/src/test/ssl/t/004_sni.pl @@ -66,8 +66,19 @@ $node->connect_fails( "pg.conf: connect fails without intermediate for sslmode=verify-ca", expected_stderr => qr/certificate verify failed/); -# Remove pg_hosts.conf and reload to make sure a missing file is treated like -# an empty file. +# Add an entry in pg_hosts.conf with no default, and reload. Since ssl_sni is +# still 'off' we should still be able to connect using the certificates in +# postgresql.conf +$node->append_conf('pg_hosts.conf', + "example.org server-cn-only.crt server-cn-only.key"); +$node->reload; +$node->connect_ok( + "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require", + "pg.conf: connect with correct server CA cert file sslmode=require"); + +# Turn on SNI support and remove pg_hosts.conf and reload to make sure a +# missing file is treated like an empty file. +$node->append_conf('postgresql.conf', 'ssl_sni = on'); ok(unlink($node->data_dir . '/pg_hosts.conf')); $node->reload; @@ -107,6 +118,10 @@ $node->connect_ok( "$connstr host=example.org sslrootcert=ssl/root_ca.crt sslmode=verify-ca", "pg_hosts.conf: connect to example.org and verify server CA"); +$node->connect_ok( + "$connstr host=Example.ORG sslrootcert=ssl/root_ca.crt sslmode=verify-ca", + "pg_hosts.conf: connect to Example.ORG and verify server CA"); + $node->connect_fails( "$connstr host=example.org sslrootcert=invalid sslmode=verify-ca", "pg_hosts.conf: connect to example.org but without server root cert, sslmode=verify-ca", @@ -121,19 +136,50 @@ $node->connect_ok( "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require", "pg_hosts.conf: connect to default with sslmode=require"); +# Use multiple hostnames for a single configuration +ok(unlink($node->data_dir . '/pg_hosts.conf')); +$node->append_conf('pg_hosts.conf', + "example.org,example.com,example.net server-cn-only+server_ca.crt server-cn-only.key root_ca.crt" +); +$node->reload; + +$node->connect_ok( + "$connstr host=example.org sslrootcert=ssl/root_ca.crt sslmode=verify-ca", + "pg_hosts.conf: connect to example.org and verify server CA"); +$node->connect_ok( + "$connstr host=example.com sslrootcert=ssl/root_ca.crt sslmode=verify-ca", + "pg_hosts.conf: connect to example.com and verify server CA"); +$node->connect_ok( + "$connstr host=example.net sslrootcert=ssl/root_ca.crt sslmode=verify-ca", + "pg_hosts.conf: connect to example.net and verify server CA"); +$node->connect_fails( + "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=example.se", + "pg_hosts.conf: connect to default with sslmode=require", + expected_stderr => qr/unrecognized name/); + +# Add an incorrect entry specifying a default entry combined with hostnames +ok(unlink($node->data_dir . '/pg_hosts.conf')); +$node->append_conf('pg_hosts.conf', + "example.org,*,example.net server-cn-only+server_ca.crt server-cn-only.key root_ca.crt" +); +my $result = $node->restart(fail_ok => 1); +is($result, 0, + 'pg_hosts.conf: restart fails with default entry combined with hostnames' +); + # Modify pg_hosts.conf to no longer have the default host entry. ok(unlink($node->data_dir . '/pg_hosts.conf')); $node->append_conf('pg_hosts.conf', "example.org server-cn-only+server_ca.crt server-cn-only.key root_ca.crt" ); -$node->reload; +$node->restart; # Connecting without a hostname as well as with a hostname which isn't in the # pg_hosts configuration should fail. $node->connect_fails( "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslsni=0", "pg_hosts.conf: connect to default with sslmode=require", - expected_stderr => qr/missing extension/); + expected_stderr => qr/handshake failure/); $node->connect_fails( "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=example.com", "pg_hosts.conf: connect to default with sslmode=require", @@ -145,7 +191,7 @@ ok(unlink($node->data_dir . '/pg_hosts.conf')); $node->append_conf('pg_hosts.conf', 'localhost server-cn-only.crt server-password.key root+client_ca.crt "echo wrongpassword" on' ); -my $result = $node->restart(fail_ok => 1); +$result = $node->restart(fail_ok => 1); is($result, 0, 'pg_hosts.conf: restart fails with password-protected key when using the wrong passphrase command' ); @@ -179,16 +225,21 @@ $node->connect_ok( ); # Test reloading a passphrase protected key without reloading support in the -# passphrase hook. Connecting after restart should succeed but not after the -# following reload. +# passphrase hook. Restarting should not give any errors in the log, but the +# subsequent reload should fail with an error regarding reloading. ok(unlink($node->data_dir . '/pg_hosts.conf')); $node->append_conf('pg_hosts.conf', 'localhost server-cn-only.crt server-password.key root+client_ca.crt "echo secret1" off' ); +my $node_loglocation = -s $node->logfile; $result = $node->restart(fail_ok => 1); is($result, 1, 'pg_hosts.conf: restart succeeds with password-protected key when using the correct passphrase command' ); +my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); +unlike($log, qr/cannot be reloaded because it requires a passphrase/, + 'log reload failure due to passphrase command reloading'); + SKIP: { # Passphrase reloads must be enabled on Windows to succeed even without a @@ -199,19 +250,26 @@ SKIP: "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=localhost", "pg_hosts.conf: connect with correct server CA cert file sslmode=require" ); + # Reloading should fail since the passphrase cannot be reloaded, with an + # error recorded in the log. Since we keep existing contexts around it + # should still work. + $node_loglocation = -s $node->logfile; + $node->reload; + $node->connect_ok( + "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=localhost", + "pg_hosts.conf: connect with correct server CA cert file sslmode=require" + ); + $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); + like($log, + qr/cannot be reloaded because it requires a passphrase/, + 'log reload failure due to passphrase command reloading'); } -$node->reload; -$node->connect_fails( - "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require host=localhost", - "pg_hosts.conf: connect fails since the passphrase protected key cannot be reloaded" -); - # Configure with only non-SNI connections allowed ok(unlink($node->data_dir . '/pg_hosts.conf')); $node->append_conf('pg_hosts.conf', - "no_sni server-cn-only.crt server-cn-only.key"); -$node->reload; + "/no_sni/ server-cn-only.crt server-cn-only.key"); +$node->restart; $node->connect_ok( "$connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslsni=0", @@ -222,68 +280,54 @@ $node->connect_fails( "pg_hosts.conf: only non-SNI connections allowed, connecting with SNI", expected_stderr => qr/unrecognized name/); -# Test client CAs by connecting to hosts in pg_hosts.conf while at the same -# time swapping out default contexts containing different CA configurations. +# Test client CAs # pg_hosts configuration ok(unlink($node->data_dir . '/pg_hosts.conf')); # example.org has an unconfigured CA. $node->append_conf('pg_hosts.conf', - 'example.org server-cn-only.crt server-cn-only.key ""'); + 'example.org server-cn-only.crt server-cn-only.key'); # example.com uses the client CA. $node->append_conf('pg_hosts.conf', 'example.com server-cn-only.crt server-cn-only.key root+client_ca.crt'); # example.net uses the server CA (which is wrong). $node->append_conf('pg_hosts.conf', 'example.net server-cn-only.crt server-cn-only.key root+server_ca.crt'); -$node->reload; +$node->restart; $connstr = "user=ssltestuser dbname=certdb hostaddr=$SERVERHOSTADDR sslmode=require sslsni=1"; -foreach my $default_ca ("", "root+client_ca", "root+server_ca") -{ - # The default CA should, not matter for the purposes of these tests, since - # we connect to the other hosts explicitly. Test with various default CA - # settings to ensure it's isolated from the actual connections. - $ssl_server->switch_server_cert( - $node, - certfile => 'server-cn-only', - cafile => $default_ca); - - # example.org is unconfigured and should fail. - $node->connect_fails( - "$connstr host=example.org sslcertmode=require sslcert=ssl/client.crt " - . $ssl_server->sslkey('client.key'), - "host: 'example.org', ca: '$default_ca': connect with sslcert, no client CA configured", - expected_stderr => qr/unknown ca/); - - # example.com is configured and should require a valid client cert. - $node->connect_fails( - "$connstr host=example.com sslcertmode=disable", - "host: 'example.com', ca: '$default_ca': connect fails if no client certificate sent", - expected_stderr => qr/connection requires a valid client certificate/ - ); +# example.org is unconfigured and should fail. +$node->connect_fails( + "$connstr host=example.org sslcertmode=require sslcert=ssl/client.crt" + . $ssl_server->sslkey('client.key'), + "host: 'example.org', ca: '': connect with sslcert, no client CA configured", + expected_stderr => qr/client certificates can only be checked if a root certificate store is available/); - $node->connect_ok( - "$connstr host=example.com sslrootcert=ssl/root+server_ca.crt sslcertmode=require sslcert=ssl/client.crt " - . $ssl_server->sslkey('client.key'), - "host: 'example.com', ca: '$default_ca': connect with sslcert, client certificate sent" - ); +# example.com is configured and should require a valid client cert. +$node->connect_fails( + "$connstr host=example.com sslcertmode=disable", + "host: 'example.com', ca: 'root+client_ca.crt': connect fails if no client certificate sent", + expected_stderr => qr/connection requires a valid client certificate/); - # example.net is configured and should require a client cert, but will - # always fail verification. - $node->connect_fails( - "$connstr host=example.net sslcertmode=disable", - "host: 'example.net', ca: '$default_ca': connect fails if no client certificate sent", - expected_stderr => qr/connection requires a valid client certificate/ - ); +$node->connect_ok( + "$connstr host=example.com sslcertmode=require sslcert=ssl/client.crt " + . $ssl_server->sslkey('client.key'), + "host: 'example.com', ca: 'root+client_ca.crt': connect with sslcert, client certificate sent" +); - $node->connect_fails( - "$connstr host=example.net sslcertmode=require sslcert=ssl/client.crt " - . $ssl_server->sslkey('client.key'), - "host: 'example.net', ca: '$default_ca': connect with sslcert, client certificate sent", - expected_stderr => qr/unknown ca/); -} +# example.net is configured and should require a client cert, but will +# always fail verification. +$node->connect_fails( + "$connstr host=example.net sslcertmode=disable", + "host: 'example.net', ca: 'root+server_ca.crt': connect fails if no client certificate sent", + expected_stderr => qr/connection requires a valid client certificate/); + +$node->connect_fails( + "$connstr host=example.net sslcertmode=require sslcert=ssl/client.crt " + . $ssl_server->sslkey('client.key'), + "host: 'example.net', ca: 'root+server_ca.crt': connect with sslcert, client certificate sent", + expected_stderr => qr/unknown ca/); done_testing(); -- 2.39.3 (Apple Git-146)