Skip to content

Commit ee0554b

Browse files
committed
security: add sql client error for TLSCipherRestrict
Currently, TLSCipherRestrict added in #143554 does not log errors to client making it difficult to debug issues related to ciphers presented in client tls negotiation. We will be adding a client facing error message with code `SQLSTATE: 08004` for `pgcode.SQLserverRejectedEstablishmentOfSQLconnection`. fixes #136999 Epic CRDB-45351 Release Note(security): The client for the sql connection will additionally get a new error if trying to connect with unsupported cipher along with the error in `OPS` channel. * client error: ``` > ./cockroach sql --certs-dir=certs --url "postgresql://root@localhost:26257" # # Welcome to the CockroachDB SQL shell. # All statements must be terminated by a semicolon. # To exit, type: \q. # ERROR: cannot use SSL/TLS with the requested ciphers: presented cipher TLS_AES_128_GCM_SHA256 not in allowed cipher suite list SQLSTATE: 08004 Failed running "sql" ``` * ops channel error: ``` E250512 06:53:28.160841 24028 1@server/server_sql.go:1977 ⋮ [T1,Vsystem,n1,client=‹127.0.0.1:62122›] 479 serving SQL client conn: presented cipher TLS_AES_128_GCM_SHA256 not in allowed cipher suite list ```
1 parent 2e75789 commit ee0554b

File tree

4 files changed

+7
-6
lines changed

4 files changed

+7
-6
lines changed

pkg/security/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ go_library(
5050
"//pkg/util/uuid",
5151
"@com_github_cockroachdb_errors//:errors",
5252
"@com_github_cockroachdb_errors//oserror",
53+
"@com_github_cockroachdb_redact//:redact",
5354
"@com_github_go_ldap_ldap_v3//:ldap",
5455
"@org_golang_x_crypto//bcrypt",
5556
"@org_golang_x_crypto//ocsp",

pkg/security/tls_ciphersuites.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
1313
"github.com/cockroachdb/errors"
14+
"github.com/cockroachdb/redact"
1415
"golang.org/x/exp/slices"
1516
)
1617

@@ -179,7 +180,7 @@ func (*tlsRestrictConfiguration) configureTLSRestrict(ciphers []string) {
179180
return &cipherRestrictError{errors.Errorf("cipher id %v does match implemented tls ciphers", selectedCipherID)}
180181
}
181182
if !slices.Contains(tlsRestrictConfig.c, cName) {
182-
return &cipherRestrictError{errors.Newf("presented cipher %s not in allowed cipher suite list", cName)}
183+
return &cipherRestrictError{errors.Newf("presented cipher %s not in allowed cipher suite list", redact.SafeString(cName))}
183184
}
184185
return
185186
}

pkg/security/tls_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func TestTLSCipherRestrict(t *testing.T) {
138138
wantErr: false},
139139
{name: "invalid ciphers", ciphers: []string{"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"}, wantErr: true,
140140
httpsErr: []string{"\": EOF", "connect: connection refused", "read: connection reset by peer", "http: server closed idle connection"},
141-
sqlErr: "failed to connect to `host=127.0.0.1 user=root database=`: failed to receive message (unexpected EOF)",
141+
sqlErr: "^failed to connect to `host=127\\.0\\.0\\.1 user=root database=`: server error \\(ERROR: cannot use SSL\\/TLS with the requested ciphers: presented cipher [^ ]+ not in allowed cipher suite list \\(SQLSTATE 08004\\)\\)$",
142142
rpcErr: "initial connection heartbeat failed: grpc:",
143143
cipherErr: "^presented cipher [^ ]+ not in allowed cipher suite list$"},
144144
}
@@ -242,7 +242,7 @@ func TestTLSCipherRestrict(t *testing.T) {
242242
errVal := cipherErrC.err
243243
cipherErrC.Unlock()
244244
require.Regexp(t, tt.cipherErr, errVal.Error())
245-
require.Equal(t, tt.sqlErr, err.Error())
245+
require.Regexp(t, tt.sqlErr, err.Error())
246246
}
247247

248248
// test rpc connection for root user.

pkg/sql/pgwire/pre_serve.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,8 @@ func (s *PreServeConnHandler) maybeUpgradeToSecureConn(
492492
}
493493
newConn = tls.Server(conn, tlsConfig)
494494
// Conditionally perform handshake connection and determine additional restrictions.
495-
serverErr = security.TLSCipherRestrict(newConn)
496-
if serverErr != nil {
497-
_ = newConn.Close()
495+
if err := security.TLSCipherRestrict(newConn); err != nil {
496+
clientErr = pgerror.Wrapf(err, pgcode.SQLserverRejectedEstablishmentOfSQLconnection, "cannot use SSL/TLS with the requested ciphers")
498497
return
499498
}
500499
newConnType = hba.ConnHostSSL

0 commit comments

Comments
 (0)