pgwire: bug in the HBA parsing logic

[knz]

There was a bug that might cause rules to mistakenly allow unwanted user logins.
(found while working on #31113)

Security Impact

Any sites using HBA configuration with rules applying to multiple (comma-separated) usernames are affected.

Rules intended to apply to a particular user may not be applied properly, and cause that user to be able to authenticate using a different, possibly less secure authentication method.

The bug applies to CockroachDB versions 19.2 (and possibly 19.1? Unsure.)

Workaround

Rules intended to apply to multiple users should be rewritten as multiple rules side-by-side each applying to just one user.

This problem is also fixed in the next patch release of CockroachDB 19.2 see #43713 and #43714.

Background

The HBA logic is used for rule-based authentication via the cluster setting server.host_based_authentication.configuration.
This defines authentication rules that decide whether to accept or deny SQL connections.

Each rule defines a filter on the username and client IP address. If the username and address both match, the authentication method selected by the rule is applied.

For example, the following HBA configuration requires users foo and bar to always present a valid client certificate, and any other user to present either a cert or a password:

host all foo,bar all cert
host all all all cert-password

This notation foo,bar is equivalent to the following two rules:

host all foo all cert
host all bar all cert
host all all all cert-password

Detail of the bug

The parsing logic for the username column was faulty.
For example, the rule host all foo,bar,baz all cert
was mis-interpreted as:

host all "foo,bar" all cert
host all baz all cert

This was, in effect, requiring cert auth for a user whose name is foo,bar (including comma), and another user with name baz, instead of the desired cert requirement on 3 users foo, bar and baz.

Additionally, the parsing logic for quoted usernames was invalid.
For example, the rule host all foo,"bar" all cert was mis-interpreted as a rule applying to users foo and bar" (where the second username was considered to contain a quote at the end), instead of foo and bar as desired.

[knz]

@kenliu this issue is effectively closed by #43713 which is merged already. Should we issue a patch to 19.1 too?

[kenliu]

let's backport to 19.1 and 2.1 (supported releases)

[knz]

Ok sent PR for 19.1: #43812

(The functionality did not exist in 2.1.)

[knz]

the fix has been backported to 19.1 and 19.2. Should we close this issue now?