Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tinkerbell GRPC Insecure Mode #567

Merged
merged 10 commits into from
Feb 11, 2022
+132 −129
Merged

Tinkerbell GRPC Insecure Mode #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f71358
client: support connecting to GRPC servers without TLS
nshalman Dec 10, 2021
12e37dc
docs: Sort envvars table
mmlb Jan 27, 2022
4af9470
client: Use packethost/pkg/env helper functions instead of stdlib
mmlb Jan 18, 2022
c91e7b8
client: refactor GetConnection to use NewClientConn
mmlb Jan 18, 2022
aba55c1
http: Do the right thing if no cert is passed to SetupHTTP
mmlb Jan 24, 2022
1f564b4
Minor tweak of docker-compose
mmlb Jan 25, 2022
be6b4ac
Use /healthz for docker-compose healthcheck
mmlb Jan 25, 2022
ddde0da
tink-server: Use packethost/pkg/env helper functions instead of stdlib
mmlb Jan 18, 2022
a21393b
tink-server: support GRPC insecure mode
nshalman Dec 10, 2021
ae0df5f
docs: Add TINKERBELL_TLS ENVVARS.md
mmlb Jan 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 30 additions & 24 deletions client/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we didn't create this issue, but I just realized that err here is nil.

An improvement would be decoding the certificate's PEM and parsing the resulting ASN.1 DER using the x509 library. That, or creating a new error that says something like failed to parse cert - cert pool append certs from pem failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd agree, but I'm actually looking to drop the /cert completely so this will go away anyway.

}

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()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent :)

Might want to add a client option to print the trace id in this case e.g. in the logs, so it's easy to take to a search box.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example elsewhere of how to do that I can crib from?

)
if err != nil {
return nil, errors.Wrap(err, "connect to tinkerbell server")
}
Expand All @@ -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.
Expand Down
68 changes: 28 additions & 40 deletions cmd/tink-server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -37,6 +37,7 @@ type DaemonConfig struct {
TLSCert string
CertDir string
HTTPAuthority string
TLS bool
}

func (c *DaemonConfig) AddFlags(fs *pflag.FlagSet) {
Expand All @@ -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() {
Expand Down Expand Up @@ -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:
Expand Down
53 changes: 27 additions & 26 deletions docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}
Expand All @@ -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}
Expand All @@ -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
Expand All @@ -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:
Expand Down
Loading