Tinkerbell GRPC Insecure Mode (#567)

## Description

This PR, along with some minor cleanup, enables tink-server to run without a TLS certificate and enables tink-cli to connect to it without needing TLS to be set up.

At the time of opening the PR, this change is intended to only extend the capabilities of tink-server and tink-cli rather than fundamentally altering them, though I am open to the latter before this lands if that's the right answer.

## Why is this needed

This will simplify development and testing of the tinkerbell stack, and could theoretically be used in the sandbox.
Alternately, the sandbox could include TLS termination similar to how many applications use Kubernetes ingress for TLS termination.

## How Has This Been Tested?

Template creation and retrieval
```
cat focal.yaml | docker exec -i tink_tink-cli_1 tink template create
docker exec -i tink_tink-cli_1 tink template get
```
- [x] against `make run-stack`
- [ ] with the sandbox

## How are existing users impacted? What migration steps/scripts do we need?

This is intended to not impact 

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

client/main.go

@@ -5,8 +5,8 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
-	"os"
 
+	"github.com/packethost/pkg/env"
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 	"github.com/tinkerbell/tink/protos/hardware"
@@ -44,42 +44,55 @@ func NewFullClient(conn grpc.ClientConnInterface) *FullClient {
 type ConnOptions struct {
 	CertURL       string
 	GRPCAuthority string
+	TLS           bool
 }
 
 func (o *ConnOptions) SetFlags(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&o.CertURL, "tinkerbell-cert-url", "http://127.0.0.1:42114/cert", "The URL where the certificate is located")
-	flagSet.StringVar(&o.GRPCAuthority, "tinkerbell-grpc-authority", "127.0.0.1:42113", "Link to tink-server grcp api")
+	flagSet.StringVar(&o.GRPCAuthority, "tinkerbell-grpc-authority", "127.0.0.1:42113", "Connection info for tink-server")
+	flagSet.BoolVar(&o.TLS, "tinkerbell-tls", true, "Connect to server via TLS or not")
 }
 
 // This function is bad and ideally should be removed, but for now it moves all the bad into one place.
 // This is the legacy of packethost/cacher running behind an ingress that couldn't terminate TLS on behalf
 // of GRPC. All of this functionality should be ripped out in favor of either using trusted certificates
-// or moving the establishment of trust in the certificate out to the environment (or running in insecure mode
+// or moving the establishment of trust in the certificate out to the environment (or running in no-tls mode
 // e.g. for development.)
 func grpcCredentialFromCertEndpoint(url string) (credentials.TransportCredentials, error) {
 	resp, err := http.Get(url)
 	if err != nil {
 		return nil, errors.Wrap(err, "fetch cert")
 	}
 	defer resp.Body.Close()
+
 	certs, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return nil, errors.Wrap(err, "read cert")
 	}
+
 	cp := x509.NewCertPool()
 	ok := cp.AppendCertsFromPEM(certs)
 	if !ok {
 		return nil, errors.Wrap(err, "parse cert")
 	}
+
 	return credentials.NewClientTLSFromCert(cp, ""), nil
 }
 
 func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
-	creds, err := grpcCredentialFromCertEndpoint(opt.CertURL)
-	if err != nil {
-		return nil, errors.Wrap(err, "obtain trusted certificate")
+	method := grpc.WithInsecure()
+	if opt.TLS {
+		creds, err := grpcCredentialFromCertEndpoint(opt.CertURL)
+		if err != nil {
+			return nil, err
+		}
+		method = grpc.WithTransportCredentials(creds)
 	}
-	conn, err := grpc.Dial(opt.GRPCAuthority, grpc.WithTransportCredentials(creds))
+	conn, err := grpc.Dial(opt.GRPCAuthority,
+		method,
+		grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),
+		grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()),
+	)
 	if err != nil {
 		return nil, errors.Wrap(err, "connect to tinkerbell server")
 	}
