Merge #32892

32892: pgwire: support hba.conf auth configuration r=mjibson a=mjibson

The upcoming GSSAPI support requires that we have configurable
authentication. Today, pgwire auth uses certs if a client has a TLS client
cert, otherwise it uses a password. Adding GSSAPI as a third kind of login
forces us to allow administrators to configure how their auth should
function. Postgres already has a well documented and straightforward
pg_hba.conf specification. We have chosen to implement some of it in
order to make transition easy, and because it is a good format.

The hba package is a hba.conf file parser. The parser is implemented
in ragel which is a state machine generator from regular expressions to
actions. This format is complicated enough that writing a parser by hand
(due to, for example, the different kinds of strings and IP addresses)
is annoying, and ragel is able to accomplish that work with a simpler
format. It is not hooked up to the Makefile so it is possible the .rl
file could be out-of-sync with its generated .go file, but we don't
expect this file to change often and that is an acceptable risk.

A new cluster setting "server.hba_conf" has been added. The default
(empty string) preserves the old behavior of cert-then-password. We
have some differences from Postgres' hba.conf (although we can easily
expand to support more as needed). We only support the 'host' connection
method and database must be 'all' (since our database security mechanism
is somewhat different than postgres'. We do not support the @ or +
modifiers. Addresses must be IPs, or 'all', but arbitrary hostnames
or domains are unsupported. The auth methods we support are cert and
password which work the same as postgres. In addition, the cert-password
method does the cockroach default of cert-then-password. Thus, "host
all all all cert-password" is a configuration that is identical to our
unchanged default auth method.

Root is hard coded to require a certificate, preventing users from ever
setting a hba.conf file accidentally preventing all logins.

Release note (sql change): add support for configuring authentication
via a hba.conf cluster setting.

Co-authored-by: Matt Jibson <[email protected]>

docs/generated/settings/settings.html

@@ -52,6 +52,7 @@
 <tr><td><code>server.failed_reservation_timeout</code></td><td>duration</td><td><code>5s</code></td><td>the amount of time to consider the store throttled for up-replication after a failed reservation call</td></tr>
 <tr><td><code>server.heap_profile.max_profiles</code></td><td>integer</td><td><code>5</code></td><td>maximum number of profiles to be kept. Profiles with lower score are GC'ed, but latest profile is always kept</td></tr>
 <tr><td><code>server.heap_profile.system_memory_threshold_fraction</code></td><td>float</td><td><code>0.85</code></td><td>fraction of system memory beyond which if Rss increases, then heap profile is triggered</td></tr>
+<tr><td><code>server.host_based_authentication.configuration</code></td><td>string</td><td><code></code></td><td>host-based authentication configuration to use during connection authentication</td></tr>
 <tr><td><code>server.rangelog.ttl</code></td><td>duration</td><td><code>720h0m0s</code></td><td>if nonzero, range log entries older than this duration are deleted every 10m0s. Should not be lowered below 24 hours</td></tr>
 <tr><td><code>server.remote_debugging.mode</code></td><td>string</td><td><code>local</code></td><td>set to enable remote debugging, localhost-only or disable (any, local, off)</td></tr>
 <tr><td><code>server.shutdown.drain_wait</code></td><td>duration</td><td><code>0s</code></td><td>the amount of time a server waits in an unready state before proceeding with the rest of the shutdown process</td></tr>

pkg/sql/pgwire/conn.go

@@ -29,8 +29,10 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/hba"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -143,6 +145,7 @@ func serveConn(
 	ie *sql.InternalExecutor,
 	stopper *stop.Stopper,
 	insecure bool,
+	auth *hba.Conf,
 ) error {
 	sArgs.RemoteAddr = netConn.RemoteAddr()
 
@@ -152,7 +155,7 @@ func serveConn(
 
 	c := newConn(netConn, sArgs, metrics, resultsBufferBytes)
 
-	if err := c.handleAuthentication(ctx, insecure, ie); err != nil {
+	if err := c.handleAuthentication(ctx, insecure, ie, auth); err != nil {
 		_ = c.conn.Close()
 		reserved.Close(ctx)
 		return err
@@ -1280,7 +1283,7 @@ func (r *pgwireReader) ReadByte() (byte, error) {
 // point the sql.Session does not exist yet! If need exists to access the
 // database to look up authentication data, use the internal executor.
 func (c *conn) handleAuthentication(
-	ctx context.Context, insecure bool, ie *sql.InternalExecutor,
+	ctx context.Context, insecure bool, ie *sql.InternalExecutor, auth *hba.Conf,
 ) error {
 	sendError := func(err error) error {
 		_ /* err */ = writeErr(err, &c.msgBuilder, c.conn)
@@ -1300,31 +1303,65 @@ func (c *conn) handleAuthentication(
 	}
 
 	if tlsConn, ok := c.conn.(*tls.Conn); ok {
-		var authenticationHook security.UserAuthHook
-
 		tlsState := tlsConn.ConnectionState()
-		// If no certificates are provided, default to password
-		// authentication.
-		if len(tlsState.PeerCertificates) == 0 {
-			password, err := c.sendAuthPasswordRequest()
-			if err != nil {
-				return sendError(err)
-			}
-			authenticationHook = security.UserAuthPasswordHook(
-				insecure, password, hashedPassword,
-			)
+		var methodFn AuthMethod
+
+		if auth == nil {
+			methodFn = authCertPassword
+		} else if c.sessionArgs.User == security.RootUser {
+			// If a hba.conf file is specified, hard code the root user to always use
+			// cert auth. This prevents users from shooting themselves in the foot and
+			// making root not able to login, thus disallowing anyone from fixing the
+			// hba.conf file.
+			methodFn = authCert
 		} else {
-			// Normalize the username contained in the certificate.
-			tlsState.PeerCertificates[0].Subject.CommonName = tree.Name(
-				tlsState.PeerCertificates[0].Subject.CommonName,
-			).Normalize()
-			var err error
-			authenticationHook, err = security.UserAuthCertHook(insecure, &tlsState)
+			addr, _, err := net.SplitHostPort(c.conn.RemoteAddr().String())
 			if err != nil {
 				return sendError(err)
 			}
+			ip := net.ParseIP(addr)
+			for _, entry := range auth.Entries {
+				switch a := entry.Address.(type) {
+				case *net.IPNet:
+					if !a.Contains(ip) {
+						continue
+					}
+				case hba.String:
+					if !a.IsSpecial("all") {
+						return sendError(errors.Errorf("unexpected %s address: %q", serverHBAConfSetting, a.Value))
+					}
+				default:
+					return sendError(errors.Errorf("unexpected address type %T", a))
+				}
+				match := false
+				for _, u := range entry.User {
+					if u.IsSpecial("all") {
+						match = true
+						break
+					}
+					if u.Value == c.sessionArgs.User {
+						match = true
+						break
+					}
+				}
+				if !match {
+					continue
+				}
+				methodFn = hbaAuthMethods[entry.Method]
+				if methodFn == nil {
+					return sendError(errors.Errorf("unknown auth method %s", entry.Method))
+				}
+				break
+			}
+			if methodFn == nil {
+				return sendError(errors.Errorf("no %s entry for host %q, user %q", serverHBAConfSetting, addr, c.sessionArgs.User))
+			}
 		}
 
+		authenticationHook, err := methodFn(c, tlsState, insecure, hashedPassword)
+		if err != nil {
+			return sendError(err)
+		}
 		if err := authenticationHook(c.sessionArgs.User, true /* public */); err != nil {
 			return sendError(err)
 		}
@@ -1335,9 +1372,98 @@ func (c *conn) handleAuthentication(
 	return c.msgBuilder.finishMsg(c.conn)
 }
 
-// sendAuthPasswordRequest requests a cleartext password from the client and
+const serverHBAConfSetting = "server.host_based_authentication.configuration"
+
+var connAuthConf = settings.RegisterValidatedStringSetting(
+	serverHBAConfSetting,
+	"host-based authentication configuration to use during connection authentication",
+	"",
+	func(values *settings.Values, s string) error {
+		if s == "" {
+			return nil
+		}
+		conf, err := hba.Parse(s)
+		if err != nil {
+			return err
+		}
+		for _, entry := range conf.Entries {
+			for _, db := range entry.Database {
+				if !db.IsSpecial("all") {
+					return errors.New("database must be specified as all")
+				}
+			}
+			if addr, ok := entry.Address.(hba.String); ok && !addr.IsSpecial("all") {
+				return errors.New("host addresses not supported")
+			}
+			if hbaAuthMethods[entry.Method] == nil {
+				return errors.Errorf("unknown auth method %q", entry.Method)
+			}
+		}
+		return nil
+	},
+)
+
+// AuthConn defines exported methods of a conn needed for pgwire authentication.
+type AuthConn interface {
+	SendAuthPasswordRequest() (string, error)
+}
+
+// AuthMethod defines a method for authentication of a connection.
+type AuthMethod func(c AuthConn, tlsState tls.ConnectionState, insecure bool, hashedPassword []byte) (security.UserAuthHook, error)
+
+var hbaAuthMethods = map[string]AuthMethod{}
+
+// RegisterAuthMethod registers an AuthMethod for pgwire authentication.
+func RegisterAuthMethod(method string, fn AuthMethod) {
+	hbaAuthMethods[method] = fn
+}
+
+func authPassword(
+	c AuthConn, tlsState tls.ConnectionState, insecure bool, hashedPassword []byte,
+) (security.UserAuthHook, error) {
+	password, err := c.SendAuthPasswordRequest()
+	if err != nil {
+		return nil, err
+	}
+	return security.UserAuthPasswordHook(
+		insecure, password, hashedPassword,
+	), nil
+}
+
+func authCert(
+	c AuthConn, tlsState tls.ConnectionState, insecure bool, hashedPassword []byte,
+) (security.UserAuthHook, error) {
+	if len(tlsState.PeerCertificates) == 0 {
+		return nil, errors.New("no TLS peer certificates, but required for auth")
+	}
+	// Normalize the username contained in the certificate.
+	tlsState.PeerCertificates[0].Subject.CommonName = tree.Name(
+		tlsState.PeerCertificates[0].Subject.CommonName,
+	).Normalize()
+	return security.UserAuthCertHook(insecure, &tlsState)
+}
+
+func authCertPassword(
+	c AuthConn, tlsState tls.ConnectionState, insecure bool, hashedPassword []byte,
+) (security.UserAuthHook, error) {
+	var fn AuthMethod
+	if len(tlsState.PeerCertificates) == 0 {
+		fn = authPassword
+	} else {
+		fn = authCert
+	}
+	return fn(c, tlsState, insecure, hashedPassword)
+}
+
+func init() {
+	RegisterAuthMethod("password", authPassword)
+	RegisterAuthMethod("cert", authCert)
+	RegisterAuthMethod("cert-password", authCertPassword)
+}
+
+// SendAuthPasswordRequest requests a cleartext password from the client and
 // returns it.
-func (c *conn) sendAuthPasswordRequest() (string, error) {
+func (c *conn) SendAuthPasswordRequest() (string, error) {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgAuth)
 	c.msgBuilder.putInt32(authCleartextPassword)
 	if err := c.msgBuilder.finishMsg(c.conn); err != nil {

pkg/sql/pgwire/hba/Makefile

@@ -0,0 +1,11 @@
+conf.go: conf.rl
+	# Use of -T0 here produces the smallest amount of generated code. We
+	# don't care about parsing performance so optimize instead for small files
+	# and fast compilations.
+	ragel -Z -T0 conf.rl -o conf.go
+	(echo "// Code generated by ragel. DO NOT EDIT."; \
+	 echo "// GENERATED FILE DO NOT EDIT"; \
+	 cat conf.go) > conf.go.tmp
+	mv conf.go.tmp conf.go
+	../../../../bin/gofmt -w -s conf.go
+	../../../../bin/goimports -w conf.go