From 2451248afd8d87c6dd423d22254e2f74c78aff95 Mon Sep 17 00:00:00 2001 From: Jeffrey Morgan <251292+jmorganca@users.noreply.github.com> Date: Tue, 10 Aug 2021 12:20:29 -0400 Subject: [PATCH] default to tls enabled with a warning error if tls verification fails (#173) --- internal/cmd/cmd.go | 6 +----- internal/engine/engine.go | 41 ++++++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 0a858bac90..4368433cd8 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -785,11 +785,7 @@ func newEngineCmd() *cobra.Command { }, } - skipTLSVerify := true - // TODO (https://github.com/infrahq/infra/issues/58): warn users instead of skipping TLS verification - // OR find a way to include the server certificate in the api key - // skipTLSVerify := len(os.Getenv("INFRA_ENGINE_SKIP_TLS_VERIFY")) > 0 - cmd.PersistentFlags().BoolVarP(&options.SkipTLSVerify, "skip-tls-verify", "k", skipTLSVerify, "skip TLS verification") + cmd.PersistentFlags().BoolVar(&options.ForceTLSVerify, "force-tls-verify", false, "force TLS verification") cmd.Flags().StringVarP(&options.Registry, "registry", "r", os.Getenv("INFRA_ENGINE_REGISTRY"), "registry hostname") cmd.Flags().StringVarP(&options.Name, "name", "n", os.Getenv("INFRA_ENGINE_NAME"), "cluster name") cmd.Flags().StringVarP(&options.Endpoint, "endpoint", "e", os.Getenv("INFRA_ENGINE_ENDPOINT"), "cluster endpoint") diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 2f4cfb9cca..7a084a2b0d 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -29,11 +29,11 @@ import ( ) type Options struct { - Registry string - Name string - Endpoint string - SkipTLSVerify bool - APIKey string + Registry string + Name string + Endpoint string + ForceTLSVerify bool + APIKey string } type RoleBinding struct { @@ -220,7 +220,32 @@ func Run(options Options) error { registry += ":443" } - creds := credentials.NewTLS(&tls.Config{InsecureSkipVerify: options.SkipTLSVerify}) + tlsConfig := &tls.Config{} + if !options.ForceTLSVerify { + // TODO (https://github.com/infrahq/infra/issues/174) + // Find a way to re-use the built-in TLS verification code vs + // this custom code based on the official go TLS example code + // which states this is approximately the same. + tlsConfig.InsecureSkipVerify = true + tlsConfig.VerifyConnection = func(cs tls.ConnectionState) error { + opts := x509.VerifyOptions{ + DNSName: cs.ServerName, + Intermediates: x509.NewCertPool(), + } + for _, cert := range cs.PeerCertificates[1:] { + opts.Intermediates.AddCert(cert) + } + _, err := cs.PeerCertificates[0].Verify(opts) + + if err != nil { + fmt.Println("Warning: could not verify registry TLS certificates: " + err.Error()) + } + + return nil + } + } + + creds := credentials.NewTLS(tlsConfig) conn, err := grpc.Dial(registry, grpc.WithTransportCredentials(creds), withClientAuthUnaryInterceptor(options.APIKey)) if err != nil { return err @@ -347,9 +372,7 @@ func Run(options Options) error { Transport: &BearerTransport{ Token: options.APIKey, Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: options.SkipTLSVerify, - }, + TLSClientConfig: tlsConfig, }, }, },