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

Jump to bottom

[fftls] Allow for In-Memory or Inlined TLS Certificates #132

Merged
merged 5 commits into from
Mar 11, 2024
Merged

[fftls] Allow for In-Memory or Inlined TLS Certificates #132

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d1b60ec
[fftls] Allow for In-Memory or Inlined TLS Certificates
onelapahead Mar 11, 2024
cb44f29
fix linter
onelapahead Mar 11, 2024
7116b31
fix coverage
onelapahead Mar 11, 2024
4d97472
linter
onelapahead Mar 11, 2024
0a735f3
comments
onelapahead Mar 11, 2024
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
18 changes: 17 additions & 1 deletion pkg/fftls/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand All @@ -23,14 +23,21 @@ import (
const (
// HTTPConfTLSCAFile the TLS certificate authority file for the HTTP server
HTTPConfTLSCAFile = "caFile"
// HTTPConfTLSCA the TLS certificate authority in PEM format
HTTPConfTLSCA = "ca"
// HTTPConfTLSCertFile the TLS certificate file for the HTTP server
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should document (probably in this comment so it's visible to the developer) that if these are set they will take precedence over the "file" fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well its the opposite in that the file fields take precedence over these fields for backwards compatibility's sake.

That could be the wrong approach, but I'm updating the comments now to reflect the current behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments updated

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right sorry. Had the switch statement backwards in my head.

HTTPConfTLSCertFile = "certFile"
// HTTPConfTLSCert the TLS certificate in PEM format
HTTPConfTLSCert = "cert"
// HTTPConfTLSClientAuth whether the HTTP server requires a mutual TLS connection
HTTPConfTLSClientAuth = "clientAuth"
// HTTPConfTLSEnabled whether TLS is enabled for the HTTP server
HTTPConfTLSEnabled = "enabled"
// HTTPConfTLSKeyFile the private key file for TLS on the server
HTTPConfTLSKeyFile = "keyFile"
// HTTPConfTLSKey the TLS certificate key in PEM format
HTTPConfTLSKey = "key"

// HTTPConfTLSInsecureSkipHostVerify disables host verification - insecure (for dev only)
HTTPConfTLSInsecureSkipHostVerify = "insecureSkipHostVerify"

Expand All @@ -44,18 +51,24 @@ type Config struct {
Enabled bool `ffstruct:"tlsconfig" json:"enabled"`
ClientAuth bool `ffstruct:"tlsconfig" json:"clientAuth,omitempty"`
CAFile string `ffstruct:"tlsconfig" json:"caFile,omitempty"`
CA string `ffstruct:"tlsconfig" json:"ca,omitempty"`
CertFile string `ffstruct:"tlsconfig" json:"certFile,omitempty"`
Cert string `ffstruct:"tlsconfig" json:"cert,omitempty"`
KeyFile string `ffstruct:"tlsconfig" json:"keyFile,omitempty"`
Key string `ffstruct:"tlsconfig" json:"key,omitempty"`
InsecureSkipHostVerify bool `ffstruct:"tlsconfig" json:"insecureSkipHostVerify"`
RequiredDNAttributes map[string]interface{} `ffstruct:"tlsconfig" json:"requiredDNAttributes,omitempty"`
}

func InitTLSConfig(conf config.Section) {
conf.AddKnownKey(HTTPConfTLSEnabled, defaultHTTPTLSEnabled)
conf.AddKnownKey(HTTPConfTLSCAFile)
conf.AddKnownKey(HTTPConfTLSCA)
conf.AddKnownKey(HTTPConfTLSClientAuth)
conf.AddKnownKey(HTTPConfTLSCertFile)
conf.AddKnownKey(HTTPConfTLSCert)
conf.AddKnownKey(HTTPConfTLSKeyFile)
conf.AddKnownKey(HTTPConfTLSKey)
conf.AddKnownKey(HTTPConfTLSRequiredDNAttributes)
conf.AddKnownKey(HTTPConfTLSInsecureSkipHostVerify)
}
Expand All @@ -65,8 +78,11 @@ func GenerateConfig(conf config.Section) *Config {
Enabled: conf.GetBool(HTTPConfTLSEnabled),
ClientAuth: conf.GetBool(HTTPConfTLSClientAuth),
CAFile: conf.GetString(HTTPConfTLSCAFile),
CA: conf.GetString(HTTPConfTLSCA),
CertFile: conf.GetString(HTTPConfTLSCertFile),
Cert: conf.GetString(HTTPConfTLSCert),
KeyFile: conf.GetString(HTTPConfTLSKeyFile),
Key: conf.GetString(HTTPConfTLSKey),
InsecureSkipHostVerify: conf.GetBool(HTTPConfTLSInsecureSkipHostVerify),
RequiredDNAttributes: conf.GetObject(HTTPConfTLSRequiredDNAttributes),
}
Expand Down
20 changes: 16 additions & 4 deletions pkg/fftls/fftls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -62,7 +62,8 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co
var err error
// Support custom CA file
var rootCAs *x509.CertPool
if config.CAFile != "" {
switch {
case config.CAFile != "":
rootCAs = x509.NewCertPool()
var caBytes []byte
caBytes, err = os.ReadFile(config.CAFile)
Expand All @@ -72,7 +73,13 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co
err = i18n.NewError(ctx, i18n.MsgInvalidCAFile)
}
}
} else {
case config.CA != "":
rootCAs = x509.NewCertPool()
ok := rootCAs.AppendCertsFromPEM([]byte(config.CA))
if !ok {
err = i18n.NewError(ctx, i18n.MsgInvalidCAFile)
}
default:
rootCAs, err = x509.SystemCertPool()
}

Expand All @@ -89,7 +96,12 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co
if err != nil {
return nil, i18n.WrapError(ctx, err, i18n.MsgInvalidKeyPairFiles)
}

tlsConfig.Certificates = []tls.Certificate{cert}
} else if config.Cert != "" && config.Key != "" {
cert, err := tls.X509KeyPair([]byte(config.Cert), []byte(config.Key))
if err != nil {
return nil, i18n.WrapError(ctx, err, i18n.MsgInvalidKeyPairFiles)
}
tlsConfig.Certificates = []tls.Certificate{cert}
}

Expand Down
98 changes: 80 additions & 18 deletions pkg/fftls/fftls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand All @@ -24,9 +24,11 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"github.com/stretchr/testify/require"
"math/big"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason to pull in require here, as opposed to assert? I think for unit tests we typically have stuck with assert, though I am noticing that is not as true in this particular repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So require fails the test immediately whereas assert keep runs the test if it doesn't pass. So I always use require for anything involve setup, where if it errors / is not successful, the entire things fails.

It's just syntactic sugar for:

if err != nil {
  t.Fatal(err)
}

I believe

"net"
"os"
"strings"
"testing"
"time"

Expand All @@ -35,6 +37,34 @@ import (
)

func buildSelfSignedTLSKeyPair(t *testing.T, subject pkix.Name) (string, string) {
// Create an X509 certificate pair
privatekey, _ := rsa.GenerateKey(rand.Reader, 2048)
publickey := &privatekey.PublicKey
var privateKeyBytes []byte = x509.MarshalPKCS1PrivateKey(privatekey)
privateKeyBlock := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: privateKeyBytes}
privateKeyPEM := &strings.Builder{}
err := pem.Encode(privateKeyPEM, privateKeyBlock)
require.NoError(t, err)
serialNumber, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
x509Template := &x509.Certificate{
SerialNumber: serialNumber,
Subject: subject,
NotBefore: time.Now(),
NotAfter: time.Now().Add(100 * time.Second),
KeyUsage: x509.KeyUsageDigitalSignature,
BasicConstraintsValid: true,
IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)},
}
require.NoError(t, err)
derBytes, err := x509.CreateCertificate(rand.Reader, x509Template, x509Template, publickey, privatekey)
require.NoError(t, err)
publicKeyPEM := &strings.Builder{}
err = pem.Encode(publicKeyPEM, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
require.NoError(t, err)
return publicKeyPEM.String(), privateKeyPEM.String()
}

