Skip to content

Commit

Permalink
Use custom verifyPeer function when client doesn't set trust certs
Browse files Browse the repository at this point in the history
  • Loading branch information
@ZhenLian
ZhenLian committed Dec 5, 2019
1 parent 781f1c0 commit 74939ed
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 62 deletions.
130 changes: 70 additions & 60 deletions security/advancedtls/advancedtls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,37 +41,6 @@ type sysConn = syscall.Conn
type CustomVerificationFunc func(serverName string,
rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error

// NewClientTLS uses ClientOptions to construct a TransportCredentials based on TLS.
func NewClientTLS(o *ClientOptions) (credentials.TransportCredentials, error) {
conf, err := o.config()
if err != nil {
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: true,
getRootCAs: o.GetRootCAs,
verifyFunc: o.VerifyPeer,
}
tc.config.NextProtos = appendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
}

// NewServerTLS uses ServerOptions to construct a TransportCredentials based on TLS.
func NewServerTLS(o *ServerOptions) (credentials.TransportCredentials, error) {
conf, err := o.config()
if err != nil {
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: false,
getRootCAs: o.GetRootCAs,
}
tc.config.NextProtos = appendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
}

// ClientOptions contains all the fields and functions needed to be filled by the client.
type ClientOptions struct {
// ---------------General Rules For Certificate Setting-----------------------------------------
Expand Down Expand Up @@ -144,8 +113,9 @@ type ServerOptions struct {
}

func (o *ClientOptions) config() (*tls.Config, error) {
if o.RootCACerts == nil && o.GetRootCAs == nil {
return nil, fmt.Errorf("either RootCACerts or GetRootCAs must be specified")
if o.RootCACerts == nil && o.GetRootCAs == nil && o.VerifyPeer == nil {
return nil, fmt.Errorf(
"client needs to provide root CA certs, or a custom verification function")
}
config := &tls.Config{
ServerName: o.ServerNameOverride,
Expand All @@ -161,31 +131,27 @@ func (o *ServerOptions) config() (*tls.Config, error) {
if o.Certificates == nil && o.GetCertificate == nil {
return nil, fmt.Errorf("either Certificates or GetCertificate must be specified")
}
if !o.MutualAuth {
return &tls.Config{
ClientAuth: tls.NoClientCert,
Certificates: o.Certificates,
GetCertificate: o.GetCertificate,
}, nil
}
if o.GetRootCAs == nil && o.RootCACerts == nil {
if o.MutualAuth && o.GetRootCAs == nil && o.RootCACerts == nil {
return nil, fmt.Errorf("server needs to provide root CA certs if using mutual TLS")
}
// We fall back to normal config settings if users don't need to reload root certificates.
if o.RootCACerts != nil {
return &tls.Config{
ClientAuth: tls.RequireAndVerifyClientCert,
Certificates: o.Certificates,
GetCertificate: o.GetCertificate,
ClientCAs: o.RootCACerts,
}, nil
clientAuth := tls.NoClientCert
if o.MutualAuth {
// We fall back to normal config settings if users don't need to reload root certificates.
if o.RootCACerts != nil {
clientAuth = tls.RequireAndVerifyClientCert
} else {
clientAuth = tls.RequireAnyClientCert
}
}
return &tls.Config{
ClientAuth: tls.RequireAnyClientCert,
config := &tls.Config{
ClientAuth: clientAuth,
Certificates: o.Certificates,
GetCertificate: o.GetCertificate,
}, nil

}
if o.RootCACerts != nil {
config.ClientCAs = o.RootCACerts
}
return config, nil
}

// advancedTLSCreds is the credentials required for authenticating a connection using TLS.
Expand All @@ -206,10 +172,10 @@ func (c advancedTLSCreds) Info() credentials.ProtocolInfo {

func (c *advancedTLSCreds) ClientHandshake(ctx context.Context, authority string,
rawConn net.Conn) (_ net.Conn, _ credentials.AuthInfo, err error) {
// use local cfg to avoid clobbering ServerName if using multiple endpoints
// Use local cfg to avoid clobbering ServerName if using multiple endpoints.
cfg := cloneTLSConfig(c.config)
// we return the full authority name to users if ServerName is empty without
// stripping the trailing port
// We return the full authority name to users if ServerName is empty without
// stripping the trailing port.
if cfg.ServerName == "" {
cfg.ServerName = authority
}
Expand Down Expand Up @@ -266,7 +232,16 @@ func (c *advancedTLSCreds) OverrideServerName(serverNameOverride string) error {
func buildVerifyFunc(c *advancedTLSCreds,
serverName string,
rawConn net.Conn) func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
verifyFunc := func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
return func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
// If users didn't specify either rootCAs or getRootCAs on client side,
// as we see some use cases such as https://github.com/grpc/grpc/pull/20530,
// instead of failing, we just don't validate the server cert and let
// application decide via VerifyPeer
if c.isClient && c.config.RootCAs == nil && c.getRootCAs == nil {
if c.verifyFunc != nil {
return c.verifyFunc(serverName, rawCerts, verifiedChains)
}
}
var rootCAs *x509.CertPool
if c.isClient {
rootCAs = c.config.RootCAs
Expand All @@ -284,7 +259,10 @@ func buildVerifyFunc(c *advancedTLSCreds,
// verify peers' certificates against RootCAs and get verifiedChains
certs := make([]*x509.Certificate, len(rawCerts))
for i, asn1Data := range rawCerts {
cert, _ := x509.ParseCertificate(asn1Data)
cert, err := x509.ParseCertificate(asn1Data)
if err != nil {
return err
}
certs[i] = cert
}
opts := x509.VerifyOptions{
Expand All @@ -294,6 +272,8 @@ func buildVerifyFunc(c *advancedTLSCreds,
}
if !c.isClient {
opts.KeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}
} else {
opts.KeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
}
for _, cert := range certs[1:] {
opts.Intermediates.AddCert(cert)
Expand All @@ -311,11 +291,41 @@ func buildVerifyFunc(c *advancedTLSCreds,
}
return nil
}
return verifyFunc
}

// NewClientTLS uses ClientOptions to construct a TransportCredentials based on TLS.
func NewClientTLS(o *ClientOptions) (credentials.TransportCredentials, error) {
conf, err := o.config()
if err != nil {
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: true,
getRootCAs: o.GetRootCAs,
verifyFunc: o.VerifyPeer,
}
tc.config.NextProtos = appendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
}

// NewServerTLS uses ServerOptions to construct a TransportCredentials based on TLS.
func NewServerTLS(o *ServerOptions) (credentials.TransportCredentials, error) {
conf, err := o.config()
if err != nil {
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: false,
getRootCAs: o.GetRootCAs,
}
tc.config.NextProtos = appendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
}

// TODO(ZhenLian): The code below are duplicates with gRPC-Go under
// TODO(ZhenLian): credentials/internal. Consider refactoring in the future.
// credentials/internal. Consider refactoring in the future.
const alpnProtoStrH2 = "h2"

func appendH2ToNextProtos(ps []string) []string {
Expand Down
26 changes: 24 additions & 2 deletions security/advancedtls/advancedtls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ func TestClientServerHandshake(t *testing.T) {
}{
// Client: nil setting
// Server: only set serverCert with mutual TLS off
// Expected Behavior: client option creating time failure
// Reason: client side must either set root or getRoot
// Expected Behavior: server side failure
// Reason: if either clientCert or clientGetClientCert is not set and
// verifyFunc is not set, we will fail directly
{
"Client_no_trust_cert_Server_peer_cert",
nil,
Expand All @@ -105,6 +106,27 @@ func TestClientServerHandshake(t *testing.T) {
nil,
nil,
nil,
true,
},
// Client: nil setting except verifyFunc
// Server: only set serverCert with mutual TLS off
// Expected Behavior: success
// Reason: we will use verifyFunc to verify the server,
// if either clientCert or clientGetClientCert is not set
{
"Client_no_trust_cert_verifyFunc_Server_peer_cert",
nil,
nil,
nil,
nil,
verifyFunc,
false,
false,
false,
[]tls.Certificate{serverPeerCert},
nil,
nil,
nil,
false,
},
// Client: only set clientRoot
Expand Down

0 comments on commit 74939ed

Please sign in to comment.