Skip to content

Commit

Permalink
Add separation between keys when exporting CAs (#18562)
Browse files Browse the repository at this point in the history 
Adding a new line was sometimes not enough to separate the keys.

This was causing our tests to sometimes fail.
  • Loading branch information
@marcoandredinis
marcoandredinis authored Dec 5, 2022
1 parent c9e64e1 commit 8bdcd19
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 68 deletions.
2 changes: 1 addition & 1 deletion docs/pages/reference/cli.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ $ tctl auth export [<flags>]

| Name | Default Value(s) | Allowed Value(s) | Description |
| - | - | - | - |
| `--keys` | none | none | if set, will print private keys |
| `--keys` | none | none | if set, only exports private keys |
| `--fingerprint` | none | **string** e.g. `SHA256:<fingerprint>` | filter authority by fingerprint |
| `--compat` | none | version number | export certificates compatible with specific version of Teleport |
| `--type` | none | `user, host` or `tls` | certificate type |
Expand Down
37 changes: 12 additions & 25 deletions lib/client/ca_export.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ func ExportAuthorities(ctx context.Context, client auth.ClientI, req ExportAutho
return exportAuth(ctx, client, req, false /* exportSecrets */)
}

// ExportAuthoritiesWithSecrets exports the Authority Certificate and its secrets (private keys).
// It exports the secrets first and then the certificate (separated by an empty line).
// ExportAuthoritiesSecrets exports the Authority Certificate secrets (private keys).
// See ExportAuthorities for more information.
func ExportAuthoritiesWithSecrets(ctx context.Context, client auth.ClientI, req ExportAuthoritiesRequest) (string, error) {
func ExportAuthoritiesSecrets(ctx context.Context, client auth.ClientI, req ExportAuthoritiesRequest) (string, error) {
return exportAuth(ctx, client, req, true /* exportSecrets */)
}

Expand Down Expand Up @@ -232,6 +231,7 @@ func exportTLSAuthority(ctx context.Context, client auth.ClientI, req exportTLSA
if err != nil {
return "", trace.Wrap(err)
}

certAuthority, err := client.GetCertAuthority(
ctx,
types.CertAuthID{Type: req.AuthType, DomainName: clusterName},
Expand All @@ -246,34 +246,21 @@ func exportTLSAuthority(ctx context.Context, client auth.ClientI, req exportTLSA
}
keyPair := certAuthority.GetActiveKeys().TLS[0]

ret := strings.Builder{}
marshalKeyPair := func(data []byte) error {
if !req.UnpackPEM {
ret.Write(data)
return nil
}

b, _ := pem.Decode(data)
if b == nil {
return trace.BadParameter("invalid PEM data")
}
ret.Write(b.Bytes)

return nil
bytesToExport := keyPair.Cert
if req.ExportPrivateKeys {
bytesToExport = keyPair.Key
}

if req.ExportPrivateKeys {
if err := marshalKeyPair(keyPair.Key); err != nil {
return "", trace.Wrap(err)
}
ret.WriteString("\n\n")
if !req.UnpackPEM {
return string(bytesToExport), nil
}

if err := marshalKeyPair(keyPair.Cert); err != nil {
return "", trace.Wrap(err)
b, _ := pem.Decode(bytesToExport)
if b == nil {
return "", trace.BadParameter("invalid PEM data")
}

return ret.String(), nil
return string(b.Bytes), nil
}

// userCAFormat returns the certificate authority public key exported as a single
Expand Down
73 changes: 32 additions & 41 deletions lib/client/ca_export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"context"
"crypto/x509"
"encoding/pem"
"strings"
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -65,15 +65,17 @@ func TestExportAuthorities(t *testing.T) {
}

validateTLSCertificatePEMFunc := func(t *testing.T, s string) {
pemBlock, _ := pem.Decode([]byte(s))
pemBlock, rest := pem.Decode([]byte(s))
require.NotNil(t, pemBlock, "pem.Decode failed")
require.Empty(t, rest)

validateTLSCertificateDERFunc(t, string(pemBlock.Bytes))
}

validatePrivateKeyPEMFunc := func(t *testing.T, s string) {
pemBlock, _ := pem.Decode([]byte(s))
pemBlock, rest := pem.Decode([]byte(s))
require.NotNil(t, pemBlock, "pem.Decode failed")
require.Empty(t, rest)

require.Equal(t, "RSA PRIVATE KEY", pemBlock.Type, "unexpected private key type")

Expand All @@ -83,31 +85,26 @@ func TestExportAuthorities(t *testing.T) {
}

validatePrivateKeyDERFunc := func(t *testing.T, s string) {
res := strings.Split(s, "\n\n")
require.Len(t, res, 2, "expected private key and certificate separated by one empty line")

privKey, err := x509.ParsePKCS1PrivateKey([]byte(res[0]))
privKey, err := x509.ParsePKCS1PrivateKey([]byte(s))
require.NoError(t, err, "x509.ParsePKCS1PrivateKey failed")
require.NotNil(t, privKey, "x509.ParsePKCS1PrivateKey returned a nil certificate")

validateTLSCertificateDERFunc(t, res[1])
}

for _, exportSecrets := range []bool{false, true} {
for _, tt := range []struct {
name string
req ExportAuthoritiesRequest
errorCheck require.ErrorAssertionFunc
assertOutput func(t *testing.T, output string)
assertSecrets func(t *testing.T, output string)
name string
req ExportAuthoritiesRequest
errorCheck require.ErrorAssertionFunc
assertNoSecrets func(t *testing.T, output string)
assertSecrets func(t *testing.T, output string)
}{
{
name: "ssh host and user ca",
req: ExportAuthoritiesRequest{
AuthType: "",
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
require.Contains(t, output, "@cert-authority localcluster,*.localcluster ssh-rsa")
require.Contains(t, output, "cert-authority ssh-rsa")
},
Expand All @@ -119,7 +116,7 @@ func TestExportAuthorities(t *testing.T) {
AuthType: "user",
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
require.Contains(t, output, "cert-authority ssh-rsa")
},
assertSecrets: validatePrivateKeyPEMFunc,
Expand All @@ -130,7 +127,7 @@ func TestExportAuthorities(t *testing.T) {
AuthType: "host",
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
require.Contains(t, output, "@cert-authority localcluster,*.localcluster ssh-rsa")
},
assertSecrets: validatePrivateKeyPEMFunc,
Expand All @@ -140,18 +137,18 @@ func TestExportAuthorities(t *testing.T) {
req: ExportAuthoritiesRequest{
AuthType: "tls",
},
errorCheck: require.NoError,
assertOutput: validateTLSCertificatePEMFunc,
assertSecrets: validatePrivateKeyPEMFunc,
errorCheck: require.NoError,
assertNoSecrets: validateTLSCertificatePEMFunc,
assertSecrets: validatePrivateKeyPEMFunc,
},
{
name: "windows",
req: ExportAuthoritiesRequest{
AuthType: "windows",
},
errorCheck: require.NoError,
assertOutput: validateTLSCertificateDERFunc,
assertSecrets: validatePrivateKeyDERFunc,
errorCheck: require.NoError,
assertNoSecrets: validateTLSCertificateDERFunc,
assertSecrets: validatePrivateKeyDERFunc,
},
{
name: "invalid",
Expand All @@ -169,7 +166,7 @@ func TestExportAuthorities(t *testing.T) {
ExportAuthorityFingerprint: "not found fingerprint",
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
require.Empty(t, output)
},
assertSecrets: func(t *testing.T, output string) {
Expand All @@ -183,7 +180,7 @@ func TestExportAuthorities(t *testing.T) {
ExportAuthorityFingerprint: "fake fingerprint",
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
require.Empty(t, output)
},
assertSecrets: func(t *testing.T, output string) {
Expand All @@ -197,44 +194,38 @@ func TestExportAuthorities(t *testing.T) {
UseCompatVersion: true,
},
errorCheck: require.NoError,
assertOutput: func(t *testing.T, output string) {
assertNoSecrets: func(t *testing.T, output string) {
// compat version (using 1.0) returns cert-authority to be used in the server
// even when asking for ssh authorized hosts / known hosts
require.Contains(t, output, "@cert-authority localcluster,*.localcluster ssh-rsa")
},
assertSecrets: validatePrivateKeyPEMFunc,
},
} {
testName := tt.name
if exportSecrets {
testName = tt.name + "_exportSecrets"
}
t.Run(testName, func(t *testing.T) {
t.Run(fmt.Sprintf("%s_exportSecrets_%v", tt.name, exportSecrets), func(t *testing.T) {
mockedClient := &mockAuthClient{
server: testAuth.AuthServer,
}
var (
err error
exported string
)
exportFunc := ExportAuthorities
checkFunc := tt.assertNoSecrets

if exportSecrets {
exported, err = ExportAuthoritiesWithSecrets(ctx, mockedClient, tt.req)
tt.errorCheck(t, err)
} else {
exported, err = ExportAuthorities(ctx, mockedClient, tt.req)
tt.errorCheck(t, err)
exportFunc = ExportAuthoritiesSecrets
checkFunc = tt.assertSecrets
}

exported, err = exportFunc(ctx, mockedClient, tt.req)
tt.errorCheck(t, err)

if err != nil {
return
}

if exportSecrets {
tt.assertSecrets(t, exported)
} else {
tt.assertOutput(t, exported)
}
checkFunc(t, exported)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion tool/tctl/common/auth_command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ var allowedCertificateTypes = []string{"user", "host", "tls-host", "tls-user", "
func (a *AuthCommand) ExportAuthorities(ctx context.Context, clt auth.ClientI) error {
exportFunc := client.ExportAuthorities
if a.exportPrivateKeys {
exportFunc = client.ExportAuthoritiesWithSecrets
exportFunc = client.ExportAuthoritiesSecrets
}

authorities, err := exportFunc(
Expand Down

0 comments on commit 8bdcd19

Please sign in to comment.