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

TLS Auth - Support for encrypted private key #2488

Merged
merged 7 commits into from
Apr 21, 2022
Merged

TLS Auth - Support for encrypted private key #2488

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1c140cc
encrypted private key support for TLSAuth
Gabrielopesantos Apr 5, 2022
2ea9573
Throw an error when pkcs8 format encrypted key is provided
Gabrielopesantos Apr 8, 2022
931b769
Tests refactoring and fix ci issues
Gabrielopesantos Apr 11, 2022
013d52b
Removed if clause to check if key is encrypted instead of always tryi…
Gabrielopesantos Apr 13, 2022
6d3910b
Fix tests and add test where password is provided and key is not encr…
Gabrielopesantos Apr 14, 2022
e55c117
fix ci staticcheck of DecryptPEMBlock
Gabrielopesantos Apr 14, 2022
7f9bcc9
Update lib/options.go
Gabrielopesantos Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 37 additions & 3 deletions lib/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ package lib

import (
"crypto/tls"
"crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
"net"
"reflect"
Expand Down Expand Up @@ -130,8 +132,9 @@ func (s *TLSCipherSuites) UnmarshalJSON(data []byte) error {
// Fields for TLSAuth. Unmarshalling hack.
type TLSAuthFields struct {
// Certificate and key as a PEM-encoded string, including "-----BEGIN CERTIFICATE-----".
Cert string `json:"cert"`
Key string `json:"key"`
Cert string `json:"cert"`
Key string `json:"key"`
Password string `json:"password"`

// Domains to present the certificate to. May contain wildcards, eg. "*.example.com".
Domains []string `json:"domains"`
Expand All @@ -154,8 +157,16 @@ func (c *TLSAuth) UnmarshalJSON(data []byte) error {
}

func (c *TLSAuth) Certificate() (*tls.Certificate, error) {
key := []byte(c.Key)
var err error
if c.Password != "" {
key, err = parsePrivateKey(c.Key, c.Password)
if err != nil {
return nil, err
}
}
if c.certificate == nil {
cert, err := tls.X509KeyPair([]byte(c.Cert), []byte(c.Key))
cert, err := tls.X509KeyPair([]byte(c.Cert), key)
if err != nil {
return nil, err
}
Expand All @@ -164,6 +175,29 @@ func (c *TLSAuth) Certificate() (*tls.Certificate, error) {
return c.certificate, nil
}

func parsePrivateKey(privKey, password string) ([]byte, error) {
key := []byte(privKey)
block, _ := pem.Decode(key)
mstoykov marked this conversation as resolved.
Show resolved Hide resolved
if block == nil {
return nil, fmt.Errorf("failed to decode PEM key")
}
blockType := block.Type
mstoykov marked this conversation as resolved.
Show resolved Hide resolved
if blockType == "ENCRYPTED PRIVATE KEY" {
return nil, fmt.Errorf("encrypted pkcs8 formatted key is not supported")
}
if encrypted := x509.IsEncryptedPEMBlock(block); encrypted {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have password, shouldn't we always try to decrypt it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we always try decrypt, in cases were a non encrypted key is provided, since it does not a DEK header, x509.DecryptPEMBlock (https://pkg.go.dev/crypto/x509#DecryptPEMBlock) will return an error. In my opinion a good approach would be just add the warning saying that a password is not required if the key is not encrypted, as suggested below.

If both of you you agree with this, if possible, tell me how can I add a warning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess that you can't log here 🤔 . But on the other hand I would argue it should be an outright error if there is a password, and it isn't encrypted. If you have a password and not DEK header ... you maybe shouldn't have password ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I understand your point. I just thought it would confuse some users the first time they use the feature. Made the change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

confuse some users

I would argue it will be even more confusing if you add an option and nothing changes - if we at least then error out differently IMO it's better.

mstoykov marked this conversation as resolved.
Show resolved Hide resolved
decryptedKey, err := x509.DecryptPEMBlock(block, []byte(password))
if err != nil {
return nil, err
}
key = pem.EncodeToMemory(&pem.Block{
Type: blockType,
Bytes: decryptedKey,
})
}
mstoykov marked this conversation as resolved.
Show resolved Hide resolved
return key, nil
}

// IPNet is a wrapper around net.IPNet for JSON unmarshalling
type IPNet struct {
net.IPNet
Expand Down
119 changes: 119 additions & 0 deletions lib/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ func TestOptions(t *testing.T) {
"AwEHoUQDQgAEF8XzmC7x8Ns0Y2Wyu2c77ge+6I/ghcDTjWOMZzMPmRRDxqKFLuGD\n" +
"zW1Kss13WODGSS8+j7dNCPOeLKyK6cbeIg==\n" +
"-----END EC PRIVATE KEY-----",
Password: "iNGNlsrtnO+4HiQAwRgIhANYDaM18sXAdkjyvuiP4IXA041jdK48Jd6a8aD",
}, nil},
}
opts := Options{}.Apply(Options{TLSAuth: tlsAuth})
Expand Down Expand Up @@ -286,6 +287,124 @@ func TestOptions(t *testing.T) {
assert.Error(t, json.Unmarshal([]byte(jsonStr), &opts))
})
})

t.Run("TLSAuth encrypted key and invalid password", func(t *testing.T) {
tlsAuth := []*TLSAuth{
{TLSAuthFields{
Domains: []string{"example.com", "*.example.com"},
Cert: "-----BEGIN CERTIFICATE-----\n" +
"MIIBoTCCAUegAwIBAgIUQl0J1Gkd6U2NIMwMDnpfH8c1myEwCgYIKoZIzj0EAwIw\n" +
"EDEOMAwGA1UEAxMFTXkgQ0EwHhcNMTcwODE1MTYxODAwWhcNMTgwODE1MTYxODAw\n" +
"WjAQMQ4wDAYDVQQDEwV1c2VyMTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLaf\n" +
"xEOmBHkzbqd9/0VZX/39qO2yQq2Gz5faRdvy38kuLMCV+9HYrfMx6GYCZzTUIq6h\n" +
"8QXOrlgYTixuUVfhJNWjfzB9MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggr\n" +
"BgEFBQcDAQYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxmQiq5K3\n" +
"KUnVME945Byt3Ysvkh8wHwYDVR0jBBgwFoAU3qEhcpRgpsqo9V+LFns9a+oZIYww\n" +
"CgYIKoZIzj0EAwIDSAAwRQIgSGxnJ+/cLUNTzt7fhr/mjJn7ShsTW33dAdfLM7H2\n" +
"z/gCIQDyVf8DePtxlkMBScTxZmIlMQdNc6+6VGZQ4QscruVLmg==\n" +
"-----END CERTIFICATE-----",
Key: "-----BEGIN EC PRIVATE KEY-----\n" +
"Proc-Type: 4,ENCRYPTED\n" +
"DEK-Info: AES-256-CBC,DF2445CBFE2E5B112FB2B721063757E5\n" +
"o/VKNZjQcRM2hatqUkQ0dTolL7i2i5hJX9XYsl+TMsq8ZkC83uY/JdR986QS+W2c\n" +
"EoQGtVGVeL0KGvGpzjTX3YAKXM7Lg5btAeS8GvJ9S7YFd8s0q1pqDdffl2RyjJav\n" +
"t1jx6XvLu2nBrOUARvHqjkkJQCTdRf2a34GJdbZqE+4=\n" +
"-----END EC PRIVATE KEY-----",
Password: "iZfYGcrgFHOg4nweEo7ufT",
}, nil},
}
opts := Options{}.Apply(Options{TLSAuth: tlsAuth})
assert.Equal(t, tlsAuth, opts.TLSAuth)

t.Run("Roundtrip", func(t *testing.T) {
optsData, err := json.Marshal(opts)
assert.NoError(t, err)

var opts2 Options
errMsg := "x509: decryption password incorrect"
err = json.Unmarshal(optsData, &opts2)
assert.Error(t, err)
assert.Contains(t, err.Error(), errMsg)
})
})

t.Run("TLSAuth encrypted key and valid password", func(t *testing.T) {
tlsAuth := []*TLSAuth{
{TLSAuthFields{
Domains: []string{"example.com", "*.example.com"},
Cert: "-----BEGIN CERTIFICATE-----\n" +
"MIIBoTCCAUegAwIBAgIUQl0J1Gkd6U2NIMwMDnpfH8c1myEwCgYIKoZIzj0EAwIw\n" +
"EDEOMAwGA1UEAxMFTXkgQ0EwHhcNMTcwODE1MTYxODAwWhcNMTgwODE1MTYxODAw\n" +
"WjAQMQ4wDAYDVQQDEwV1c2VyMTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLaf\n" +
"xEOmBHkzbqd9/0VZX/39qO2yQq2Gz5faRdvy38kuLMCV+9HYrfMx6GYCZzTUIq6h\n" +
"8QXOrlgYTixuUVfhJNWjfzB9MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggr\n" +
"BgEFBQcDAQYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxmQiq5K3\n" +
"KUnVME945Byt3Ysvkh8wHwYDVR0jBBgwFoAU3qEhcpRgpsqo9V+LFns9a+oZIYww\n" +
"CgYIKoZIzj0EAwIDSAAwRQIgSGxnJ+/cLUNTzt7fhr/mjJn7ShsTW33dAdfLM7H2\n" +
"z/gCIQDyVf8DePtxlkMBScTxZmIlMQdNc6+6VGZQ4QscruVLmg==\n" +
"-----END CERTIFICATE-----",
Key: "-----BEGIN EC PRIVATE KEY-----\n" +
"Proc-Type: 4,ENCRYPTED\n" +
"DEK-Info: AES-256-CBC,DF2445CBFE2E5B112FB2B721063757E5\n" +
"o/VKNZjQcRM2hatqUkQ0dTolL7i2i5hJX9XYsl+TMsq8ZkC83uY/JdR986QS+W2c\n" +
"EoQGtVGVeL0KGvGpzjTX3YAKXM7Lg5btAeS8GvJ9S7YFd8s0q1pqDdffl2RyjJav\n" +
"t1jx6XvLu2nBrOUARvHqjkkJQCTdRf2a34GJdbZqE+4=\n" +
"-----END EC PRIVATE KEY-----",
Password: "12345",
}, nil},
}
opts := Options{}.Apply(Options{TLSAuth: tlsAuth})
assert.Equal(t, tlsAuth, opts.TLSAuth)

t.Run("Roundtrip", func(t *testing.T) {
optsData, err := json.Marshal(opts)
assert.NoError(t, err)

var opts2 Options
err = json.Unmarshal(optsData, &opts2)
assert.NoError(t, err)
})
})
t.Run("TLSAuth encrypted pks8 format key and valid password", func(t *testing.T) {
tlsAuth := []*TLSAuth{
{TLSAuthFields{
Domains: []string{"example.com", "*.example.com"},
Cert: "-----BEGIN CERTIFICATE-----\n" +
"MIIBoTCCAUegAwIBAgIUQl0J1Gkd6U2NIMwMDnpfH8c1myEwCgYIKoZIzj0EAwIw\n" +
"EDEOMAwGA1UEAxMFTXkgQ0EwHhcNMTcwODE1MTYxODAwWhcNMTgwODE1MTYxODAw\n" +
"WjAQMQ4wDAYDVQQDEwV1c2VyMTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLaf\n" +
"xEOmBHkzbqd9/0VZX/39qO2yQq2Gz5faRdvy38kuLMCV+9HYrfMx6GYCZzTUIq6h\n" +
"8QXOrlgYTixuUVfhJNWjfzB9MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggr\n" +
"BgEFBQcDAQYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxmQiq5K3\n" +
"KUnVME945Byt3Ysvkh8wHwYDVR0jBBgwFoAU3qEhcpRgpsqo9V+LFns9a+oZIYww\n" +
"CgYIKoZIzj0EAwIDSAAwRQIgSGxnJ+/cLUNTzt7fhr/mjJn7ShsTW33dAdfLM7H2\n" +
"z/gCIQDyVf8DePtxlkMBScTxZmIlMQdNc6+6VGZQ4QscruVLmg==\n" +
"-----END CERTIFICATE-----",
Key: "-----BEGIN ENCRYPTED PRIVATE KEY-----\n" +
"MIHsMFcGCSqGSIb3DQEFDTBKMCkGCSqGSIb3DQEFDDAcBAjcfarGfrRgUgICCAAw\n" +
"DAYIKoZIhvcNAgkFADAdBglghkgBZQMEASoEEFmtmKEFmThbkbpxmC6iBvoEgZCE\n" +
"pDCpH/yCLmSpjdi/PC74I794nzHyCWf/oS0JhM0Q7J+abZP+p5pnreKft1f15Dbw\n" +
"QG9alfoM6EffJcVo3gf1tgQrpGGFMwczc4VhQgSGDy0XjZSbd2K0QCFGSmD2ZIR1\n" +
"qPG3WepWjKmIsYffGeKZx+FjXHSFeGk7RnssNAyKcPruDQIdWWyXxX1+ugBKuBw=\n" +
"-----END ENCRYPTED PRIVATE KEY-----\n",
Password: "12345",
}, nil},
}
opts := Options{}.Apply(Options{TLSAuth: tlsAuth})
assert.Equal(t, tlsAuth, opts.TLSAuth)

t.Run("Roundtrip", func(t *testing.T) {
optsData, err := json.Marshal(opts)
assert.NoError(t, err)

var opts2 Options
errMsg := "encrypted pkcs8 formatted key is not supported"
err = json.Unmarshal(optsData, &opts2)
assert.Error(t, err)
assert.Contains(t, err.Error(), errMsg)
})
})

t.Run("NoConnectionReuse", func(t *testing.T) {
opts := Options{}.Apply(Options{NoConnectionReuse: null.BoolFrom(true)})
assert.True(t, opts.NoConnectionReuse.Valid)
Expand Down