Skip to content

Commit

Permalink
fix: null mTLS certificate causes panic (#42)
Browse files Browse the repository at this point in the history
  • Loading branch information
hgiasac authored Dec 12, 2024
1 parent 588255c commit 849466c
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 21 deletions.
50 changes: 49 additions & 1 deletion connector/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ func (mts *mockTLSServer) createMockTLSServer(t *testing.T, dir string) *httptes
tlsConfig := &tls.Config{
ClientCAs: caCertPool,
Certificates: []tls.Certificate{cert},
ClientAuth: tls.RequireAndVerifyClientCert,
ClientAuth: tls.RequestClientCert,
}

server := httptest.NewUnstartedServer(mux)
Expand Down Expand Up @@ -1448,10 +1448,58 @@ func TestConnectorTLS(t *testing.T) {
})
}()

time.Sleep(1)
assert.Equal(t, 1, mockServer.Count())
assert.Equal(t, 1, mockServer1.Count())
}

func TestConnectorTLSInsecure(t *testing.T) {
mockServer := &mockTLSServer{}
server := mockServer.createMockTLSServer(t, "testdata/tls/certs")
defer server.Close()

t.Setenv("PET_STORE_URL", server.URL)
t.Setenv("PET_STORE_S1_URL", server.URL)
t.Setenv("PET_STORE_INSECURE_SKIP_VERIFY", "true")

connServer, err := connector.NewServer(NewHTTPConnector(), &connector.ServerOptions{
Configuration: "testdata/tls",
}, connector.WithoutRecovery())
assert.NilError(t, err)
testServer := connServer.BuildTestServer()
defer testServer.Close()

func() {
findPetsBody := []byte(`{
"collection": "findPets",
"query": {
"fields": {
"__value": {
"type": "column",
"column": "__value"
}
}
},
"arguments": {},
"collection_relationships": {}
}`)

res, err := http.Post(fmt.Sprintf("%s/query", testServer.URL), "application/json", bytes.NewBuffer(findPetsBody))
assert.NilError(t, err)
assertHTTPResponse(t, res, http.StatusOK, schema.QueryResponse{
{
Rows: []map[string]any{
{
"__value": []any{},
},
},
},
})
}()

assert.Equal(t, 1, mockServer.Count())
}