@@ -88,29 +101,22 @@ func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
 
 // GetConnection returns a gRPC client connection.
 func GetConnection() (*grpc.ClientConn, error) {
-	certURL := os.Getenv("TINKERBELL_CERT_URL")
-	if certURL == "" {
-		return nil, errors.New("undefined TINKERBELL_CERT_URL")
+	opts := ConnOptions{
+		CertURL:       env.Get("TINKERBELL_CERT_URL"),
+		GRPCAuthority: env.Get("TINKERBELL_GRPC_AUTHORITY"),
+		TLS:           env.Bool("TINKERBELL_TLS", true),
 	}
 
-	grpcAuthority := os.Getenv("TINKERBELL_GRPC_AUTHORITY")
-	if grpcAuthority == "" {
+	if opts.GRPCAuthority == "" {
 		return nil, errors.New("undefined TINKERBELL_GRPC_AUTHORITY")
 	}
 
-	creds, err := grpcCredentialFromCertEndpoint(certURL)
-	if err != nil {
-		return nil, errors.Wrap(err, "obtain trusted certificate")
+	if opts.TLS {
+		if opts.CertURL == "" {
+			return nil, errors.New("undefined TINKERBELL_CERT_URL")
+		}
 	}
-	conn, err := grpc.Dial(grpcAuthority,
-		grpc.WithTransportCredentials(creds),
-		grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),
-		grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()),
-	)
-	if err != nil {
-		return nil, errors.Wrap(err, "connect to tinkerbell server")
-	}
-	return conn, nil
+	return NewClientConn(&opts)
 }
 
 // Setup : create a connection to server.

cmd/tink-server/main.go

@@ -6,17 +6,17 @@ import (
 	"fmt"
 	"os"
 	"os/signal"
-	"strconv"
 	"strings"
 	"syscall"
 
 	"github.com/equinix-labs/otel-init-go/otelinit"
+	"github.com/packethost/pkg/env"
 	"github.com/packethost/pkg/log"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"github.com/tinkerbell/tink/db"
-	rpcServer "github.com/tinkerbell/tink/grpc-server"
+	grpcServer "github.com/tinkerbell/tink/grpc-server"
 	httpServer "github.com/tinkerbell/tink/http-server"
 	"github.com/tinkerbell/tink/metrics"
 )
@@ -37,6 +37,7 @@ type DaemonConfig struct {
 	TLSCert       string
 	CertDir       string
 	HTTPAuthority string
+	TLS           bool
 }
 
 func (c *DaemonConfig) AddFlags(fs *pflag.FlagSet) {
@@ -50,41 +51,23 @@ func (c *DaemonConfig) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&c.TLSCert, "tls-cert", "", "")
 	fs.StringVar(&c.CertDir, "cert-dir", "", "")
 	fs.StringVar(&c.HTTPAuthority, "http-authority", ":42114", "The address used to expose the HTTP server")
+	fs.BoolVar(&c.TLS, "tls", true, "Run in tls protected mode (disabling should only be done for development or if behind TLS terminating proxy)")
 }
 
 func (c *DaemonConfig) PopulateFromLegacyEnvVar() {
-	if f := os.Getenv("FACILITY"); f != "" {
-		c.Facility = f
-	}
-	if pgdb := os.Getenv("PGDATABASE"); pgdb != "" {
-		c.PGDatabase = pgdb
-	}
-	if pguser := os.Getenv("PGUSER"); pguser != "" {
-		c.PGUSer = pguser
-	}
-	if pgpass := os.Getenv("PGPASSWORD"); pgpass != "" {
-		c.PGPassword = pgpass
-	}
-	if pgssl := os.Getenv("PGSSLMODE"); pgssl != "" {
-		c.PGSSLMode = pgssl
-	}
-	if onlyMigration, isSet := os.LookupEnv("ONLY_MIGRATION"); isSet {
-		if b, err := strconv.ParseBool(onlyMigration); err != nil {
-			c.OnlyMigration = b
-		}
-	}
-	if tlsCert := os.Getenv("TINKERBELL_TLS_CERT"); tlsCert != "" {
-		c.TLSCert = tlsCert
-	}
-	if certDir := os.Getenv("TINKERBELL_CERTS_DIR"); certDir != "" {
-		c.CertDir = certDir
-	}
-	if grpcAuthority := os.Getenv("TINKERBELL_GRPC_AUTHORITY"); grpcAuthority != "" {
-		c.GRPCAuthority = grpcAuthority
-	}
-	if httpAuthority := os.Getenv("TINKERBELL_HTTP_AUTHORITY"); httpAuthority != "" {
-		c.HTTPAuthority = httpAuthority
-	}
+	c.Facility = env.Get("FACILITY", c.Facility)
+
+	c.PGDatabase = env.Get("PGDATABASE", c.PGDatabase)
+	c.PGUSer = env.Get("PGUSER", c.PGUSer)
+	c.PGPassword = env.Get("PGPASSWORD", c.PGPassword)
+	c.PGSSLMode = env.Get("PGSSLMODE", c.PGSSLMode)
+	c.OnlyMigration = env.Bool("ONLY_MIGRATION", c.OnlyMigration)
+
+	c.TLSCert = env.Get("TINKERBELL_TLS_CERT", c.TLSCert)
+	c.CertDir = env.Get("TINKERBELL_CERTS_DIR", c.CertDir)
+	c.GRPCAuthority = env.Get("TINKERBELL_GRPC_AUTHORITY", c.GRPCAuthority)
+	c.HTTPAuthority = env.Get("TINKERBELL_HTTP_AUTHORITY", c.HTTPAuthority)
+	c.TLS = env.Bool("TINKERBELL_LS", c.TLS)
 }
 
 func main() {
@@ -172,18 +155,23 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command {
 				logger.Info("Your database schema is not up to date. Please apply migrations running tink-server with env var ONLY_MIGRATION set.")
 			}
 
-			cert, modT := rpcServer.SetupGRPC(ctx, logger, &rpcServer.ConfigGRPCServer{
+			grpcConfig := &grpcServer.ConfigGRPCServer{
 				Facility:      config.Facility,
-				TLSCert:       config.TLSCert,
+				TLSCert:       "insecure",
 				GRPCAuthority: config.GRPCAuthority,
 				DB:            tinkDB,
-			}, errCh)
+			}
+			if config.TLS {
+				grpcConfig.TLSCert = config.TLSCert
+			}
+			cert, modT := grpcServer.SetupGRPC(ctx, logger, grpcConfig, errCh)
 
-			httpServer.SetupHTTP(ctx, logger, &httpServer.Config{
+			httpConfig := &httpServer.Config{
+				HTTPAuthority: config.HTTPAuthority,
 				CertPEM:       cert,
 				ModTime:       modT,
-				HTTPAuthority: config.HTTPAuthority,
-			}, errCh)
+			}
+			httpServer.SetupHTTP(ctx, logger, httpConfig, errCh)
 
 			select {
 			case err = <-errCh:

docker-compose.yaml

@@ -1,21 +1,9 @@
 version: "3.8"
 services:
-  tls-gen:
-    image: cfssl/cfssl
-    entrypoint: /bin/bash
-    command:
-      - /code/tls/generate.sh
-    environment:
-      FACILITY: ${FACILITY:-onprem}
-    volumes:
-      - ${PWD}/deploy:/code
-      - certs:/certs/${FACILITY:-onprem}:rw
-
   tinkerbell:
     build:
       context: ./cmd/tink-server/
       dockerfile: Dockerfile
-    restart: unless-stopped
     environment:
       FACILITY: ${FACILITY:-onprem}
       PACKET_ENV: ${PACKET_ENV:-testing}
@@ -30,27 +18,39 @@ services:
       PGUSER: tinkerbell
       TINKERBELL_GRPC_AUTHORITY: :42113
       TINKERBELL_HTTP_AUTHORITY: :42114
+      TINKERBELL_TLS: ${TINKERBERLL_TLS:"true"}
+    volumes:
+      - certs:/certs/${FACILITY:-onprem}:rw
+    ports:
+      - 42113:42113/tcp
+      - 42114:42114/tcp
     depends_on:
-      tink-server-migration:
-        condition: service_started
       db:
         condition: service_healthy
+      tink-server-migration:
+        condition: service_started
       tls-gen:
         condition: service_started
-    volumes:
-      - certs:/certs/${FACILITY:-onprem}:rw
     healthcheck:
-      test: ["CMD-SHELL", "wget -qO- 127.0.0.1:42114/cert"] # port needs to match TINKERBELL_HTTP_AUTHORITY
+      test: ["CMD-SHELL", "wget -qO- 127.0.0.1:42114/healthz"] # port needs to match TINKERBELL_HTTP_AUTHORITY
       interval: 5s
       timeout: 2s
       retries: 3
-    ports:
-      - 42113:42113/tcp
-      - 42114:42114/tcp
+    restart: unless-stopped
+
+  tls-gen:
+    image: cfssl/cfssl
+    entrypoint: /bin/bash
+    command:
+      - /code/tls/generate.sh
+    environment:
+      FACILITY: ${FACILITY:-onprem}
+    volumes:
+      - ${PWD}/deploy:/code
+      - certs:/certs/${FACILITY:-onprem}:rw
 
   tink-server-migration:
     image: quay.io/tinkerbell/tink:latest
-    restart: on-failure
     environment:
       ONLY_MIGRATION: "true"
       FACILITY: ${FACILITY:-onprem}
@@ -62,13 +62,14 @@ services:
       PGUSER: tinkerbell
       TINKERBELL_GRPC_AUTHORITY: :42113
       TINKERBELL_HTTP_AUTHORITY: :42114
+      TINKERBELL_TLS: ${TINKERBERLL_TLS:"true"}
     depends_on:
       db:
         condition: service_healthy
+    restart: on-failure
 
   db:
     image: postgres:14-alpine
-    restart: unless-stopped
     environment:
       POSTGRES_DB: tinkerbell
       POSTGRES_PASSWORD: tinkerbell
@@ -82,20 +83,20 @@ services:
       interval: 1s
       timeout: 1s
       retries: 30
+    restart: unless-stopped
 
   tink-cli:
     build:
       context: ./cmd/tink-cli/
       dockerfile: Dockerfile
-    restart: unless-stopped
     environment:
-      TINKERBELL_GRPC_AUTHORITY: tinkerbell:42113
       TINKERBELL_CERT_URL: http://tinkerbell:42114/cert
+      TINKERBELL_GRPC_AUTHORITY: tinkerbell:42113
+      TINKERBELL_TLS: ${TINKERBERLL_TLS:"true"}
     depends_on:
       tinkerbell:
         condition: service_healthy
-      db:
-        condition: service_healthy
+    restart: unless-stopped
 
 volumes:
   postgres_data: