commit dcae9a7c1cef593055c932c85244852a119b0313 Author: Jacob Champion Date: Thu Jun 23 15:38:12 2022 -0700 squash! libpq: let client reject unexpected auth methods - Add "none" and method negation with "!". - Test unknown methods to round out coverage. TODO: should "none,scram-sha-256" allow an incomplete SCRAM handshake? diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 00990ce47d..cc831158b7 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1233,6 +1233,20 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname for the connection to succeed. By default, any authentication method is accepted, and the server is free to skip authentication altogether. + + Methods may be negated with the addition of a ! + prefix, in which case the server must not attempt + the listed method; any other method is accepted, and the server is free + not to authenticate the client at all. If a comma-separated list is + provided, the server may not attempt any of the + listed negated methods. Negated and non-negated forms may not be + combined in the same setting. + + + As a final special case, the none method requires the + server not to use an authentication challenge. (It may also be negated, + to require some form of authentication.) + The following methods may be specified: @@ -1286,6 +1300,17 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + + none + + + The server must not prompt the client for an authentication + exchange. (This does not prohibit client certificate authentication + via TLS, nor GSS authentication via its encrypted transport.) + + + diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 59c3575cc3..f789bc7ec3 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -902,9 +902,14 @@ check_expected_areq(AuthRequest areq, PGconn *conn) { case AUTH_REQ_OK: /* - * Check to make sure we've actually finished our exchange. + * Check to make sure we've actually finished our exchange (or + * else that the user has allowed an authentication-less + * connection). + * + * TODO: how should !auth_required interact with an incomplete + * SCRAM exchange? */ - if (conn->client_finished_auth) + if (!conn->auth_required || conn->client_finished_auth) break; /* diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index f2579ff7ee..7d5bf337f6 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1265,17 +1265,65 @@ connectOptions2(PGconn *conn) if (conn->require_auth) { char *s = conn->require_auth; - bool more = true; + bool first, more; + bool negated = false; + + /* + * By default, start from an empty set of allowed options and add to it. + */ + conn->auth_required = true; + conn->allowed_auth_methods = 0; - while (more) + for (first = true, more = true; more; first = false) { - char *method; + char *method, *part; uint32 bits; - method = parse_comma_separated_list(&s, &more); - if (method == NULL) + part = parse_comma_separated_list(&s, &more); + if (part == NULL) goto oom_error; + /* + * Check for negation, e.g. '!password'. If one element is negated, + * they all have to be. + */ + method = part; + if (*method == '!') + { + if (first) + { + /* + * Switch to a permissive set of allowed options, and + * subtract from it. + */ + conn->auth_required = false; + conn->allowed_auth_methods = -1; + } + else if (!negated) + { + conn->status = CONNECTION_BAD; + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("negative require_auth method \"%s\" cannot be mixed with non-negative methods"), + method); + + free(part); + return false; + } + + negated = true; + method++; + } + else if (negated) + { + conn->status = CONNECTION_BAD; + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("require_auth method \"%s\" cannot be mixed with negative methods"), + method); + + free(part); + return false; + } + if (strcmp(method, "password") == 0) { bits = (1 << AUTH_REQ_PASSWORD); @@ -1301,6 +1349,31 @@ connectOptions2(PGconn *conn) bits |= (1 << AUTH_REQ_SASL_CONT); bits |= (1 << AUTH_REQ_SASL_FIN); } + else if (strcmp(method, "none") == 0) + { + /* + * Special case: let the user explicitly allow (or disallow) + * connections where the server does not send an explicit + * authentication challenge, such as "trust" and "cert" auth. + */ + if (negated) /* "!none" */ + { + if (conn->auth_required) + goto duplicate; + + conn->auth_required = true; + } + else /* "none" */ + { + if (!conn->auth_required) + goto duplicate; + + conn->auth_required = false; + } + + free(part); + continue; /* avoid the bitmask manipulation below */ + } else { conn->status = CONNECTION_BAD; @@ -1308,27 +1381,41 @@ connectOptions2(PGconn *conn) libpq_gettext("invalid require_auth method: \"%s\""), method); - free(method); + free(part); return false; } - /* - * Sanity check; a duplicated method probably indicates a typo in a - * setting where typos are extremely risky. - */ - if ((conn->allowed_auth_methods & bits) == bits) + /* Update the bitmask. */ + if (negated) { - conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("require_auth method \"%s\" is specified more than once"), - method); + if ((conn->allowed_auth_methods & bits) == 0) + goto duplicate; - free(method); - return false; + conn->allowed_auth_methods &= ~bits; } + else + { + if ((conn->allowed_auth_methods & bits) == bits) + goto duplicate; + + conn->allowed_auth_methods |= bits; + } + + free(part); + continue; - conn->allowed_auth_methods |= bits; - free(method); +duplicate: + /* + * A duplicated method probably indicates a typo in a setting where + * typos are extremely risky. + */ + conn->status = CONNECTION_BAD; + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("require_auth method \"%s\" is specified more than once"), + part); + + free(part); + return false; } } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 6216af7f87..3278196eea 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -455,6 +455,7 @@ struct pg_conn bool write_failed; /* have we had a write failure on sock? */ char *write_err_msg; /* write error message, or NULL if OOM */ + bool auth_required; /* require an authentication challenge from the server? */ uint32 allowed_auth_methods; /* bitmask of acceptable AuthRequest codes */ /* Transient state needed while establishing connection */ diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index dd27092134..473a93d6db 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -82,12 +82,12 @@ test_role($node, 'scram_role', 'trust', 0, test_role($node, 'md5_role', 'trust', 0, log_unlike => [qr/connection authenticated:/]); -# All require_auth options should fail. +# All positive require_auth options should fail... $node->connect_fails("user=scram_role require_auth=gss", "GSS authentication can be required: fails with trust auth", expected_stderr => qr/server did not complete authentication/); $node->connect_fails("user=scram_role require_auth=sspi", - "GSS authentication can be required: fails with trust auth", + "SSPI authentication can be required: fails with trust auth", expected_stderr => qr/server did not complete authentication/); $node->connect_fails("user=scram_role require_auth=password", "password authentication can be required: fails with trust auth", @@ -102,6 +102,54 @@ $node->connect_fails("user=scram_role require_auth=password,scram-sha-256", "multiple authentication types can be required: fails with trust auth", expected_stderr => qr/server did not complete authentication/); +# ...and negative require_auth options should succeed. +$node->connect_ok("user=scram_role require_auth=!gss", + "GSS authentication can be forbidden: succeeds with trust auth"); +$node->connect_ok("user=scram_role require_auth=!sspi", + "SSPI authentication can be forbidden: succeeds with trust auth"); +$node->connect_ok("user=scram_role require_auth=!password", + "password authentication can be forbidden: succeeds with trust auth"); +$node->connect_ok("user=scram_role require_auth=!md5", + "md5 authentication can be forbidden: succeeds with trust auth"); +$node->connect_ok("user=scram_role require_auth=!scram-sha-256", + "SCRAM authentication can be forbidden: succeeds with trust auth"); +$node->connect_ok("user=scram_role require_auth=!password,!scram-sha-256", + "multiple authentication types can be forbidden: succeeds with trust auth"); + +# require_auth=[!]none should interact correctly with trust auth. +$node->connect_ok("user=scram_role require_auth=none", + "all authentication can be forbidden: succeeds with trust auth"); +$node->connect_fails("user=scram_role require_auth=!none", + "any authentication can be required: fails with trust auth", + expected_stderr => qr/server did not complete authentication/); + +# Negative and positive require_auth options can't be mixed. +$node->connect_fails("user=scram_role require_auth=scram-sha-256,!md5", + "negative require_auth methods can't be mixed with positive", + expected_stderr => qr/negative require_auth method "!md5" cannot be mixed with non-negative methods/); +$node->connect_fails("user=scram_role require_auth=!password,scram-sha-256", + "positive require_auth methods can't be mixed with negative", + expected_stderr => qr/require_auth method "scram-sha-256" cannot be mixed with negative methods/); + +# require_auth methods can't be duplicated. +$node->connect_fails("user=scram_role require_auth=password,md5,password", + "require_auth methods can't be duplicated: positive case", + expected_stderr => qr/require_auth method "password" is specified more than once/); +$node->connect_fails("user=scram_role require_auth=!password,!md5,!password", + "require_auth methods can't be duplicated: negative case", + expected_stderr => qr/require_auth method "!password" is specified more than once/); +$node->connect_fails("user=scram_role require_auth=none,md5,none", + "require_auth methods can't be duplicated: none case", + expected_stderr => qr/require_auth method "none" is specified more than once/); +$node->connect_fails("user=scram_role require_auth=!none,!md5,!none", + "require_auth methods can't be duplicated: !none case", + expected_stderr => qr/require_auth method "!none" is specified more than once/); + +# Unknown require_auth methods are caught. +$node->connect_fails("user=scram_role require_auth=none,abcdefg", + "unknown require_auth methods are rejected", + expected_stderr => qr/invalid require_auth method: "abcdefg"/); + # For plain "password" method, all users should also be able to connect. reset_pg_hba($node, 'password'); test_role($node, 'scram_role', 'password', 0, @@ -114,16 +162,29 @@ test_role($node, 'md5_role', 'password', 0, # require_auth should succeed with a plaintext password... $node->connect_ok("user=scram_role require_auth=password", "password authentication can be required: works with password auth"); +$node->connect_ok("user=scram_role require_auth=!none", + "any authentication can be required: works with password auth"); $node->connect_ok("user=scram_role require_auth=scram-sha-256,password,md5", "multiple authentication types can be required: works with password auth"); -# ...and fail for other auth types. +# ...fail for other auth types... $node->connect_fails("user=scram_role require_auth=md5", "md5 authentication can be required: fails with password auth", expected_stderr => qr/server requested a cleartext password/); $node->connect_fails("user=scram_role require_auth=scram-sha-256", "SCRAM authentication can be required: fails with password auth", expected_stderr => qr/server requested a cleartext password/); +$node->connect_fails("user=scram_role require_auth=none", + "all authentication can be forbidden: fails with password auth", + expected_stderr => qr/server requested a cleartext password/); + +# ...and allow password authentication to be prohibited. +$node->connect_fails("user=scram_role require_auth=!password", + "password authentication can be forbidden: fails with password auth", + expected_stderr => qr/server requested a cleartext password/); +$node->connect_fails("user=scram_role require_auth=!password,!md5,!scram-sha-256", + "multiple authentication types can be forbidden: fails with password auth", + expected_stderr => qr/server requested a cleartext password/); # For "scram-sha-256" method, user "scram_role" should be able to connect. reset_pg_hba($node, 'scram-sha-256'); @@ -141,16 +202,29 @@ test_role($node, 'md5_role', 'scram-sha-256', 2, # require_auth should succeed with SCRAM... $node->connect_ok("user=scram_role require_auth=scram-sha-256", "SCRAM authentication can be required: works with SCRAM auth"); +$node->connect_ok("user=scram_role require_auth=!none", + "any authentication can be required: works with SCRAM auth"); $node->connect_ok("user=scram_role require_auth=password,scram-sha-256,md5", "multiple authentication types can be required: works with SCRAM auth"); -# ...and fail for other auth types. +# ...fail for other auth types... $node->connect_fails("user=scram_role require_auth=password", "password authentication can be required: fails with SCRAM auth", expected_stderr => qr/server requested SASL authentication/); $node->connect_fails("user=scram_role require_auth=md5", "md5 authentication can be required: fails with SCRAM auth", expected_stderr => qr/server requested SASL authentication/); +$node->connect_fails("user=scram_role require_auth=none", + "all authentication can be forbidden: fails with SCRAM auth", + expected_stderr => qr/server requested SASL authentication/); + +# ...and allow SCRAM authentication to be prohibited. +$node->connect_fails("user=scram_role require_auth=!scram-sha-256", + "SCRAM authentication can be forbidden: fails with SCRAM auth", + expected_stderr => qr/server requested SASL authentication/); +$node->connect_fails("user=scram_role require_auth=!password,!md5,!scram-sha-256", + "multiple authentication types can be forbidden: fails with SCRAM auth", + expected_stderr => qr/server requested SASL authentication/); # Test that bad passwords are rejected. $ENV{"PGPASSWORD"} = 'badpass'; @@ -171,16 +245,29 @@ test_role($node, 'md5_role', 'md5', 0, # require_auth should succeed with MD5... $node->connect_ok("user=md5_role require_auth=md5", "MD5 authentication can be required: works with MD5 auth"); +$node->connect_ok("user=md5_role require_auth=!none", + "any authentication can be required: works with MD5 auth"); $node->connect_ok("user=md5_role require_auth=md5,scram-sha-256,password", "multiple authentication types can be required: works with MD5 auth"); -# ...and fail for other auth types. +# ...fail for other auth types... $node->connect_fails("user=md5_role require_auth=password", "password authentication can be required: fails with MD5 auth", expected_stderr => qr/server requested a hashed password/); $node->connect_fails("user=md5_role require_auth=scram-sha-256", "SCRAM authentication can be required: fails with MD5 auth", expected_stderr => qr/server requested a hashed password/); +$node->connect_fails("user=md5_role require_auth=none", + "all authentication can be forbidden: fails with MD5 auth", + expected_stderr => qr/server requested a hashed password/); + +# ...and allow MD5 authentication to be prohibited. +$node->connect_fails("user=md5_role require_auth=!md5", + "password authentication can be forbidden: fails with MD5 auth", + expected_stderr => qr/server requested a hashed password/); +$node->connect_fails("user=md5_role require_auth=!password,!md5,!scram-sha-256", + "multiple authentication types can be forbidden: fails with MD5 auth", + expected_stderr => qr/server requested a hashed password/); # Tests for channel binding without SSL. # Using the password authentication method; channel binding can't work