Skip to content

Commit

Permalink
refactor: extract truststore encoding to internal package
Browse files Browse the repository at this point in the history
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
  • Loading branch information
erikgb committed Jul 19, 2024
1 parent cd0369c commit 756a098
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 160 deletions.
3 changes: 2 additions & 1 deletion pkg/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"

trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
"github.com/cert-manager/trust-manager/pkg/bundle/internal/truststore"
"github.com/cert-manager/trust-manager/pkg/fspkg"
"github.com/cert-manager/trust-manager/test/dummy"
"github.com/cert-manager/trust-manager/test/gen"
Expand All @@ -48,7 +49,7 @@ import (
func testEncodeJKS(t *testing.T, data string) []byte {
t.Helper()

encoded, err := jksEncoder{password: trustapi.DefaultJKSPassword}.encode(data)
encoded, err := truststore.NewJKSEncoder(trustapi.DefaultJKSPassword).Encode(data)
if err != nil {
t.Error(err)
}
Expand Down
123 changes: 123 additions & 0 deletions pkg/bundle/internal/truststore/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package truststore

import (
"bytes"
"crypto/sha256"
"encoding/hex"
"fmt"

"github.com/pavlo-v-chernykh/keystore-go/v4"
"software.sslmate.com/src/go-pkcs12"

"github.com/cert-manager/trust-manager/pkg/util"
)

type Encoder interface {
Encode(trustBundle string) ([]byte, error)
}

func NewJKSEncoder(password string) Encoder {
return &jksEncoder{password: password}
}

type jksEncoder struct {
password string
}

// Encode creates a binary JKS file from the given PEM-encoded trust bundle and Password.
// Note that the Password is not treated securely; JKS files generally seem to expect a Password
// to exist and so we have the option for one.
func (e jksEncoder) Encode(trustBundle string) ([]byte, error) {
cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
if err != nil {
return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
}

// WithOrderedAliases ensures that trusted certs are added to the JKS file in order,
// which makes the files appear to be reliably deterministic.
ks := keystore.New(keystore.WithOrderedAliases())

for _, c := range cas {
alias := certAlias(c.Raw, c.Subject.String())

// Note on CreationTime:
// Debian's JKS trust store sets the creation time to match the time that certs are added to the
// trust store (i.e., it's effectively time.Now() at the instant the file is generated).
// Using that method would make our JKS files in trust-manager non-deterministic, leaving us with
// two options if we want to maintain determinism:
// - Using something from the cert being added (e.g. NotBefore / NotAfter)
// - Using a fixed time (i.e. unix epoch)
// We use NotBefore here, arbitrarily.

err = ks.SetTrustedCertificateEntry(alias, keystore.TrustedCertificateEntry{
CreationTime: c.NotBefore,
Certificate: keystore.Certificate{
Type: "X509",
Content: c.Raw,
},
})

if err != nil {
// this error should never happen if we set jks.Certificate correctly
return nil, fmt.Errorf("failed to add cert with alias %q to trust store: %w", alias, err)
}
}

buf := &bytes.Buffer{}

err = ks.Store(buf, []byte(e.password))
if err != nil {
return nil, fmt.Errorf("failed to create JKS file: %w", err)
}

return buf.Bytes(), nil
}

func NewPKCS12Encoder(password string) Encoder {
return &pkcs12Encoder{password: password}
}

type pkcs12Encoder struct {
password string
}

func (e pkcs12Encoder) Encode(trustBundle string) ([]byte, error) {
cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
if err != nil {
return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
}

var entries []pkcs12.TrustStoreEntry
for _, c := range cas {
entries = append(entries, pkcs12.TrustStoreEntry{
Cert: c,
FriendlyName: certAlias(c.Raw, c.Subject.String()),
})
}

encoder := pkcs12.LegacyRC2

if e.password == "" {
encoder = pkcs12.Passwordless
}

return encoder.EncodeTrustStoreEntries(entries, e.password)
}

// certAlias creates a JKS-safe alias for the given DER-encoded certificate, such that
// any two certificates will have a different aliases unless they're identical in every way.
// This unique alias fixes an issue where we used the Issuer field as an alias, leading to
// different certs being treated as identical.
// The friendlyName is included in the alias as a UX feature when examining JKS files using a
// tool like `keytool`.
func certAlias(derData []byte, friendlyName string) string {
certHashBytes := sha256.Sum256(derData)
certHash := hex.EncodeToString(certHashBytes[:])

// Since certHash is the part which actually distinguishes between two
// certificates, put it first so that it won't be truncated if a cert
// with a really long subject is added. Not sure what the upper limit
// for length actually is, but it shouldn't matter here.

return certHash[:8] + "|" + friendlyName
}
68 changes: 68 additions & 0 deletions pkg/bundle/internal/truststore/types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package truststore

import (
"bytes"
"crypto/x509"
"encoding/pem"
"testing"

"github.com/pavlo-v-chernykh/keystore-go/v4"

"github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
"github.com/cert-manager/trust-manager/test/dummy"
)

func Test_encodeJKSAliases(t *testing.T) {
// IMPORTANT: We use TestCertificate1 and TestCertificate2 here because they're defined
// to be self-signed and to also use the same Subject, while being different certs.
// This test ensures that the aliases we create when adding to a JKS file is different under
// these conditions (where the issuer / subject is identical).
// Using different dummy certs would allow this test to pass but wouldn't actually test anything useful!
bundle := dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2)

jksFile, err := jksEncoder{password: v1alpha1.DefaultJKSPassword}.Encode(bundle)
if err != nil {
t.Fatalf("didn't expect an error but got: %s", err)
}

reader := bytes.NewReader(jksFile)

ks := keystore.New()

err = ks.Load(reader, []byte(v1alpha1.DefaultJKSPassword))
if err != nil {
t.Fatalf("failed to parse generated JKS file: %s", err)
}

entryNames := ks.Aliases()

if len(entryNames) != 2 {
t.Fatalf("expected two certs in JKS file but got %d", len(entryNames))
}
}

func Test_certAlias(t *testing.T) {
// We might not ever rely on aliases being stable, but this test seeks
// to enforce stability for now. It'll be easy to remove.

// If this test starts failing after TestCertificate1 is updated, it'll
// need to be updated with the new alias for the new cert.

block, _ := pem.Decode([]byte(dummy.TestCertificate1))
if block == nil {
t.Fatalf("couldn't parse a PEM block from TestCertificate1")
}

cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
t.Fatalf("Dummy certificate TestCertificate1 couldn't be parsed: %s", err)
}

alias := certAlias(cert.Raw, cert.Subject.String())

expectedAlias := "548b988f|CN=cmct-test-root,O=cert-manager"

if alias != expectedAlias {
t.Fatalf("expected alias to be %q but got %q", expectedAlias, alias)
}
}
106 changes: 3 additions & 103 deletions pkg/bundle/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,18 @@ import (
"bytes"
"context"
"crypto/sha256"
"encoding/hex"
"encoding/pem"
"fmt"
"slices"
"strings"

jks "github.com/pavlo-v-chernykh/keystore-go/v4"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"software.sslmate.com/src/go-pkcs12"

trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
"github.com/cert-manager/trust-manager/pkg/bundle/internal/truststore"
"github.com/cert-manager/trust-manager/pkg/util"
)

