From af74ca811d0677f2efe85a1760e0714a1a9a28e2 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 10 Jan 2024 09:45:34 +0200 Subject: [PATCH v7 11/12] WIP: refactor state machine in libpq --- src/interfaces/libpq/fe-connect.c | 447 +++++++++++++---------- src/interfaces/libpq/fe-secure-openssl.c | 12 +- src/interfaces/libpq/libpq-fe.h | 3 +- src/interfaces/libpq/libpq-int.h | 18 +- 4 files changed, 268 insertions(+), 212 deletions(-) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 2025cace2fd..e9c538f3ec8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -399,6 +399,10 @@ static bool connectOptions1(PGconn *conn, const char *conninfo); static bool connectOptions2(PGconn *conn); static int connectDBStart(PGconn *conn); static int connectDBComplete(PGconn *conn); +static bool init_allowed_encryption_methods(PGconn *conn); +static int encryption_negotiation_failed(PGconn *conn); +static bool connection_failed(PGconn *conn); +static bool select_next_encryption_method(PGconn *conn, bool negotiation_failure); static PGPing internal_ping(PGconn *conn); static PGconn *makeEmptyPGconn(void); static void pqFreeCommandQueue(PGcmdQueueEntry *queue); @@ -1685,18 +1689,6 @@ connectOptions2(PGconn *conn) conn->gssencmode); return false; } -#endif -#ifdef USE_SSL - /* GSS is incompatible with direct SSL connections so it requires the - * default postgres style connection ssl negotiation */ - if (strcmp(conn->gssencmode, "require") == 0 && - strcmp(conn->sslnegotiation, "postgres") != 0) - { - conn->status = CONNECTION_BAD; - libpq_append_conn_error(conn, "gssencmode value \"%s\" invalid when Direct SSL negotiation is enabled", - conn->gssencmode); - return false; - } #endif } else @@ -2826,16 +2818,9 @@ keep_going: /* We will come back to here until there is */ conn->pversion = PG_PROTOCOL(3, 0); conn->send_appname = true; -#ifdef USE_SSL - /* initialize these values based on SSL mode */ - conn->allow_ssl_try = (conn->sslmode[0] != 'd'); /* "disable" */ - conn->wait_ssl_try = (conn->sslmode[0] == 'a'); /* "allow" */ - /* direct ssl is incompatible with "allow" or "disabled" ssl */ - conn->allow_direct_ssl_try = conn->allow_ssl_try && !conn->wait_ssl_try && (conn->sslnegotiation[0] != 'p'); -#endif -#ifdef ENABLE_GSS - conn->try_gss = (conn->gssencmode[0] != 'd'); /* "disable" */ -#endif + conn->failed_enc_methods = 0; + conn->current_enc_method = 0; + conn->allowed_enc_methods = 0; reset_connection_state_machine = false; need_new_connection = true; } @@ -2861,6 +2846,32 @@ keep_going: /* We will come back to here until there is need_new_connection = false; } +#define ENCRYPTION_NEGOTIATION_FAILED() \ + do { \ + switch (encryption_negotiation_failed(conn)) \ + { \ + case 0: \ + goto error_return; \ + case 1: \ + conn->status = CONNECTION_MADE; \ + return PGRES_POLLING_WRITING; \ + case 2: \ + need_new_connection = true; \ + goto keep_going; \ + } \ + } while(0); + +#define CONNECTION_FAILED() \ + do { \ + if (connection_failed(conn)) \ + { \ + need_new_connection = true; \ + goto keep_going; \ + } \ + else \ + goto error_return; \ + } while(0); + /* Now try to advance the state machine for this connection */ switch (conn->status) { @@ -3175,18 +3186,6 @@ keep_going: /* We will come back to here until there is goto error_return; } - /* - * Make sure we can write before advancing to next step. - */ - conn->status = CONNECTION_MADE; - return PGRES_POLLING_WRITING; - } - - case CONNECTION_MADE: - { - char *startpacket; - int packetlen; - /* * Implement requirepeer check, if requested and it's a * Unix-domain socket. @@ -3235,30 +3234,31 @@ keep_going: /* We will come back to here until there is #endif /* WIN32 */ } - if (conn->raddr.addr.ss_family == AF_UNIX) - { - /* Don't request SSL or GSSAPI over Unix sockets */ -#ifdef USE_SSL - conn->allow_ssl_try = false; -#endif -#ifdef ENABLE_GSS - conn->try_gss = false; -#endif - } + /* Choose encryption method to try first */ + if (!init_allowed_encryption_methods(conn)) + goto error_return; + + /* + * Make sure we can write before advancing to next step. + */ + conn->status = CONNECTION_MADE; + return PGRES_POLLING_WRITING; + } + + case CONNECTION_MADE: + { + char *startpacket; + int packetlen; #ifdef ENABLE_GSS /* - * If GSSAPI encryption is enabled, then call - * pg_GSS_have_cred_cache() which will return true if we can - * acquire credentials (and give us a handle to use in - * conn->gcred), and then send a packet to the server asking - * for GSSAPI Encryption (and skip past SSL negotiation and - * regular startup below). + * If GSSAPI encryption is enabled, send a packet to the + * server asking for GSSAPI Encryption and proceed with GSSAPI + * handshake. We will come back here after GSSAPI encryption + * has been established, with conn->gctx set. */ - if (conn->try_gss && !conn->gctx) - conn->try_gss = pg_GSS_have_cred_cache(&conn->gcred); - if (conn->try_gss && !conn->gctx) + if (conn->current_enc_method == ENC_GSSAPI && !conn->gctx) { ProtocolVersion pv = pg_hton32(NEGOTIATE_GSS_CODE); @@ -3273,12 +3273,6 @@ keep_going: /* We will come back to here until there is conn->status = CONNECTION_GSS_STARTUP; return PGRES_POLLING_READING; } - else if (!conn->gctx && conn->gssencmode[0] == 'r') - { - libpq_append_conn_error(conn, - "GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)"); - goto error_return; - } #endif #ifdef USE_SSL @@ -3294,39 +3288,22 @@ keep_going: /* We will come back to here until there is goto error_return; /* - * If SSL is enabled and direct SSL connections are enabled - * and we haven't already established an SSL connection (or - * already tried a direct connection and failed or succeeded) - * then try just enabling SSL directly. - * - * If we fail then we'll either fail the connection (if - * sslnegotiation is set to requiredirect or turn - * allow_direct_ssl_try to false + * If direct SSL is enabled, jump right into SSL handshake. + * We will come back here after SSL encryption has been established, + * with ssl_in_use set. */ - if (conn->allow_ssl_try - && !conn->wait_ssl_try - && conn->allow_direct_ssl_try - && !conn->ssl_in_use -#ifdef ENABLE_GSS - && !conn->gssenc -#endif - ) + if (conn->current_enc_method == ENC_DIRECT_SSL && !conn->ssl_in_use) { conn->status = CONNECTION_SSL_STARTUP; return PGRES_POLLING_WRITING; } /* - * If SSL is enabled and we haven't already got encryption of - * some sort running, request SSL instead of sending the - * startup message. + * If negotiated SSL is enabled, request SSL and proceed with + * SSL handshake. We will come back here after SSL encryption + * has been established, with ssl_in_use set. */ - if (conn->allow_ssl_try && !conn->wait_ssl_try && - !conn->ssl_in_use -#ifdef ENABLE_GSS - && !conn->gssenc -#endif - ) + if (conn->current_enc_method == ENC_NEGOTIATED_SSL && !conn->ssl_in_use) { ProtocolVersion pv; @@ -3351,8 +3328,11 @@ keep_going: /* We will come back to here until there is #endif /* USE_SSL */ /* - * Build the startup packet. + * We have now established encryption, or we are happy to + * proceed without. */ + + /* Build the startup packet. */ startpacket = pqBuildStartupPacket3(conn, &packetlen, EnvironmentOptions); if (!startpacket) @@ -3393,10 +3373,9 @@ keep_going: /* We will come back to here until there is /* * On first time through, get the postmaster's response to our * SSL negotiation packet. If we are trying a direct ssl - * connection skip reading the negotiation packet and go - * straight to initiating an ssl connection. + * connection, go straight to initiating ssl. */ - if (!conn->ssl_in_use && !conn->allow_direct_ssl_try) + if (!conn->ssl_in_use && conn->current_enc_method == ENC_NEGOTIATED_SSL) { /* * We use pqReadData here since it has the logic to @@ -3441,19 +3420,8 @@ keep_going: /* We will come back to here until there is /* mark byte consumed */ conn->inStart = conn->inCursor; /* OK to do without SSL? */ - if (conn->sslmode[0] == 'r' || /* "require" */ - conn->sslmode[0] == 'v') /* "verify-ca" or - * "verify-full" */ - { - /* Require SSL, but server does not want it */ - libpq_append_conn_error(conn, "server does not support SSL, but SSL was required"); - goto error_return; - } - /* Otherwise, proceed with normal startup */ - conn->allow_ssl_try = false; /* We can proceed using this connection */ - conn->status = CONNECTION_MADE; - return PGRES_POLLING_WRITING; + ENCRYPTION_NEGOTIATION_FAILED(); } else if (SSLok == 'E') { @@ -3503,32 +3471,7 @@ keep_going: /* We will come back to here until there is if (pollres == PGRES_POLLING_FAILED) { /* Failed direct ssl connection, possibly try a new connection with postgres negotiation */ - if (conn->allow_direct_ssl_try) - { - /* if it's requiredirect then it's a hard failure */ - if (conn->sslnegotiation[0] == 'r') - goto error_return; - /* otherwise only retry using postgres connection */ - conn->allow_direct_ssl_try = false; - need_new_connection = true; - goto keep_going; - } - - /* - * Failed ... if sslmode is "prefer" then do a non-SSL - * retry - */ - if (conn->sslmode[0] == 'p' /* "prefer" */ - && conn->allow_ssl_try /* redundant? */ - && !conn->wait_ssl_try) /* redundant? */ - { - /* only retry once */ - conn->allow_ssl_try = false; - need_new_connection = true; - goto keep_going; - } - /* Else it's a hard failure */ - goto error_return; + CONNECTION_FAILED(); } /* Else, return POLLING_READING or POLLING_WRITING status */ return pollres; @@ -3547,7 +3490,7 @@ keep_going: /* We will come back to here until there is * If we haven't yet, get the postmaster's response to our * negotiation packet */ - if (conn->try_gss && !conn->gctx) + if (!conn->gctx) { char gss_ok; int rdresult = pqReadData(conn); @@ -3571,9 +3514,7 @@ keep_going: /* We will come back to here until there is * error message on retry). Server gets fussy if we * don't hang up the socket, though. */ - conn->try_gss = false; - need_new_connection = true; - goto keep_going; + CONNECTION_FAILED(); } /* mark byte consumed */ @@ -3581,17 +3522,8 @@ keep_going: /* We will come back to here until there is if (gss_ok == 'N') { - /* Server doesn't want GSSAPI; fall back if we can */ - if (conn->gssencmode[0] == 'r') - { - libpq_append_conn_error(conn, "server doesn't support GSSAPI encryption, but it was required"); - goto error_return; - } - - conn->try_gss = false; /* We can proceed using this connection */ - conn->status = CONNECTION_MADE; - return PGRES_POLLING_WRITING; + ENCRYPTION_NEGOTIATION_FAILED(); } else if (gss_ok != 'G') { @@ -3623,18 +3555,7 @@ keep_going: /* We will come back to here until there is } else if (pollres == PGRES_POLLING_FAILED) { - if (conn->gssencmode[0] == 'p') - { - /* - * We failed, but we can retry on "prefer". Have to - * drop the current connection to do so, though. - */ - conn->try_gss = false; - need_new_connection = true; - goto keep_going; - } - /* Else it's a hard failure */ - goto error_return; + CONNECTION_FAILED(); } /* Else, return POLLING_READING or POLLING_WRITING status */ return pollres; @@ -3810,55 +3731,7 @@ keep_going: /* We will come back to here until there is /* Check to see if we should mention pgpassfile */ pgpassfileWarning(conn); -#ifdef ENABLE_GSS - - /* - * If gssencmode is "prefer" and we're using GSSAPI, retry - * without it. - */ - if (conn->gssenc && conn->gssencmode[0] == 'p') - { - /* only retry once */ - conn->try_gss = false; - need_new_connection = true; - goto keep_going; - } -#endif - -#ifdef USE_SSL - - /* - * if sslmode is "allow" and we haven't tried an SSL - * connection already, then retry with an SSL connection - */ - if (conn->sslmode[0] == 'a' /* "allow" */ - && !conn->ssl_in_use - && conn->allow_ssl_try - && conn->wait_ssl_try) - { - /* only retry once */ - conn->wait_ssl_try = false; - need_new_connection = true; - goto keep_going; - } - - /* - * if sslmode is "prefer" and we're in an SSL connection, - * then do a non-SSL retry - */ - if (conn->sslmode[0] == 'p' /* "prefer" */ - && conn->ssl_in_use - && conn->allow_ssl_try /* redundant? */ - && !conn->wait_ssl_try) /* redundant? */ - { - /* only retry once */ - conn->allow_ssl_try = false; - need_new_connection = true; - goto keep_going; - } -#endif - - goto error_return; + CONNECTION_FAILED(); } else if (beresp == PqMsg_NegotiateProtocolVersion) { @@ -4298,6 +4171,178 @@ error_return: return PGRES_POLLING_FAILED; } +static bool +init_allowed_encryption_methods(PGconn *conn) +{ + if (conn->raddr.addr.ss_family == AF_UNIX) + { + /* Don't request SSL or GSSAPI over Unix sockets */ + conn->allowed_enc_methods &= ~(ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL | ENC_GSSAPI); + + /* to give a better error message */ + /* XXX: we probably should not do this. sslmode=require works differently */ + if (conn->gssencmode[0] == 'r') + { + libpq_append_conn_error(conn, + "GSSAPI encryption required but it is not supported over a local socket)"); + conn->allowed_enc_methods = 0; + conn->current_enc_method = ENC_ERROR; + return false; + } + + conn->allowed_enc_methods = ENC_PLAINTEXT; + conn->current_enc_method = ENC_PLAINTEXT; + return true; + } + + /* initialize these values based on sslmode and gssencmode */ + conn->allowed_enc_methods = 0; + +#ifdef USE_SSL + /* sslmode anything but 'disable', and GSSAPI not required */ + if (conn->sslmode[0] != 'd' && conn->gssencmode[0] != 'r') + { + if (conn->sslnegotiation[0] == 'p') + conn->allowed_enc_methods |= ENC_NEGOTIATED_SSL; + else if (conn->sslnegotiation[0] == 'd') + conn->allowed_enc_methods |= ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL; + else if (conn->sslnegotiation[0] == 'r') + conn->allowed_enc_methods |= ENC_DIRECT_SSL; + } +#endif + +#ifdef ENABLE_GSS + if (conn->gssencmode[0] != 'd') + conn->allowed_enc_methods |= ENC_GSSAPI; +#endif + + if ((conn->sslmode[0] == 'd' || conn->sslmode[0] == 'p' || conn->sslmode[0] == 'a') && + (conn->gssencmode[0] == 'd' || conn->gssencmode[0] == 'p')) + { + conn->allowed_enc_methods |= ENC_PLAINTEXT; + } + + return select_next_encryption_method(conn, false); +} + +static int +encryption_negotiation_failed(PGconn *conn) +{ + Assert((conn->failed_enc_methods & conn->current_enc_method) == 0); + conn->failed_enc_methods |= conn->current_enc_method; + + if (select_next_encryption_method(conn, true)) + { + if (conn->current_enc_method == ENC_DIRECT_SSL) + return 2; + else + return 1; + } + else + return 0; +} + +static bool +connection_failed(PGconn *conn) +{ + Assert((conn->failed_enc_methods & conn->current_enc_method) == 0); + conn->failed_enc_methods |= conn->current_enc_method; + + /* If the server reported an error after the SSL handshake, no point in retrying with negotiated vs direct SSL */ + if ((conn->current_enc_method & (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL)) != 0 && conn->ssl_handshake_started) + conn->failed_enc_methods |= (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL) & conn->allowed_enc_methods; + else + conn->failed_enc_methods |= conn->current_enc_method; + + return select_next_encryption_method(conn, false); +} + +static bool +select_next_encryption_method(PGconn *conn, bool negotiation_failure) +{ + int remaining_methods; + + remaining_methods = conn->allowed_enc_methods & ~conn->failed_enc_methods; + + /* + * Try GSSAPI before SSL + */ +#ifdef ENABLE_GSS + if ((remaining_methods & ENC_GSSAPI) != 0) + { + /* + * If GSSAPI encryption is enabled, then call + * pg_GSS_have_cred_cache() which will return true if we can + * acquire credentials (and give us a handle to use in + * conn->gcred), and then send a packet to the server asking + * for GSSAPI Encryption (and skip past SSL negotiation and + * regular startup below). + */ + if (!conn->gctx) + { + if (!pg_GSS_have_cred_cache(&conn->gcred)) + { + conn->allowed_enc_methods &= ~ENC_GSSAPI; + remaining_methods &= ~ENC_GSSAPI; + + if (conn->gssencmode[0] == 'r') + { + libpq_append_conn_error(conn, + "GSSAPI encryption required but no credential cache"); + } + } + } + if ((remaining_methods & ENC_GSSAPI) != 0) + { + conn->current_enc_method = ENC_GSSAPI; + return true; + } + } +#endif + + /* With sslmode=allow, try plaintext connection before SSL. */ + if (conn->sslmode[0] == 'a' && (remaining_methods & ENC_PLAINTEXT) != 0) + { + conn->current_enc_method = ENC_PLAINTEXT; + return true; + } + + /* + * Try SSL. If enabled, try direct SSL. Unless we have a valid TCP + * connection that failed negotiating GSSAPI encryption; in that case we + * prefer to reuse the connection with negotiated SSL, instead of + * reconnecting to do direct SSL. The point of direct SSL is to avoid the + * roundtrip from the negotiation, but reconnecting would also incur a + * roundtrip. + */ + if (negotiation_failure && (remaining_methods & ENC_NEGOTIATED_SSL) != 0) + { + conn->current_enc_method = ENC_NEGOTIATED_SSL; + return true; + } + + if ((remaining_methods & ENC_DIRECT_SSL) != 0) + { + conn->current_enc_method = ENC_DIRECT_SSL; + return true; + } + + if ((remaining_methods & ENC_NEGOTIATED_SSL) != 0) + { + conn->current_enc_method = ENC_NEGOTIATED_SSL; + return true; + } + + if (conn->sslmode[0] != 'a' && (remaining_methods & ENC_PLAINTEXT) != 0) + { + conn->current_enc_method = ENC_PLAINTEXT; + return true; + } + + /* No more options */ + conn->current_enc_method = ENC_ERROR; + return false; +} /* * internal_ping diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index fcd402dc6b7..4a12c279350 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1502,6 +1502,7 @@ open_client_SSL(PGconn *conn) SOCK_ERRNO_SET(0); ERR_clear_error(); r = SSL_connect(conn->ssl); + if (r <= 0) { int save_errno = SOCK_ERRNO; @@ -1605,7 +1606,7 @@ open_client_SSL(PGconn *conn) /* * We already checked the server certificate in initialize_SSL() using - * SSL_CTX_set_verify(), if root.crt exists. + * SSL_set_verify(), if root.crt exists. */ /* get server certificate */ @@ -1649,6 +1650,7 @@ pgtls_close(PGconn *conn) SSL_free(conn->ssl); conn->ssl = NULL; conn->ssl_in_use = false; + conn->ssl_handshake_started = false; destroy_needed = true; } @@ -1672,7 +1674,7 @@ pgtls_close(PGconn *conn) { /* * In the non-SSL case, just remove the crypto callbacks if the - * connection has then loaded. This code path has no dependency on + * connection has loaded them. This code path has no dependency on * any pending SSL calls. */ if (conn->crypto_loaded) @@ -1859,9 +1861,10 @@ static BIO_METHOD *my_bio_methods; static int my_sock_read(BIO *h, char *buf, int size) { + PGconn *conn = (PGconn *) BIO_get_app_data(h); int res; - res = pqsecure_raw_read((PGconn *) BIO_get_app_data(h), buf, size); + res = pqsecure_raw_read(conn, buf, size); BIO_clear_retry_flags(h); if (res < 0) { @@ -1883,6 +1886,9 @@ my_sock_read(BIO *h, char *buf, int size) } } + if (res > 0) + conn->ssl_handshake_started = true; + return res; } diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 3a4e8ae4080..69632a48e26 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -72,14 +72,13 @@ typedef enum CONNECTION_AUTH_OK, /* Received authentication; waiting for * backend startup. */ CONNECTION_SETENV, /* This state is no longer used. */ - CONNECTION_SSL_STARTUP, /* Negotiating SSL. */ + CONNECTION_SSL_STARTUP, /* Performing SSL handshake. */ CONNECTION_NEEDED, /* Internal state: connect() needed */ CONNECTION_CHECK_WRITABLE, /* Checking if session is read-write. */ CONNECTION_CONSUME, /* Consuming any extra messages. */ CONNECTION_GSS_STARTUP, /* Negotiating GSSAPI. */ CONNECTION_CHECK_TARGET, /* Checking target server properties. */ CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */ - CONNECTION_DIRECT_SSL_STARTUP /* Starting SSL without PG Negotiation. */ } ConnStatusType; typedef enum diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 6daf34b9afe..05a7256410f 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -76,6 +76,7 @@ typedef struct #include #include + #ifndef OPENSSL_NO_ENGINE #define USE_SSL_ENGINE #endif @@ -231,6 +232,12 @@ typedef enum PGASYNC_PIPELINE_IDLE, /* "Idle" between commands in pipeline mode */ } PGAsyncStatusType; +#define ENC_ERROR 0 +#define ENC_DIRECT_SSL 0x01 +#define ENC_GSSAPI 0x02 +#define ENC_NEGOTIATED_SSL 0x04 +#define ENC_PLAINTEXT 0x08 + /* Target server type (decoded value of target_session_attrs) */ typedef enum { @@ -545,17 +552,17 @@ struct pg_conn void *sasl_state; int scram_sha_256_iterations; + uint8 allowed_enc_methods; + uint8 failed_enc_methods; + uint8 current_enc_method; + /* SSL structures */ bool ssl_in_use; + bool ssl_handshake_started; bool ssl_cert_requested; /* Did the server ask us for a cert? */ bool ssl_cert_sent; /* Did we send one in reply? */ #ifdef USE_SSL - bool allow_direct_ssl_try; /* Try to make a direct SSL connection - * without an "SSL negotiation packet" */ - bool allow_ssl_try; /* Allowed to try SSL negotiation */ - bool wait_ssl_try; /* Delay SSL negotiation until after - * attempting normal connection */ #ifdef USE_OPENSSL SSL *ssl; /* SSL status, if have SSL connection */ X509 *peer; /* X509 cert of server */ @@ -578,7 +585,6 @@ struct pg_conn gss_name_t gtarg_nam; /* GSS target name */ /* The following are encryption-only */ - bool try_gss; /* GSS attempting permitted */ bool gssenc; /* GSS encryption is usable */ gss_cred_id_t gcred; /* GSS credential temp storage. */ -- 2.39.2