sql: fix read-only SSL var

The variable will now look at the connection state to determine if
SSL/TLS is being used, rather than relying on server configuration
params, which aren't sufficient to be able to determine the type of
connection.

Release note: None

pkg/sql/conn_executor.go

@@ -880,6 +880,7 @@ func newSessionData(args SessionArgs) *sessiondata.SessionData {
 		},
 		LocalUnmigratableSessionData: sessiondata.LocalUnmigratableSessionData{
 			RemoteAddr: args.RemoteAddr,
+			IsSSL:      args.IsSSL,
 		},
 		LocalOnlySessionData: sessiondatapb.LocalOnlySessionData{
 			ResultsBufferSize:   args.ConnResultsBufferSize,

pkg/sql/exec_util.go

@@ -2119,6 +2119,7 @@ type SessionDefaults map[string]string
 type SessionArgs struct {
 	User                        username.SQLUsername
 	IsSuperuser                 bool
+	IsSSL                       bool
 	SystemIdentity              username.SQLUsername
 	SessionDefaults             SessionDefaults
 	CustomOptionSessionDefaults SessionDefaults

pkg/sql/pgwire/auth_test.go

@@ -48,6 +48,7 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/stdstrings"
 	"github.com/cockroachdb/redact"
+	pgx "github.com/jackc/pgx/v4"
 	"github.com/lib/pq"
 	"github.com/stretchr/testify/require"
 )
@@ -773,4 +774,51 @@ func TestClientAddrOverride(t *testing.T) {
 	}
 }
 
+// TestSSLSessionVar checks that the read-only SSL session variable correctly
+// reflects the state of the connection.
+func TestSSLSessionVar(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	sc := log.ScopeWithoutShowLogs(t)
+	defer sc.Close(t)
+
+	// Start a server.
+	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	s.(*server.TestServer).Cfg.AcceptSQLWithoutTLS = true
+	ctx := context.Background()
+	defer s.Stopper().Stop(ctx)
+
+	// Ensure the test user exists.
+	if _, err := db.Exec(fmt.Sprintf(`CREATE USER %s WITH PASSWORD 'abc'`, username.TestUser)); err != nil {
+		t.Fatal(err)
+	}
+
+	pgURLWithCerts, cleanupFuncCerts := sqlutils.PGUrlWithOptionalClientCerts(
+		t, s.ServingSQLAddr(), "TestSSLSessionVarCerts" /* prefix */, url.User(username.TestUser), true,
+	)
+	defer cleanupFuncCerts()
+
+	pgURLWithoutCerts, cleanupFuncWithoutCerts := sqlutils.PGUrlWithOptionalClientCerts(
+		t, s.ServingSQLAddr(), "TestSSLSessionVarNoCerts" /* prefix */, url.UserPassword(username.TestUser, "abc"), false,
+	)
+	defer cleanupFuncWithoutCerts()
+	q := pgURLWithoutCerts.Query()
+	q.Set("sslmode", "disable")
+	pgURLWithoutCerts.RawQuery = q.Encode()
+
+	// Connect with certs.
+	connWithCerts, err := pgx.Connect(ctx, pgURLWithCerts.String())
+	require.NoError(t, err)
+	var result string
+	err = connWithCerts.QueryRow(ctx, "SHOW ssl").Scan(&result)
+	require.NoError(t, err)
+	require.Equal(t, "on", result)
+
+	// Connect without certs.
+	connWithoutCerts, err := pgx.Connect(ctx, pgURLWithoutCerts.String())
+	require.NoError(t, err)
+	err = connWithoutCerts.QueryRow(ctx, "SHOW ssl").Scan(&result)
+	require.NoError(t, err)
+	require.Equal(t, "off", result)
+}
+
 var sessionTerminatedRe = regexp.MustCompile("client_session_end")

pkg/sql/pgwire/pre_serve.go

@@ -404,6 +404,7 @@ func (s *PreServeConnHandler) PreServe(
 		st.Reserved.Close(ctx)
 		return conn, st, s.sendErr(ctx, conn, err)
 	}
+	st.clientParameters.IsSSL = st.ConnType == hba.ConnHostSSL
 
 	st.State = PreServeReady
 	return conn, st, nil

pkg/sql/pgwire/server.go

@@ -746,8 +746,7 @@ func (s *Server) ServeConn(
 		return s.sendErr(ctx, st, conn, newAdminShutdownErr(ErrDrainingNewConn))
 	}
 
-	sArgs, err := finalizeClientParameters(ctx, preServeStatus.clientParameters,
-		&st.SV)
+	sArgs, err := finalizeClientParameters(ctx, preServeStatus.clientParameters, &st.SV)
 	if err != nil {
 		preServeStatus.Reserved.Close(ctx)
 		return s.sendErr(ctx, st, conn, err)

pkg/sql/sessiondata/session_data.go

@@ -169,12 +169,15 @@ type LocalUnmigratableSessionData struct {
 	// dependencies. Temporary tables are not supported in session migrations.
 	DatabaseIDToTempSchemaID map[uint32]uint32
 
-	///////////////////////////////////////////////////////////////////////////
+	// IsSSL indicates whether the session is using SSL/TLS.
+	IsSSL bool
+
+	// ////////////////////////////////////////////////////////////////////////
 	// WARNING: consider whether a session parameter you're adding needs to  //
 	// be propagated to the remote nodes or needs to persist amongst session //
 	// migrations. If so, they should live in the LocalOnlySessionData or    //
 	// SessionData protobuf in the sessiondatapb package.                    //
-	///////////////////////////////////////////////////////////////////////////
+	// ////////////////////////////////////////////////////////////////////////
 }
 
 // IsTemporarySchemaID returns true if the given ID refers to any of the temp

pkg/sql/vars.go

@@ -1330,8 +1330,7 @@ var varGen = map[string]sessionVar{
 	`ssl`: {
 		Hidden: true,
 		Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
-			insecure := evalCtx.ExecCfg.RPCContext.Config.Insecure || evalCtx.ExecCfg.RPCContext.Config.AcceptSQLWithoutTLS
-			return formatBoolAsPostgresSetting(!insecure), nil
+			return formatBoolAsPostgresSetting(evalCtx.SessionData().IsSSL), nil
 		},
 	},