feat: fine-grained control over transport security

This adds two new flags to the Keto CLI: * --insecure-disable-transport-security: Use plaintext instead of TLS * --insecure-skip-hostname-verification: Use TLS, but do not verify the certificate By default, the Keto CLI now connects to the remote via TLS and verifies the hostname.
ory · Sep 1, 2022 · 5f056b7 · 5f056b7
1 parent 5110f63
commit 5f056b7
Show file tree

Hide file tree

Showing 18 changed files with 100 additions and 59 deletions.
diff --git a/cmd/check/root_test.go b/cmd/check/root_test.go
@@ -14,6 +14,8 @@ func TestCheckCommand(t *testing.T) {
 	ts := client.NewTestServer(t, client.ReadServer, []*namespace.Namespace{nspace}, newCheckCmd)
 	defer ts.Shutdown(t)
 
-	stdOut := ts.Cmd.ExecNoErr(t, "subject", "access", nspace.Name, "object")
+	stdOut := ts.Cmd.ExecNoErr(t, "subject", "access", nspace.Name, "object",
+		"--insecure-skip-hostname-verification=true",
+	)
 	assert.Equal(t, "Denied\n", stdOut)
 }
diff --git a/cmd/client/grpc_client.go b/cmd/client/grpc_client.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
-	"net"
 	"os"
 	"strings"
 	"time"