func TestConnectorArgumentPresets(t *testing.T) {
mux := http.NewServeMux()
writeResponse := func(w http.ResponseWriter, data []byte) {
Expand Down
34 changes: 23 additions & 11 deletions connector/internal/security/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/base64"
"errors"
"fmt"
"log/slog"
"net/http"
"os"
"path/filepath"
Expand All @@ -16,13 +17,13 @@ import (
var systemCertPool = x509.SystemCertPool

// NewHTTPClientTLS creates a new HTTP Client with TLS configuration.
func NewHTTPClientTLS(baseClient *http.Client, tlsConfig *schema.TLSConfig) (*http.Client, error) {
func NewHTTPClientTLS(baseClient *http.Client, tlsConfig *schema.TLSConfig, logger *slog.Logger) (*http.Client, error) {
baseTransport, ok := baseClient.Transport.(*http.Transport)
if !ok {
baseTransport, _ = http.DefaultTransport.(*http.Transport)
}

tlsCfg, err := loadTLSConfig(tlsConfig)
tlsCfg, err := loadTLSConfig(tlsConfig, logger)
if err != nil {
return nil, fmt.Errorf("failed to load TLS config: %w", err)
}
Expand All @@ -40,7 +41,7 @@ func NewHTTPClientTLS(baseClient *http.Client, tlsConfig *schema.TLSConfig) (*ht

// loadTLSConfig loads TLS certificates and returns a tls.Config.
// This will set the RootCAs and Certificates of a tls.Config.
func loadTLSConfig(tlsConfig *schema.TLSConfig) (*tls.Config, error) {
func loadTLSConfig(tlsConfig *schema.TLSConfig, logger *slog.Logger) (*tls.Config, error) {
certPool, err := loadCACertPool(tlsConfig)
if err != nil {
return nil, err
Expand All @@ -67,11 +68,6 @@ func loadTLSConfig(tlsConfig *schema.TLSConfig) (*tls.Config, error) {
}
}

cert, err := loadCertificate(tlsConfig)
if err != nil {
return nil, err
}

var insecureSkipVerify bool
if tlsConfig.InsecureSkipVerify != nil {
insecureSkipVerify, err = tlsConfig.InsecureSkipVerify.GetOrDefault(false)
Expand All @@ -80,13 +76,21 @@ func loadTLSConfig(tlsConfig *schema.TLSConfig) (*tls.Config, error) {
}
}

if cert == nil && !insecureSkipVerify {
cert, err := loadCertificate(tlsConfig, insecureSkipVerify, logger)
if err != nil {
return nil, err
}

var certificates []tls.Certificate
if cert != nil {
certificates = append(certificates, *cert)
} else if !insecureSkipVerify {
return nil, nil
}

result := &tls.Config{
RootCAs: certPool,
Certificates: []tls.Certificate{*cert},
Certificates: certificates,
MinVersion: minTLS,
MaxVersion: maxTLS,
CipherSuites: cipherSuites,
Expand Down Expand Up @@ -168,7 +172,7 @@ func loadCertPem(certPem []byte, includeSystemCACertsPool bool) (*x509.CertPool,
return certPool, nil
}

func loadCertificate(tlsConfig *schema.TLSConfig) (*tls.Certificate, error) {
func loadCertificate(tlsConfig *schema.TLSConfig, insecureSkipVerify bool, logger *slog.Logger) (*tls.Certificate, error) {
var certData, keyData []byte
var certPem, keyPem string
var err error
Expand Down Expand Up @@ -199,6 +203,10 @@ func loadCertificate(tlsConfig *schema.TLSConfig) (*tls.Certificate, error) {
}
}

if len(certData) == 0 && !insecureSkipVerify {
logger.Warn("both certificate PEM and file are empty")
}

if tlsConfig.KeyPem != nil {
keyPem, err = tlsConfig.KeyPem.GetOrDefault("")
if err != nil {
Expand All @@ -225,6 +233,10 @@ func loadCertificate(tlsConfig *schema.TLSConfig) (*tls.Certificate, error) {
}
}

if len(keyData) == 0 && !insecureSkipVerify {
logger.Warn("both key PEM and file are empty")
}

if len(keyData) == 0 && len(certData) == 0 {
return nil, nil
}
Expand Down
4 changes: 2 additions & 2 deletions connector/internal/upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (um *UpstreamManager) Register(ctx context.Context, runtimeSchema *configur
httpClient := um.defaultClient

if runtimeSchema.Settings.TLS != nil {
tlsClient, err := security.NewHTTPClientTLS(httpClient, runtimeSchema.Settings.TLS)
tlsClient, err := security.NewHTTPClientTLS(httpClient, runtimeSchema.Settings.TLS, logger)
if err != nil {
return fmt.Errorf("%s: %w", namespace, err)
}
Expand Down Expand Up @@ -82,7 +82,7 @@ func (um *UpstreamManager) Register(ctx context.Context, runtimeSchema *configur

serverClient := httpClient
if server.TLS != nil {
tlsClient, err := security.NewHTTPClientTLS(um.defaultClient, server.TLS)
tlsClient, err := security.NewHTTPClientTLS(um.defaultClient, server.TLS, logger)
if err != nil {
return fmt.Errorf("%s.server[%s]: %w", namespace, serverID, err)
}
Expand Down
5 changes: 5 additions & 0 deletions connector/testdata/tls/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ settings:
env: PET_STORE_CERT_FILE
keyFile:
env: PET_STORE_KEY_FILE
insecureSkipVerify:
env: PET_STORE_INSECURE_SKIP_VERIFY
value: false
includeSystemCACertsPool:
value: true
functions:
findPets:
request:
Expand Down
49 changes: 42 additions & 7 deletions docs/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,39 @@ See the configuration example in [Hasura docs](https://hasura.io/docs/3.0/recipe
## Mutual TLS
### Basic
If the `mutualTLS` security scheme exists the TLS configuration will be generated in the `settings` field.

```yaml
settings:
servers:
- url:
env: PET_STORE_URL
securitySchemes:
mtls:
type: mutualTLS
tls:
# Provide the certificate contents as a base64-encoded string.
certPem:
env: PET_STORE_CERT_PEM
# Provide the key contents as a base64-encoded string.
keyPem:
env: PET_STORE_KEY_PEM
# Provide the CA cert contents as a base64-encoded string.
caPem:
env: PET_STORE_CA_PEM
# Additionally you can configure TLS to be enabled but skip verifying the server's certificate chain (optional).
insecureSkipVerify:
env: PET_STORE_INSECURE_SKIP_VERIFY
value: false
```

### Full Configuration:

[!NOTE]
It's recommended to use inline bases64-encoded PEM data `*_PEM` variables if you deploy the connector to Hasura DDN cloud.

```yaml
settings:
servers:
Expand Down Expand Up @@ -158,19 +189,23 @@ settings:
includeSystemCACertsPool:
env: PET_STORE_INCLUDE_SYSTEM_CA_CERT_POOL
value: false
# ServerName requested by client for virtual hosting (optional).
## ServerName requested by client for virtual hosting (optional).
serverName:
env: PET_STORE_SERVER_NAME
# Minimum acceptable TLS version (optional).
## Minimum acceptable TLS version (optional).
minVersion: "1.0"
# Maximum acceptable TLS version (optional).
## Maximum acceptable TLS version (optional).
maxVersion: "1.3"
# Explicit cipher suites can be set. If left blank, a safe default list is used (optional).
cipherSuites:
- TLS_AES_128_GCM_SHA256
## Explicit cipher suites can be set. If left blank, a safe default list is used (optional).
# cipherSuites:
# - TLS_AES_128_GCM_SHA256
```

You can configure either file path `*_FILE` or inline bases64-encoded PEM data `*_PEM`.
### Different Certificates per servers.

If the service has many servers, you can configure different TLS configurations for each server. However, you need to [manually patch the configuration](../README.md#json-patch):

Expand Down

0 comments on commit 849466c

Please sign in to comment.