transport: deny incoming peer certs with wrong IP SAN #7687
Conversation
heyitsanthony
force-pushed
the
deny-tls-ipsan
branch
from
39e32e9
to
6b5d00f
Compare
| addr := tlsConn.RemoteAddr().String() | ||
| h, _, herr := net.SplitHostPort(addr) | ||
| if herr != nil || cert.VerifyHostname(h) != nil { | ||
| return |
There was a problem hiding this comment.
what error the other side receives if we close the conn here?
There was a problem hiding this comment.
this is no ideal. we probably need to log explicitly at server side.
there is no easy way to terminate it inside the TLS handshake path?
There was a problem hiding this comment.
If it did the verification, it'd be at https://golang.org/src/crypto/tls/handshake_server.go#L735 but there is no way to set opts.DNSNames which is needed to do the check at https://golang.org/src/crypto/x509/verify.go#L298. The go standard library even discourages doing this kind of check on the server side: https://golang.org/src/crypto/tls/conn.go#L1377
There was a problem hiding this comment.
ok. let's log the error at server side. so that at least, human operator can know what is going on.
There was a problem hiding this comment.
Can't log with plog or it'll cause issues with clientv3 imports (9b84127). Should there be an optional logging function in tls.Config then?
There was a problem hiding this comment.
s/tls.Config/transport.TLSInfo
There was a problem hiding this comment.
add an interface or func seems ok.
heyitsanthony
force-pushed
the
deny-tls-ipsan
branch
from
6b5d00f
to
7949b87
Compare
heyitsanthony
force-pushed
the
deny-tls-ipsan
branch
from
7949b87
to
70a9929
Compare
|
lgtm |
/cc @hongchaodeng