@@ -23,9 +22,11 @@ import (
 type contextKeys string
 
 const (
-	FlagReadRemote        = "read-remote"
-	FlagWriteRemote       = "write-remote"
-	FlagInsecureTransport = "insecure"
+	FlagReadRemote  = "read-remote"
+	FlagWriteRemote = "write-remote"
+
+	FlagInsecureNoTransportSecurity  = "insecure-disable-transport-security"
+	FlagInsecureSkipHostVerification = "insecure-skip-hostname-verification"
 
 	EnvReadRemote  = "KETO_READ_REMOTE"
 	EnvWriteRemote = "KETO_WRITE_REMOTE"
@@ -34,6 +35,28 @@ const (
 	ContextKeyTimeout contextKeys = "timeout"
 )
 
+type securityFlags struct {
+	skipHostVerification bool
+	noTransportSecurity  bool
+}
+
+func (sf *securityFlags) transportCredentials() grpc.DialOption {
+	switch {
+	case sf.noTransportSecurity:
+		return grpc.WithTransportCredentials(insecure.NewCredentials())
+
+	case sf.skipHostVerification:
+		return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
+			// nolint explicity set through scary flag
+			InsecureSkipVerify: true,
+		}))
+
+	default:
+		// Defaults to the default host root CA bundle
+		return grpc.WithTransportCredentials(credentials.NewTLS(nil))
+	}
+}
+
 func getRemote(cmd *cobra.Command, flagRemote, envRemote string) (remote string) {
 	defer (func() {
 		if strings.HasPrefix(remote, "http://") || strings.HasPrefix(remote, "https://") {
@@ -57,23 +80,30 @@ func getToken(cmd *cobra.Command) (token string) {
 	return os.Getenv(EnvAuthToken)
 }
 
+func getSecurityFlags(cmd *cobra.Command) securityFlags {
+	return securityFlags{
+		skipHostVerification: flagx.MustGetBool(cmd, FlagInsecureSkipHostVerification),
+		noTransportSecurity:  flagx.MustGetBool(cmd, FlagInsecureNoTransportSecurity),
+	}
+}
+
 func GetReadConn(cmd *cobra.Command) (*grpc.ClientConn, error) {
 	return Conn(cmd.Context(),
 		getRemote(cmd, FlagReadRemote, EnvReadRemote),
 		getToken(cmd),
-		flagx.MustGetBool(cmd, FlagInsecureTransport),
+		getSecurityFlags(cmd),
 	)
 }
 
 func GetWriteConn(cmd *cobra.Command) (*grpc.ClientConn, error) {
 	return Conn(cmd.Context(),
 		getRemote(cmd, FlagWriteRemote, EnvWriteRemote),
 		getToken(cmd),
-		flagx.MustGetBool(cmd, FlagInsecureTransport),
+		getSecurityFlags(cmd),
 	)
 }
 
-func Conn(ctx context.Context, remote, token string, insecureTransport bool) (*grpc.ClientConn, error) {
+func Conn(ctx context.Context, remote, token string, security securityFlags) (*grpc.ClientConn, error) {
 	timeout := 3 * time.Second
 	if d, ok := ctx.Value(ContextKeyTimeout).(time.Duration); ok {
 		timeout = d
@@ -86,7 +116,7 @@ func Conn(ctx context.Context, remote, token string, insecureTransport bool) (*g
 		grpc.WithBlock(),
 		grpc.WithDisableHealthCheck(),
 	}
-	dialOpts = append(dialOpts, transportCredentials(remote, insecureTransport))
+	dialOpts = append(dialOpts, security.transportCredentials())
 	if token != "" {
 		dialOpts = append(dialOpts,
 			grpc.WithPerRPCCredentials(
@@ -96,25 +126,9 @@ func Conn(ctx context.Context, remote, token string, insecureTransport bool) (*g
 	return grpc.DialContext(ctx, remote, dialOpts...)
 }
 
-func transportCredentials(remote string, insecureTransport bool) grpc.DialOption {
-	if insecureTransport {
-		return grpc.WithTransportCredentials(insecure.NewCredentials())
-	}
-
-	host, _, err := net.SplitHostPort(remote)
-	if err == nil && (host == "127.0.0.1" || host == "localhost") {
-		return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
-			// nolint only set for local domain.
-			InsecureSkipVerify: true,
-		}))
-	}
-
-	// Defaults to the default host root CA bundle
-	return grpc.WithTransportCredentials(credentials.NewTLS(nil))
-}
-
 func RegisterRemoteURLFlags(flags *pflag.FlagSet) {
 	flags.String(FlagReadRemote, "127.0.0.1:4466", "Remote address of the read API endpoint.")
 	flags.String(FlagWriteRemote, "127.0.0.1:4467", "Remote address of the write API endpoint.")
-	flags.Bool(FlagInsecureTransport, false, "Do not use transport encryption.")
+	flags.Bool(FlagInsecureNoTransportSecurity, false, "Disables transport security. Do not use this in production.")
+	flags.Bool(FlagInsecureSkipHostVerification, false, "Disables hostname verification. Do not use this in production.")
 }
diff --git a/cmd/client/test_helpers.go b/cmd/client/test_helpers.go
@@ -71,9 +71,12 @@ func NewTestServer(t *testing.T,
 	})
 
 	ts.Cmd = &cmdx.CommandExecuter{
-		New:            newCmd,
-		PersistentArgs: []string{"--" + ts.FlagRemote, ts.Addr},
-		Ctx:            ctx,
+		New: newCmd,
+		PersistentArgs: []string{
+			"--" + ts.FlagRemote, ts.Addr,
+			"--insecure-skip-hostname-verification=true",
+		},
+		Ctx: ctx,
 	}
 
 	return ts

diff --git a/cmd/expand/root_test.go b/cmd/expand/root_test.go
@@ -18,15 +18,19 @@ func TestExpandCommand(t *testing.T) {
 	t.Run("case=unknown tuple", func(t *testing.T) {
 		t.Run("format=JSON", func(t *testing.T) {
 			stdOut := ts.Cmd.ExecNoErr(t,
-				"access", nspace.Name,
-				"object", "--"+cmdx.FlagFormat, string(cmdx.FormatJSON))
+				"access", nspace.Name, "object",
+				"--"+cmdx.FlagFormat, string(cmdx.FormatJSON),
+				"--insecure-skip-hostname-verification=true",
+			)
 			assert.Equal(t, "null\n", stdOut)
 		})
 
 		t.Run("format=default", func(t *testing.T) {
 			stdOut := ts.Cmd.ExecNoErr(t,
 				"access", nspace.Name,
-				"object", "--"+cmdx.FlagFormat, string(cmdx.FormatDefault))
+				"object", "--"+cmdx.FlagFormat, string(cmdx.FormatDefault),
+				"--insecure-skip-hostname-verification=true",
+			)
 			assert.Contains(t, stdOut, "empty tree")
 		})
 	})

diff --git a/cmd/relationtuple/delete_all_test.go b/cmd/relationtuple/delete_all_test.go
@@ -16,7 +16,7 @@ func TestDeleteAllCmd(t *testing.T) {
 
 		cmd := newDeleteAllCmd()
 		// we will get an error here because there is no server running, but we really only care about the execution path
-		stdout, _, _ := cmdx.ExecCtx(ctx, cmd, nil)
+		stdout, _, _ := cmdx.ExecCtx(ctx, cmd, nil, "--insecure-skip-hostname-verification=true")
 		assert.Contains(t, stdout, "WARNING: This operation is not reversible.")
 	})
 }
diff --git a/cmd/status/root_test.go b/cmd/status/root_test.go
@@ -76,14 +76,11 @@ func TestStatusCmd(t *testing.T) {
 					},
 				}
 
-				// TODO remove
-				// close(startServe)
-
 				require.NoError(t,
 					cmdx.ExecBackgroundCtx(ctx, newStatusCmd(), &stdIn, &stdOut, &stdErr,
 						"--"+FlagEndpoint, string(serverType),
 						"--"+ts.FlagRemote, l.Addr().String(),
-						// TODO uncomment
+						"--insecure-skip-hostname-verification=true",
 						"--"+FlagBlock,
 					).Wait(),
 				)

diff --git a/contrib/docs-code-samples/expand-api-display-access/00-create-tuples/cli.sh b/contrib/docs-code-samples/expand-api-display-access/00-create-tuples/cli.sh
@@ -18,6 +18,6 @@ files:/photos/beach.jpg#access@(directories:/photos#access)
 files:/photos/mountains.jpg#access@(files:/photos/mountains.jpg#owner)
 files:/photos/mountains.jpg#access@(directories:/photos#access)' | \
   keto relation-tuple parse - --format json | \
-    keto relation-tuple create --insecure - >/dev/null \
+    keto relation-tuple create --insecure-disable-transport-security - >/dev/null \
     && echo "Successfully created tuples" \
     || echo "Encountered error"
diff --git a/contrib/docs-code-samples/expand-api-display-access/01-expand-beach/cli.sh b/contrib/docs-code-samples/expand-api-display-access/01-expand-beach/cli.sh
@@ -3,4 +3,4 @@ set -euo pipefail
 
 export KETO_READ_REMOTE="127.0.0.1:4466"
 
-keto expand access files /photos/beach.jpg --format json-pretty --max-depth 3 --insecure
+keto expand access files /photos/beach.jpg --format json-pretty --max-depth 3 --insecure-disable-transport-security
diff --git a/contrib/docs-code-samples/expand-api-display-access/99-cleanup/cli.sh b/contrib/docs-code-samples/expand-api-display-access/99-cleanup/cli.sh
@@ -4,10 +4,10 @@ set -euo pipefail
 export KETO_READ_REMOTE="127.0.0.1:4466"
 export KETO_WRITE_REMOTE="127.0.0.1:4467"
 
-keto relation-tuple get --namespace files --format json --insecure | \
+keto relation-tuple get --namespace files --format json --insecure-disable-transport-security | \
   jq ".relation_tuples" | \
-    keto relation-tuple delete --insecure - -q > /dev/null
+    keto relation-tuple delete --insecure-disable-transport-security - -q > /dev/null
 
-keto relation-tuple get --namespace directories --format json --insecure | \
+keto relation-tuple get --namespace directories --format json --insecure-disable-transport-security | \
   jq ".relation_tuples" | \
-    keto relation-tuple delete --insecure - -q > /dev/null
+    keto relation-tuple delete --insecure-disable-transport-security - -q > /dev/null
diff --git a/contrib/docs-code-samples/list-api-display-objects/00-create-tuples/cli.sh b/contrib/docs-code-samples/list-api-display-objects/00-create-tuples/cli.sh
@@ -15,6 +15,6 @@ chats:coffee-break#member@Vincent
 chats:coffee-break#member@Julia
 chats:coffee-break#member@Patrik' | \
   keto relation-tuple parse - --format json | \
-    keto relation-tuple create - >/dev/null --insecure \
+    keto relation-tuple create - >/dev/null --insecure-disable-transport-security \
     && echo "Successfully created tuples" \
     || echo "Encountered error"
diff --git a/contrib/docs-code-samples/list-api-display-objects/01-list-PM/cli.sh b/contrib/docs-code-samples/list-api-display-objects/01-list-PM/cli.sh
@@ -3,5 +3,5 @@ set -euo pipefail
 
 export KETO_READ_REMOTE="127.0.0.1:4466"
 
-keto relation-tuple get --namespace chats --relation member --subject-id PM --format json --insecure | \
+keto relation-tuple get --namespace chats --relation member --subject-id PM --format json --insecure-disable-transport-security | \
   jq ".relation_tuples[] | .object" -r | sort
diff --git a/contrib/docs-code-samples/list-api-display-objects/02-list-coffee-break/cli.sh b/contrib/docs-code-samples/list-api-display-objects/02-list-coffee-break/cli.sh
@@ -3,5 +3,5 @@ set -euo pipefail
 
 export KETO_READ_REMOTE="127.0.0.1:4466"
 
-keto relation-tuple get --namespace chats --object coffee-break --relation member --format json --insecure | \
+keto relation-tuple get --namespace chats --object coffee-break --relation member --format json --insecure-disable-transport-security | \
   jq "[.relation_tuples[] | .subject_id] | sort | .[]" -r
diff --git a/contrib/docs-code-samples/list-api-display-objects/99-cleanup/cli.sh b/contrib/docs-code-samples/list-api-display-objects/99-cleanup/cli.sh
@@ -4,6 +4,6 @@ set -euo pipefail
 export KETO_READ_REMOTE="127.0.0.1:4466"
 export KETO_WRITE_REMOTE="127.0.0.1:4467"
 
-keto relation-tuple get --namespace chats --format json --insecure | \
+keto relation-tuple get --namespace chats --format json --insecure-disable-transport-security | \
   jq ".relation_tuples" | \
-    keto relation-tuple delete - -q > /dev/null --insecure
+    keto relation-tuple delete - -q > /dev/null --insecure-disable-transport-security
diff --git a/contrib/docs-code-samples/simple-access-check-guide/00-write-direct-access/cli.sh b/contrib/docs-code-samples/simple-access-check-guide/00-write-direct-access/cli.sh
@@ -5,6 +5,6 @@ export KETO_WRITE_REMOTE="127.0.0.1:4467"
 
 echo "messages:02y_15_4w350m3#decypher@john" | \
   keto relation-tuple parse - --format json | \
-  keto relation-tuple create - >/dev/null --insecure \
+  keto relation-tuple create - >/dev/null --insecure-disable-transport-security \
     && echo "Successfully created tuple" \
     || echo "Encountered error"
diff --git a/contrib/docs-code-samples/simple-access-check-guide/01-check-direct-access/cli.sh b/contrib/docs-code-samples/simple-access-check-guide/01-check-direct-access/cli.sh
@@ -3,4 +3,4 @@ set -euo pipefail
 
 export KETO_READ_REMOTE="127.0.0.1:4466"
 
-keto check john decypher messages 02y_15_4w350m3 --insecure
+keto check john decypher messages 02y_15_4w350m3 --insecure-disable-transport-security
diff --git a/contrib/docs-code-samples/simple-access-check-guide/99-cleanup/cli.sh b/contrib/docs-code-samples/simple-access-check-guide/99-cleanup/cli.sh
@@ -11,4 +11,4 @@ relationtuple='
   "subject_id": "john"
 }'
 
-keto relation-tuple delete <(echo "$relationtuple") -q > /dev/null --insecure
+keto relation-tuple delete <(echo "$relationtuple") -q > /dev/null --insecure-disable-transport-security
diff --git a/internal/driver/registry_factory.go b/internal/driver/registry_factory.go
@@ -5,6 +5,9 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
+	"crypto/tls"
+	"fmt"
+	"sync"
 	"testing"
 
 	"github.com/ory/x/configx"
@@ -89,21 +92,39 @@ func WithGRPCUnaryInterceptors(i ...grpc.UnaryServerInterceptor) TestRegistryOpt
 		r.defaultUnaryInterceptors = i
 	}
 }