Expand Down Expand Up @@ -210,120 +208,22 @@ func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKey
return results.String(), nil
}

type jksEncoder struct {
password string
}

// encodeJKS creates a binary JKS file from the given PEM-encoded trust bundle and password.
// Note that the password is not treated securely; JKS files generally seem to expect a password
// to exist and so we have the option for one.
func (e jksEncoder) encode(trustBundle string) ([]byte, error) {
cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
if err != nil {
return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
}

// WithOrderedAliases ensures that trusted certs are added to the JKS file in order,
// which makes the files appear to be reliably deterministic.
ks := jks.New(jks.WithOrderedAliases())

for _, c := range cas {
alias := certAlias(c.Raw, c.Subject.String())

// Note on CreationTime:
// Debian's JKS trust store sets the creation time to match the time that certs are added to the
// trust store (i.e., it's effectively time.Now() at the instant the file is generated).
// Using that method would make our JKS files in trust-manager non-deterministic, leaving us with
// two options if we want to maintain determinism:
// - Using something from the cert being added (e.g. NotBefore / NotAfter)
// - Using a fixed time (i.e. unix epoch)
// We use NotBefore here, arbitrarily.

err = ks.SetTrustedCertificateEntry(alias, jks.TrustedCertificateEntry{
CreationTime: c.NotBefore,
Certificate: jks.Certificate{
Type: "X509",
Content: c.Raw,
},
})

if err != nil {
// this error should never happen if we set jks.Certificate correctly
return nil, fmt.Errorf("failed to add cert with alias %q to trust store: %w", alias, err)
}
}

buf := &bytes.Buffer{}

err = ks.Store(buf, []byte(e.password))
if err != nil {
return nil, fmt.Errorf("failed to create JKS file: %w", err)
}

return buf.Bytes(), nil
}

// certAlias creates a JKS-safe alias for the given DER-encoded certificate, such that
// any two certificates will have a different aliases unless they're identical in every way.
// This unique alias fixes an issue where we used the Issuer field as an alias, leading to
// different certs being treated as identical.
// The friendlyName is included in the alias as a UX feature when examining JKS files using a
// tool like `keytool`.
func certAlias(derData []byte, friendlyName string) string {
certHashBytes := sha256.Sum256(derData)
certHash := hex.EncodeToString(certHashBytes[:])

// Since certHash is the part which actually distinguishes between two
// certificates, put it first so that it won't be truncated if a cert
// with a really long subject is added. Not sure what the upper limit
// for length actually is, but it shouldn't matter here.

return certHash[:8] + "|" + friendlyName
}

type pkcs12Encoder struct {
password string
}

func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) {
cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
if err != nil {
return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
}

var entries []pkcs12.TrustStoreEntry
for _, c := range cas {
entries = append(entries, pkcs12.TrustStoreEntry{
Cert: c,
FriendlyName: certAlias(c.Raw, c.Subject.String()),
})
}

encoder := pkcs12.LegacyRC2

if e.password == "" {
encoder = pkcs12.Passwordless
}

return encoder.EncodeTrustStoreEntries(entries, e.password)
}

func (b *bundleData) populateData(bundles []string, formats *trustapi.AdditionalFormats) error {
b.data = strings.Join(bundles, "\n") + "\n"

if formats != nil {
b.binaryData = make(map[string][]byte)

if formats.JKS != nil {
encoded, err := jksEncoder{password: *formats.JKS.Password}.encode(b.data)
encoded, err := truststore.NewJKSEncoder(*formats.JKS.Password).Encode(b.data)
if err != nil {
return fmt.Errorf("failed to encode JKS: %w", err)
}
b.binaryData[formats.JKS.Key] = encoded
}

if formats.PKCS12 != nil {
encoded, err := pkcs12Encoder{password: *formats.PKCS12.Password}.encode(b.data)
encoded, err := truststore.NewPKCS12Encoder(*formats.PKCS12.Password).Encode(b.data)
if err != nil {
return fmt.Errorf("failed to encode PKCS12: %w", err)
}
Expand Down
Loading

0 comments on commit 756a098

Please sign in to comment.