func buildSelfSignedTLSKeyPairFiles(t *testing.T, subject pkix.Name) (string, string) {
// Create an X509 certificate pair
privatekey, _ := rsa.GenerateKey(rand.Reader, 2048)
publickey := &privatekey.PublicKey
Expand Down Expand Up @@ -129,7 +159,7 @@ func TestTLSDefault(t *testing.T) {
func TestErrInvalidCAFile(t *testing.T) {

config.RootConfigReset()
_, notTheCAFileTheKey := buildSelfSignedTLSKeyPair(t, pkix.Name{
_, notTheCAFileTheKey := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})

Expand All @@ -140,13 +170,28 @@ func TestErrInvalidCAFile(t *testing.T) {

_, err := ConstructTLSConfig(context.Background(), conf, ClientType)
assert.Regexp(t, "FF00152", err)
}

func TestErrInvalidCA(t *testing.T) {

config.RootConfigReset()
_, notTheCATheKey := buildSelfSignedTLSKeyPair(t, pkix.Name{
CommonName: "server.example.com",
})

conf := config.RootSection("fftls_server")
InitTLSConfig(conf)
conf.Set(HTTPConfTLSEnabled, true)
conf.Set(HTTPConfTLSCA, notTheCATheKey)

_, err := ConstructTLSConfig(context.Background(), conf, ClientType)
assert.Regexp(t, "FF00152", err)
}

func TestErrInvalidKeyPairFile(t *testing.T) {

config.RootConfigReset()
notTheKeyFile, notTheCertFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
notTheKeyFile, notTheCertFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})

Expand All @@ -161,12 +206,29 @@ func TestErrInvalidKeyPairFile(t *testing.T) {

}

func TestMTLSOk(t *testing.T) {
func TestErrInvalidKeyPair(t *testing.T) {

serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
config.RootConfigReset()
notTheKey, notTheCert := buildSelfSignedTLSKeyPair(t, pkix.Name{
CommonName: "server.example.com",
})
clientPublicKeyFile, clientKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{

conf := config.RootSection("fftls_server")
InitTLSConfig(conf)
conf.Set(HTTPConfTLSEnabled, true)
conf.Set(HTTPConfTLSKey, notTheKey)
conf.Set(HTTPConfTLSCert, notTheCert)

_, err := ConstructTLSConfig(context.Background(), conf, ClientType)
assert.Regexp(t, "FF00206", err)

}

func TestMTLSOk(t *testing.T) {
serverPublicKey, serverKey := buildSelfSignedTLSKeyPair(t, pkix.Name{
CommonName: "server.example.com",
})
clientPublicKey, clientKey := buildSelfSignedTLSKeyPair(t, pkix.Name{
CommonName: "client.example.com",
})

Expand All @@ -175,9 +237,9 @@ func TestMTLSOk(t *testing.T) {
serverConf := config.RootSection("fftls_server")
InitTLSConfig(serverConf)
serverConf.Set(HTTPConfTLSEnabled, true)
serverConf.Set(HTTPConfTLSCAFile, clientPublicKeyFile)
serverConf.Set(HTTPConfTLSCertFile, serverPublicKeyFile)
serverConf.Set(HTTPConfTLSKeyFile, serverKeyFile)
serverConf.Set(HTTPConfTLSCA, clientPublicKey)
serverConf.Set(HTTPConfTLSCert, serverPublicKey)
serverConf.Set(HTTPConfTLSKey, serverKey)
serverConf.Set(HTTPConfTLSClientAuth, true)

addr, done := buildTLSListener(t, serverConf, ServerType)
Expand All @@ -186,9 +248,9 @@ func TestMTLSOk(t *testing.T) {
clientConf := config.RootSection("fftls_client")
InitTLSConfig(clientConf)
clientConf.Set(HTTPConfTLSEnabled, true)
clientConf.Set(HTTPConfTLSCAFile, serverPublicKeyFile)
clientConf.Set(HTTPConfTLSCertFile, clientPublicKeyFile)
clientConf.Set(HTTPConfTLSKeyFile, clientKeyFile)
clientConf.Set(HTTPConfTLSCA, serverPublicKey)
clientConf.Set(HTTPConfTLSCert, clientPublicKey)
clientConf.Set(HTTPConfTLSKey, clientKey)

tlsConfig, err := ConstructTLSConfig(context.Background(), clientConf, ClientType)
assert.NoError(t, err)
Expand All @@ -208,7 +270,7 @@ func TestMTLSOk(t *testing.T) {

func TestMTLSMissingClientCert(t *testing.T) {

serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})

Expand Down Expand Up @@ -243,10 +305,10 @@ func TestMTLSMissingClientCert(t *testing.T) {

func TestMTLSMatchFullSubject(t *testing.T) {

serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})
clientPublicKeyFile, clientKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
clientPublicKeyFile, clientKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "client.example.com",
Country: []string{"GB"},
Organization: []string{"hyperledger"},
Expand Down Expand Up @@ -306,10 +368,10 @@ func TestMTLSMatchFullSubject(t *testing.T) {

func TestMTLSMismatchSubject(t *testing.T) {

serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})
clientPublicKeyFile, clientKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
clientPublicKeyFile, clientKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "wrong.example.com",
})

Expand Down Expand Up @@ -429,7 +491,7 @@ func TestMTLSDNValidatorEmptyChain(t *testing.T) {

func TestConnectSkipVerification(t *testing.T) {

serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPair(t, pkix.Name{
serverPublicKeyFile, serverKeyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{
CommonName: "server.example.com",
})

Expand Down
Loading