Skip to content

Commit

Permalink
Various TLS-related fixes (#874)
Browse files Browse the repository at this point in the history
- Adds ability to enable host validation to Internode/Frontend TLS configuration.
- Updates config-template.yaml to surface environment variables for both serverName and enabling of host validation.
- TCTL commands that leveraged the Go Client SDK were not setting the TLS configuration correctly on the Client, so those commands would fail against a Temporal Cluster with TLS enabled.
Tested against a private deployment
  • Loading branch information
mastermanu committed Oct 19, 2020
1 parent 05ccdf0 commit f3e0b03
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 11 deletions.
2 changes: 1 addition & 1 deletion common/rpc/encryption/localStoreTlsFactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,6 @@ func newClientTLSConfig(localProvider CertProvider, remoteProvider CertProvider)
clientCerts,
serverCa,
remoteProvider.GetSettings().Client.ServerName,
true,
!remoteProvider.GetSettings().Client.DisableHostVerification,
), nil
}
5 changes: 5 additions & 0 deletions common/service/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ type (
// This name should be referenced by the certificate specified in the ServerTLS section.
ServerName string `yaml:"serverName"`

// If you want to verify the temporal server hostname and server cert, then you should turn this on
// This option is basically equivalent to InSecureSkipVerify
// See InSecureSkipVerify in http://golang.org/pkg/crypto/tls/ for more info
DisableHostVerification bool `yaml:"disableHostVerification"`

// Optional - A list of paths to files containing the PEM-encoded public key of the Certificate Authorities that are used to validate the server's TLS certificate
// You cannot specify both RootCAFiles and RootCAData
RootCAFiles []string `yaml:"rootCaFiles"`
Expand Down
4 changes: 4 additions & 0 deletions docker/config_template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ global:
# This client section is used to configure the TLS clients within
# the Temporal Cluster that connect to an Internode (history or matching)
client:
serverName: {{ default .Env.TEMPORAL_TLS_INTERNODE_SERVER_NAME "" }}
disableHostVerification: {{ default .Env.TEMPORAL_TLS_INTERNODE_DISABLE_HOST_VERIFICATION "false"}}
rootCaFiles:
- {{ default .Env.TEMPORAL_TLS_SERVER_CA_CERT "" }}
rootCaData:
Expand All @@ -160,6 +162,8 @@ global:
# This client section is used to configure the TLS clients within
# the Temporal Cluster (specifically the Worker role) that connect to the Frontend service
client:
serverName: {{ default .Env.TEMPORAL_TLS_FRONTEND_SERVER_NAME "" }}
disableHostVerification: {{ default .Env.TEMPORAL_TLS_FRONTEND_DISABLE_HOST_VERIFICATION "false"}}
rootCaFiles:
- {{ default .Env.TEMPORAL_TLS_SERVER_CA_CERT "" }}
rootCaData:
Expand Down
2 changes: 1 addition & 1 deletion tools/cli/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func newAdminElasticSearchCommands() []cli.Command {
},
cli.StringFlag{
Name: FlagInputFileWithAlias,
Usage: "Input file name. Redirect temporal wf list result (with tale format) to a file and use as delete input. " +
Usage: "Input file name. Redirect temporal wf list result (with table format) to a file and use as delete input. " +
"First line should be table header like WORKFLOW TYPE | WORKFLOW ID | RUN ID | ...",
},
cli.IntFlag{
Expand Down
42 changes: 34 additions & 8 deletions tools/cli/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,18 @@ func (b *clientFactory) SDKClient(c *cli.Context, namespace string) sdkclient.Cl
hostPort = localHostPort
}

tlsConfig, err := b.createTLSConfig(c)
if err != nil {
b.logger.Fatal("Failed to configure TLS for SDK client", zap.Error(err))
}

sdkClient, err := sdkclient.NewClient(sdkclient.Options{
HostPort: hostPort,
Namespace: namespace,
Logger: log.NewZapAdapter(b.logger),
ConnectionOptions: sdkclient.ConnectionOptions{
DisableHealthCheck: true,
TLS: tlsConfig,
},
})
if err != nil {
Expand All @@ -107,6 +113,31 @@ func (b *clientFactory) createGRPCConnection(c *cli.Context) (*grpc.ClientConn,
if hostPort == "" {
hostPort = localHostPort
}

tlsConfig, err := b.createTLSConfig(c)
if err != nil {
return nil, err
}

grpcSecurityOptions := grpc.WithInsecure()

if tlsConfig != nil {
grpcSecurityOptions = grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))
}

connection, err := grpc.Dial(hostPort, grpcSecurityOptions)
if err != nil {
b.logger.Fatal("Failed to create connection", zap.Error(err))
return nil, err
}
return connection, nil
}

func (b *clientFactory) createTLSConfig(c *cli.Context) (*tls.Config, error) {
hostPort := c.GlobalString(FlagAddress)
if hostPort == "" {
hostPort = localHostPort
}
// Ignoring error as we'll fail to dial anyway, and that will produce a meaningful error
host, _, _ := net.SplitHostPort(hostPort)

Expand All @@ -115,7 +146,6 @@ func (b *clientFactory) createGRPCConnection(c *cli.Context) (*grpc.ClientConn,
caPath := c.GlobalString(FlagTLSCaPath)
hostNameVerification := c.GlobalBool(FlagTLSEnableHostVerification)

grpcSecurityOptions := grpc.WithInsecure()
var cert *tls.Certificate
var caPool *x509.CertPool

Expand Down Expand Up @@ -144,15 +174,11 @@ func (b *clientFactory) createGRPCConnection(c *cli.Context) (*grpc.ClientConn,
if cert != nil {
tlsConfig.Certificates = []tls.Certificate{*cert}
}
grpcSecurityOptions = grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))
}

connection, err := grpc.Dial(hostPort, grpcSecurityOptions)
if err != nil {
b.logger.Fatal("Failed to create connection", zap.Error(err))
return nil, err
return tlsConfig, nil
}
return connection, nil

return nil, nil
}

func fetchCACert(path string) (*x509.CertPool, error) {
Expand Down
1 change: 0 additions & 1 deletion tools/cli/workflowCommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,6 @@ func listClosedWorkflow(client client.Client, pageSize int, earliestTime, latest

func getListResultInRaw(c *cli.Context, queryOpen bool, nextPageToken []byte) ([]*workflowpb.WorkflowExecutionInfo, []byte) {
wfClient := getWorkflowClient(c)

earliestTime := parseTime(c.String(FlagEarliestTime), time.Time{}, time.Now().UTC())
latestTime := parseTime(c.String(FlagLatestTime), time.Now().UTC(), time.Now().UTC())
workflowID := c.String(FlagWorkflowID)
Expand Down

0 comments on commit f3e0b03

Please sign in to comment.