-func WithSelfsignedTransportCredentials() TestRegistryOption {
-	return func(t testing.TB, r *RegistryDefault) {
+
+type selfSignedCert struct {
+	once sync.Once
+	cert *tls.Certificate
+	err  error
+}
+
+var sharedTestCert selfSignedCert
+
+func (s *selfSignedCert) generate() {
+	s.once.Do(func() {
 		key, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader)
 		if err != nil {
-			t.Errorf("could not create key: %v", err)
+			s.err = fmt.Errorf("could not create key: %v", err)
 			return
 		}
 
-		tlsCert, err := tlsx.CreateSelfSignedTLSCertificate(key)
+		s.cert, err = tlsx.CreateSelfSignedTLSCertificate(key)
 		if err != nil {
-			t.Errorf("could not create TLS certificate: %v", err)
+			s.err = fmt.Errorf("could not create TLS certificate: %v", err)
+		}
+	})
+}
+
+func WithSelfsignedTransportCredentials() TestRegistryOption {
+	return func(t testing.TB, r *RegistryDefault) {
+		sharedTestCert.generate()
+		if sharedTestCert.err != nil {
+			t.Error(sharedTestCert.err)
 			return
 		}
 
-		r.grpcTransportCredentials = credentials.NewServerTLSFromCert(tlsCert)
+		r.grpcTransportCredentials = credentials.NewServerTLSFromCert(sharedTestCert.cert)
 	}
 }
 

diff --git a/internal/e2e/full_suit_test.go b/internal/e2e/full_suit_test.go
@@ -76,7 +76,7 @@ func Test(t *testing.T) {
 					PersistentArgs: []string{
 						"--" + cliclient.FlagReadRemote, reg.Config(ctx).ReadAPIListenOn(),
 						"--" + cliclient.FlagWriteRemote, reg.Config(ctx).WriteAPIListenOn(),
-						"--insecure=true",
+						"--insecure-disable-transport-security=true",
 						"--" + cmdx.FlagFormat, string(cmdx.FormatJSON),
 					},
 				}},