Revert "Use PKCS#12 encoder to encode JKS"

Signed-off-by: Erik Godding Boye <[email protected]>

Author: erikgb

LICENSES

@@ -31,6 +31,7 @@ github.com/monochromegane/go-gitignore,https://github.com/monochromegane/go-giti
 github.com/munnerz/goautoneg,https://github.com/munnerz/goautoneg/blob/a7dc8b61c822/LICENSE,BSD-3-Clause
 github.com/onsi/ginkgo/v2,https://github.com/onsi/ginkgo/blob/v2.23.4/LICENSE,MIT
 github.com/onsi/gomega,https://github.com/onsi/gomega/blob/v1.37.0/LICENSE,MIT
+github.com/pavlo-v-chernykh/keystore-go/v4,https://github.com/pavlo-v-chernykh/keystore-go/blob/v4.5.0/LICENSE,MIT
 github.com/peterbourgon/diskv,https://github.com/peterbourgon/diskv/blob/v2.0.1/LICENSE,MIT
 github.com/pkg/errors,https://github.com/pkg/errors/blob/v0.9.1/LICENSE,BSD-2-Clause
 github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil,https://github.com/prometheus/client_golang/blob/v1.22.0/internal/github.com/golang/gddo/LICENSE,BSD-3-Clause

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/google/go-cmp v0.7.0
 	github.com/onsi/ginkgo/v2 v2.23.4
 	github.com/onsi/gomega v1.37.0
+	github.com/pavlo-v-chernykh/keystore-go/v4 v4.5.0
 	github.com/spf13/cobra v1.9.1
 	github.com/spf13/pflag v1.0.6
 	github.com/stretchr/testify v1.10.0

go.sum

@@ -94,6 +94,8 @@ github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus
 github.com/onsi/ginkgo/v2 v2.23.4/go.mod h1:Bt66ApGPBFzHyR+JO10Zbt0Gsp4uWxu5mIOTusL46e8=
 github.com/onsi/gomega v1.37.0 h1:CdEG8g0S133B4OswTDC/5XPSzE1OeP29QOioj2PID2Y=
 github.com/onsi/gomega v1.37.0/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0=
+github.com/pavlo-v-chernykh/keystore-go/v4 v4.5.0 h1:2nosf3P75OZv2/ZO/9Px5ZgZ5gbKrzA3joN1QMfOGMQ=
+github.com/pavlo-v-chernykh/keystore-go/v4 v4.5.0/go.mod h1:lAVhWwbNaveeJmxrxuSTxMgKpF6DjnuVpn6T8WiBwYQ=
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

pkg/bundle/bundle_test.go

@@ -55,8 +55,7 @@ func testEncodeJKS(t *testing.T, data string) []byte {
 		t.Fatal(err)
 	}
 
-	// Note: Must use the PKCS12 default password here, as encoding with (non-empty) password is non-deterministic.
-	encoded, err := truststore.NewJKSEncoder(trustapi.DefaultPKCS12Password).Encode(certPool)
+	encoded, err := truststore.NewJKSEncoder(trustapi.DefaultJKSPassword).Encode(certPool)
 	if err != nil {
 		t.Error(err)
 	}
