Skip to content

Commit dac763b

Browse files
committed
Don't panic in SSL code
This change removes panics from SSL code. This is a part of the ongoing process of removing panics from lib/pq code. Refs #681.
1 parent b200422 commit dac763b

File tree

4 files changed

+70
-40
lines changed

4 files changed

+70
-40
lines changed

conn.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -340,15 +340,19 @@ func DialOpen(d Dialer, name string) (_ driver.Conn, err error) {
340340
return nil, err
341341
}
342342

343-
// cn.ssl and cn.startup panic on error. Make sure we don't leak cn.c.
343+
err = cn.ssl(o)
344+
if err != nil {
345+
return nil, err
346+
}
347+
348+
// cn.startup panics on error. Make sure we don't leak cn.c.
344349
panicking := true
345350
defer func() {
346351
if panicking {
347352
cn.c.Close()
348353
}
349354
}()
350355

351-
cn.ssl(o)
352356
cn.buf = bufio.NewReader(cn.c)
353357
cn.startup(o)
354358

@@ -1029,30 +1033,35 @@ func (cn *conn) recv1() (t byte, r *readBuf) {
10291033
return t, r
10301034
}
10311035

1032-
func (cn *conn) ssl(o values) {
1033-
upgrade := ssl(o)
1036+
func (cn *conn) ssl(o values) error {
1037+
upgrade, err := ssl(o)
1038+
if err != nil {
1039+
return err
1040+
}
1041+
10341042
if upgrade == nil {
10351043
// Nothing to do
1036-
return
1044+
return nil
10371045
}
10381046

10391047
w := cn.writeBuf(0)
10401048
w.int32(80877103)
1041-
if err := cn.sendStartupPacket(w); err != nil {
1042-
panic(err)
1049+
if err = cn.sendStartupPacket(w); err != nil {
1050+
return err
10431051
}
10441052

10451053
b := cn.scratch[:1]
1046-
_, err := io.ReadFull(cn.c, b)
1054+
_, err = io.ReadFull(cn.c, b)
10471055
if err != nil {
1048-
panic(err)
1056+
return err
10491057
}
10501058

10511059
if b[0] != 'S' {
1052-
panic(ErrSSLNotSupported)
1060+
return ErrSSLNotSupported
10531061
}
10541062

1055-
cn.c = upgrade(cn.c)
1063+
cn.c, err = upgrade(cn.c)
1064+
return err
10561065
}
10571066

10581067
// isDriverSetting returns true iff a setting is purely for configuring the

conn_go18.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ func (cn *conn) cancel() error {
108108
can := conn{
109109
c: c,
110110
}
111-
can.ssl(cn.opts)
111+
err = can.ssl(cn.opts)
112+
if err != nil {
113+
return err
114+
}
112115

113116
w := can.writeBuf(0)
114117
w.int32(80877102) // cancel request code

error.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,11 @@ func errorf(s string, args ...interface{}) {
460460
panic(fmt.Errorf("pq: %s", fmt.Sprintf(s, args...)))
461461
}
462462

463+
// TODO(ainar-g) Rename to errorf after removing panics.
464+
func fmterrorf(s string, args ...interface{}) error {
465+
return fmt.Errorf("pq: %s", fmt.Sprintf(s, args...))
466+
}
467+
463468
func errRecoverNoErrBadConn(err *error) {
464469
e := recover()
465470
if e == nil {

ssl.go

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
// ssl generates a function to upgrade a net.Conn based on the "sslmode" and
1414
// related settings. The function is nil when no upgrade should take place.
15-
func ssl(o values) func(net.Conn) net.Conn {
15+
func ssl(o values) (f func(net.Conn) (net.Conn, error), err error) {
1616
verifyCaOnly := false
1717
tlsConf := tls.Config{}
1818
switch mode := o["sslmode"]; mode {
@@ -31,7 +31,7 @@ func ssl(o values) func(net.Conn) net.Conn {
3131
// behavior is discouraged, and applications that need certificate
3232
// validation should always use verify-ca or verify-full.
3333
if sslrootcert, ok := o["sslrootcert"]; ok {
34-
if _, err := os.Stat(sslrootcert); err == nil {
34+
if _, err = os.Stat(sslrootcert); err == nil {
3535
verifyCaOnly = true
3636
} else {
3737
delete(o, "sslrootcert")
@@ -45,29 +45,39 @@ func ssl(o values) func(net.Conn) net.Conn {
4545
case "verify-full":
4646
tlsConf.ServerName = o["host"]
4747
case "disable":
48-
return nil
48+
return nil, nil
4949
default:
50-
errorf(`unsupported sslmode %q; only "require" (default), "verify-full", "verify-ca", and "disable" supported`, mode)
50+
err = fmterrorf(`unsupported sslmode %q; only "require" (default), "verify-full", "verify-ca", and "disable" supported`, mode)
51+
return nil, err
5152
}
5253

53-
sslClientCertificates(&tlsConf, o)
54-
sslCertificateAuthority(&tlsConf, o)
54+
err = sslClientCertificates(&tlsConf, o)
55+
if err != nil {
56+
return nil, err
57+
}
58+
err = sslCertificateAuthority(&tlsConf, o)
59+
if err != nil {
60+
return nil, err
61+
}
5562
sslRenegotiation(&tlsConf)
5663

57-
return func(conn net.Conn) net.Conn {
64+
return func(conn net.Conn) (net.Conn, error) {
5865
client := tls.Client(conn, &tlsConf)
5966
if verifyCaOnly {
60-
sslVerifyCertificateAuthority(client, &tlsConf)
67+
errVerify := sslVerifyCertificateAuthority(client, &tlsConf)
68+
if errVerify != nil {
69+
return nil, errVerify
70+
}
6171
}
62-
return client
63-
}
72+
return client, nil
73+
}, nil
6474
}
6575

6676
// sslClientCertificates adds the certificate specified in the "sslcert" and
6777
// "sslkey" settings, or if they aren't set, from the .postgresql directory
6878
// in the user's home directory. The configured files must exist and have
6979
// the correct permissions.
70-
func sslClientCertificates(tlsConf *tls.Config, o values) {
80+
func sslClientCertificates(tlsConf *tls.Config, o values) (err error) {
7181
// user.Current() might fail when cross-compiling. We have to ignore the
7282
// error and continue without home directory defaults, since we wouldn't
7383
// know from where to load them.
@@ -82,13 +92,13 @@ func sslClientCertificates(tlsConf *tls.Config, o values) {
8292
}
8393
// https://github.com/postgres/postgres/blob/REL9_6_2/src/interfaces/libpq/fe-secure-openssl.c#L1045
8494
if len(sslcert) == 0 {
85-
return
95+
return nil
8696
}
8797
// https://github.com/postgres/postgres/blob/REL9_6_2/src/interfaces/libpq/fe-secure-openssl.c#L1050:L1054
88-
if _, err := os.Stat(sslcert); os.IsNotExist(err) {
89-
return
98+
if _, err = os.Stat(sslcert); os.IsNotExist(err) {
99+
return nil
90100
} else if err != nil {
91-
panic(err)
101+
return err
92102
}
93103

94104
// In libpq, the ssl key is only loaded if the setting is not blank.
@@ -100,44 +110,49 @@ func sslClientCertificates(tlsConf *tls.Config, o values) {
100110
}
101111

102112
if len(sslkey) > 0 {
103-
if err := sslKeyPermissions(sslkey); err != nil {
104-
panic(err)
113+
if err = sslKeyPermissions(sslkey); err != nil {
114+
return err
105115
}
106116
}
107117

108118
cert, err := tls.LoadX509KeyPair(sslcert, sslkey)
109119
if err != nil {
110-
panic(err)
120+
return err
111121
}
122+
112123
tlsConf.Certificates = []tls.Certificate{cert}
124+
return nil
113125
}
114126

115127
// sslCertificateAuthority adds the RootCA specified in the "sslrootcert" setting.
116-
func sslCertificateAuthority(tlsConf *tls.Config, o values) {
128+
func sslCertificateAuthority(tlsConf *tls.Config, o values) (err error) {
117129
// In libpq, the root certificate is only loaded if the setting is not blank.
118130
//
119131
// https://github.com/postgres/postgres/blob/REL9_6_2/src/interfaces/libpq/fe-secure-openssl.c#L950-L951
120132
if sslrootcert := o["sslrootcert"]; len(sslrootcert) > 0 {
121133
tlsConf.RootCAs = x509.NewCertPool()
122134

123-
cert, err := ioutil.ReadFile(sslrootcert)
135+
var cert []byte
136+
cert, err = ioutil.ReadFile(sslrootcert)
124137
if err != nil {
125-
panic(err)
138+
return err
126139
}
127140

128141
if !tlsConf.RootCAs.AppendCertsFromPEM(cert) {
129-
errorf("couldn't parse pem in sslrootcert")
142+
return fmterrorf("couldn't parse pem in sslrootcert")
130143
}
131144
}
145+
146+
return nil
132147
}
133148

134149
// sslVerifyCertificateAuthority carries out a TLS handshake to the server and
135150
// verifies the presented certificate against the CA, i.e. the one specified in
136151
// sslrootcert or the system CA if sslrootcert was not specified.
137-
func sslVerifyCertificateAuthority(client *tls.Conn, tlsConf *tls.Config) {
138-
err := client.Handshake()
152+
func sslVerifyCertificateAuthority(client *tls.Conn, tlsConf *tls.Config) (err error) {
153+
err = client.Handshake()
139154
if err != nil {
140-
panic(err)
155+
return err
141156
}
142157
certs := client.ConnectionState().PeerCertificates
143158
opts := x509.VerifyOptions{
@@ -152,7 +167,5 @@ func sslVerifyCertificateAuthority(client *tls.Conn, tlsConf *tls.Config) {
152167
opts.Intermediates.AddCert(cert)
153168
}
154169
_, err = certs[0].Verify(opts)
155-
if err != nil {
156-
panic(err)
157-
}
170+
return err
158171
}

0 commit comments

Comments
 (0)