@@ -145,8 +144,7 @@ func Test_Reconcile(t *testing.T) {
 				KeySelector: trustapi.KeySelector{
 					Key: "target.jks",
 				},
-				// Note: Must use the PKCS12 default password here, as encoding with (non-empty) password is non-deterministic.
-				Password: ptr.To(trustapi.DefaultPKCS12Password),
+				Password: ptr.To(trustapi.DefaultJKSPassword),
 			},
 		}
 		jksDefaultAdditionalFormatsOldPassword = trustapi.AdditionalFormats{

pkg/bundle/internal/truststore/types.go

@@ -17,9 +17,12 @@ limitations under the License.
 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/apis/trust/v1alpha1"
@@ -31,7 +34,52 @@ type Encoder interface {
 }
 
 func NewJKSEncoder(password string) Encoder {
-	return NewPKCS12Encoder(password, v1alpha1.LegacyRC2PKCS12Profile)
+	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 *util.CertPool) ([]byte, error) {
+	// 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 trustBundle.Certificates() {
+		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.
+
+		if err := ks.SetTrustedCertificateEntry(alias, keystore.TrustedCertificateEntry{
+			CreationTime: c.NotBefore,
+			Certificate: keystore.Certificate{
+				Type:    "X509",
+				Content: c.Raw,
+			},
+		}); 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{}
+
+	if err := ks.Store(buf, []byte(e.password)); err != nil {
+		return nil, fmt.Errorf("failed to create JKS file: %w", err)
+	}
+
+	return buf.Bytes(), nil
 }
 
 func NewPKCS12Encoder(password string, profile v1alpha1.PKCS12Profile) Encoder {

pkg/bundle/internal/truststore/types_test.go

@@ -17,12 +17,13 @@ limitations under the License.
 package truststore
 
 import (
+	"bytes"
 	"crypto/x509"
 	"encoding/pem"
 	"testing"
 
+	"github.com/pavlo-v-chernykh/keystore-go/v4"
 	"github.com/stretchr/testify/assert"
-	"software.sslmate.com/src/go-pkcs12"
 
 	"github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/util"
@@ -35,12 +36,10 @@ func Test_Encoder_Deterministic(t *testing.T) {
 		expNonDeterministic bool
 	}{
 		"JKS default password": {
-			encoder:             NewJKSEncoder(v1alpha1.DefaultJKSPassword),
-			expNonDeterministic: true,
+			encoder: NewJKSEncoder(v1alpha1.DefaultJKSPassword),
 		},
 		"JKS custom password": {
-			encoder:             NewJKSEncoder("my-password"),
-			expNonDeterministic: true,
+			encoder: NewJKSEncoder("my-password"),
 		},
 		"PKCS#12 default password": {
 			encoder: NewPKCS12Encoder(v1alpha1.DefaultPKCS12Password, ""),
@@ -105,13 +104,19 @@ func Test_encodeJKSAliases(t *testing.T) {
 		t.Fatalf("didn't expect an error but got: %s", err)
 	}
 
-	certs, err := pkcs12.DecodeTrustStore(jksFile, v1alpha1.DefaultJKSPassword)
+	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)
 	}
 
-	if len(certs) != 2 {
-		t.Fatalf("expected two certs in JKS file but got %d", len(certs))
+	entryNames := ks.Aliases()
+
+	if len(entryNames) != 2 {
+		t.Fatalf("expected two certs in JKS file but got %d", len(entryNames))
 	}
 }
 

pkg/bundle/source_test.go

@@ -17,11 +17,13 @@ limitations under the License.
 package bundle
 
 import (
+	"bytes"
 	"encoding/pem"
 	"errors"
 	"strings"
 	"testing"
 
+	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -488,14 +490,24 @@ func Test_buildSourceBundle(t *testing.T) {
 			assert.Equal(t, test.expJKS, jksExists)
 
 			if test.expJKS {
-				certs, err := pkcs12.DecodeTrustStore(binData, password)
+				reader := bytes.NewReader(binData)
+
+				ks := jks.New()
+
+				err := ks.Load(reader, []byte(password))
 				assert.Nil(t, err)
 
-				assert.Len(t, certs, 1)
+				entryNames := ks.Aliases()
+
+				assert.Len(t, entryNames, 1)
+				assert.True(t, ks.IsTrustedCertificateEntry(entryNames[0]))
+
+				// Safe to ignore errors here, we've tested that it's present and a TrustedCertificateEntry
+				cert, _ := ks.GetTrustedCertificateEntry(entryNames[0])
 
 				// Only one certificate block for this test, so we can safely ignore the `remaining` byte array
 				p, _ := pem.Decode([]byte(data))
-				assert.Equal(t, p.Bytes, certs[0].Raw)
+				assert.Equal(t, p.Bytes, cert.Certificate.Content)
 			}
 
 			binData, pkcs12Exists := resolvedBundle.Data.BinaryData[pkcs12Key]

test/env/data.go

@@ -23,11 +23,11 @@ import (
 	"fmt"
 	"strings"
 
+	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"software.sslmate.com/src/go-pkcs12"
 
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	bundlectrl "github.com/cert-manager/trust-manager/pkg/bundle"
@@ -345,24 +345,29 @@ func EventuallyBundleHasSyncedAllNamespacesContains(ctx context.Context, cl clie
 
 // CheckJKSFileSynced ensures that the given JKS data
 func CheckJKSFileSynced(jksData []byte, expectedPassword string, expectedCertPEMData string) error {
+	reader := bytes.NewReader(jksData)
 	certPool := util.NewCertPool(util.WithFilteredExpiredCerts(false))
-	if err := certPool.AddCertsFromPEM([]byte(expectedCertPEMData)); err != nil {
-		return fmt.Errorf("invalid PEM data passed to CheckJKSFileSynced: %s", err)
-	}
 
-	certs, err := pkcs12.DecodeTrustStore(jksData, expectedPassword)
+	ks := jks.New()
+
+	err := ks.Load(reader, []byte(expectedPassword))
 	if err != nil {
 		return err
 	}
 
+	err = certPool.AddCertsFromPEM([]byte(expectedCertPEMData))
+	if err != nil {
+		return fmt.Errorf("invalid PEM data passed to CheckJKSFileSynced: %s", err)
+	}
+
 	// TODO: check that the cert content matches expectedCertPEMData exactly, not just
 	// that the count is the same
 
-	certCount := len(certs)
+	aliasCount := len(ks.Aliases())
 	expectedPEMCount := certPool.Size()
 
-	if certCount != expectedPEMCount {
-		return fmt.Errorf("expected %d certificates in JKS but found %d", expectedPEMCount, certCount)
+	if aliasCount != expectedPEMCount {
+		return fmt.Errorf("expected %d certificates in JKS but found %d", expectedPEMCount, aliasCount)
 	}
 
 	return nil