Merge pull request #344 from sap-contributions/keep-java-certs

Load existing certs from pkcs12 store

certificate_loader_test.go

@@ -18,7 +18,6 @@ package libjvm_test
 
 import (
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -27,6 +26,7 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/pavlo-v-chernykh/keystore-go/v4"
 	"github.com/sclevine/spec"
+	"software.sslmate.com/src/go-pkcs12"
 
 	"github.com/paketo-buildpacks/libjvm"
 	"github.com/paketo-buildpacks/libjvm/internal"
@@ -81,7 +81,100 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 		})
 	})
 
-	context("load", func() {
+	context("load pkcs12", func() {
+		var (
+			path string
+		)
+
+		it.Before(func() {
+			in, err := os.Open(filepath.Join("testdata", "test-keystore.pkcs12"))
+			Expect(err).NotTo(HaveOccurred())
+			defer in.Close()
+
+			out, err := os.CreateTemp("", "certificate-loader")
+			Expect(err).NotTo(HaveOccurred())
+			defer out.Close()
+
+			_, err = io.Copy(out, in)
+			Expect(err).NotTo(HaveOccurred())
+
+			path = out.Name()
+		})
+
+		it.After(func() {
+			Expect(os.RemoveAll(path)).To(Succeed())
+		})
+
+		it("ignores non-existent file", func() {
+			c := libjvm.CertificateLoader{
+				CertFile: filepath.Join("testdata", "non-existent-file"),
+				Logger:   io.Discard,
+			}
+
+			Expect(c.Load(path, "changeit")).To(Succeed())
+		})
+
+		it("ignores non-existent directory", func() {
+			c := libjvm.CertificateLoader{
+				CertDirs: []string{filepath.Join("testdata", "non-existent-directory")},
+				Logger:   io.Discard,
+			}
+
+			Expect(c.Load(path, "changeit")).To(Succeed())
+		})
+
+		it("loads additional certificates from file", func() {
+			c := libjvm.CertificateLoader{
+				CertFile: filepath.Join("testdata", "certificates", "certificate-1.pem"),
+				Logger:   io.Discard,
+			}
+
+			Expect(c.Load(path, "changeit")).To(Succeed())
+
+			in, err := os.ReadFile(path)
+			Expect(err).NotTo(HaveOccurred())
+
+			ks, err := pkcs12.DecodeTrustStore(in, "")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ks).To(HaveLen(2))
+		})
+
+		it("loads additional certificates from directories", func() {
+			c := libjvm.CertificateLoader{
+				CertDirs: []string{filepath.Join("testdata", "certificates")},
+				Logger:   io.Discard,
+			}
+
+			Expect(c.Load(path, "changeit")).To(Succeed())
+
+			in, err := os.ReadFile(path)
+			Expect(err).NotTo(HaveOccurred())
+
+			ks, err := pkcs12.DecodeTrustStore(in, "")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ks).To(HaveLen(3))
+		})
+
+		internal.SkipIfRoot(it, "does not return error when keystore is read-only", func() {
+			Expect(os.Chmod(path, 0555)).To(Succeed())
+
+			c := libjvm.CertificateLoader{
+				CertDirs: []string{filepath.Join("testdata", "certificates")},
+				Logger:   io.Discard,
+			}
+
+			Expect(c.Load(path, "changeit")).To(Succeed())
+
+			in, err := os.ReadFile(path)
+			Expect(err).NotTo(HaveOccurred())
+
+			ks, err := pkcs12.DecodeTrustStore(in, "")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ks).To(HaveLen(1))
+		})
+	})
+
+	context("load jks", func() {
 		var (
 			path string
 		)
@@ -91,7 +184,7 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 			Expect(err).NotTo(HaveOccurred())
 			defer in.Close()
 
-			out, err := ioutil.TempFile("", "certificate-loader")
+			out, err := os.CreateTemp("", "certificate-loader")
 			Expect(err).NotTo(HaveOccurred())
 			defer out.Close()
 
@@ -108,7 +201,7 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 		it("ignores non-existent file", func() {
 			c := libjvm.CertificateLoader{
 				CertFile: filepath.Join("testdata", "non-existent-file"),
-				Logger:   ioutil.Discard,
+				Logger:   io.Discard,
 			}
 
 			Expect(c.Load(path, "changeit")).To(Succeed())
@@ -117,7 +210,7 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 		it("ignores non-existent directory", func() {
 			c := libjvm.CertificateLoader{
 				CertDirs: []string{filepath.Join("testdata", "non-existent-directory")},
-				Logger:   ioutil.Discard,
+				Logger:   io.Discard,
 			}
 
 			Expect(c.Load(path, "changeit")).To(Succeed())
@@ -126,7 +219,7 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 		it("loads additional certificates from file", func() {
 			c := libjvm.CertificateLoader{
 				CertFile: filepath.Join("testdata", "certificates", "certificate-1.pem"),
-				Logger:   ioutil.Discard,
+				Logger:   io.Discard,
 			}
 
 			Expect(c.Load(path, "changeit")).To(Succeed())
@@ -144,7 +237,7 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 		it("loads additional certificates from directories", func() {
 			c := libjvm.CertificateLoader{
 				CertDirs: []string{filepath.Join("testdata", "certificates")},
-				Logger:   ioutil.Discard,
+				Logger:   io.Discard,
 			}
 
 			Expect(c.Load(path, "changeit")).To(Succeed())
@@ -159,16 +252,12 @@ func testCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 			Expect(ks.Aliases()).To(HaveLen(3))
 		})
 
-		if internal.IsRoot() {
-			return
-		}
-
-		it("does not return error when keystore is read-only", func() {
+		internal.SkipIfRoot(it, "does not return error when keystore is read-only", func() {
 			Expect(os.Chmod(path, 0555)).To(Succeed())
 
 			c := libjvm.CertificateLoader{
 				CertDirs: []string{filepath.Join("testdata", "certificates")},
-				Logger:   ioutil.Discard,
+				Logger:   io.Discard,
 			}
 
 			Expect(c.Load(path, "changeit")).To(Succeed())

helper/link_local_dns_test.go

@@ -90,11 +90,7 @@ networkaddress.cache.negative.ttl=0
 `)))
 		})
 
-		if internal.IsRoot() {
-			return
-		}
-
-		it("warns if file is read-only", func() {
+		internal.SkipIfRoot(it, "warns if file is read-only", func() {
 			Expect(os.Chmod(path, 0555)).To(Succeed())
 
 			config := &ddns.ClientConfig{Servers: []string{"169.254.0.1"}}

helper/openssl_certificate_loader_test.go

@@ -98,11 +98,7 @@ func testOpenSSLCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 			Expect(ks.Aliases()).To(HaveLen(3))
 		})
 
-		if internal.IsRoot() {
-			return
-		}
-
-		it("does use temp keystore if keystore is read-only", func() {
+		internal.SkipIfRoot(it, "does use temp keystore if keystore is read-only", func() {
 			Expect(os.Chmod(path, 0555)).To(Succeed())
 
 			o := helper.OpenSSLCertificateLoader{CertificateLoader: cl, Logger: bard.NewLogger(ioutil.Discard)}
@@ -122,7 +118,7 @@ func testOpenSSLCertificateLoader(t *testing.T, context spec.G, it spec.S) {
 			Expect(env).To(HaveKeyWithValue("JAVA_TOOL_OPTIONS", fmt.Sprintf("-Djavax.net.ssl.trustStore=%s", helper.TmpTrustStore)))
 		})
 
-		it("does not return error when keystore and /tmp/truststore are read-only", func() {
+		internal.SkipIfRoot(it, "does not return error when keystore and /tmp/truststore are read-only", func() {
 			Expect(os.Chmod(path, 0555)).To(Succeed())
 			_, err := os.OpenFile(helper.TmpTrustStore, os.O_CREATE, 0)
 			Expect(err).NotTo(HaveOccurred())

helper/security_providers_configurer_test.go

@@ -106,11 +106,7 @@ security.provider.6=FOXTROT
 `)))
 				})
 
-				if internal.IsRoot() {
-					return
-				}
-
-				it("warns if the file is read-only", func() {
+				internal.SkipIfRoot(it, "warns if the file is read-only", func() {
 					Expect(os.Chmod(path, 0555)).To(Succeed())
 
 					Expect(helper.SecurityProvidersConfigurer{}.Execute()).To(BeNil())

internal/if_not_root.go

@@ -18,9 +18,18 @@ package internal
 
 import (
 	"os/user"
+
+	"github.com/sclevine/spec"
 )
 
-func IsRoot() bool {
+func isRoot() bool {
 	u, _ := user.Current()
 	return u.Username == "root"
 }
+
+func SkipIfRoot(it spec.S, text string, f func(), options ...spec.Option) {
+	if isRoot() {
+		it = it.Pend
+	}
+	it(text, f, options...)
+}

keystore.go

@@ -47,7 +47,7 @@ func DetectKeystore(location string) (Keystore, error) {
 		return NewJKSKeystore(location, "changeit")
 	}
 
-	return NewPasswordLessPKCS12Keystore(location), nil
+	return NewPasswordLessPKCS12Keystore(location)
 }
 
 var _ Keystore = &JKSKeystore{}
@@ -119,11 +119,29 @@ type PasswordLessPKCS12Keystore struct {
 	entries  []pkcs12.TrustStoreEntry
 }
 
-func NewPasswordLessPKCS12Keystore(location string) *PasswordLessPKCS12Keystore {
+func NewPasswordLessPKCS12Keystore(location string) (*PasswordLessPKCS12Keystore, error) {
+	in, err := os.ReadFile(location)
+	if err != nil {
+		return nil, err
+	}
+
+	x509Certs, err := pkcs12.DecodeTrustStore(in, "")
+	if err != nil {
+		return nil, err
+	}
+
+	var entries []pkcs12.TrustStoreEntry
+	for _, x509Cert := range x509Certs {
+		entries = append(entries, pkcs12.TrustStoreEntry{
+			Cert:         x509Cert,
+			FriendlyName: x509Cert.Subject.String(),
+		})
+	}
+
 	return &PasswordLessPKCS12Keystore{
 		location: location,
-		entries:  []pkcs12.TrustStoreEntry{},
-	}
+		entries:  entries,
+	}, nil
 }
 
 func (k *PasswordLessPKCS12Keystore) Add(name string, b *pem.Block) error {
@@ -141,6 +159,10 @@ func (k *PasswordLessPKCS12Keystore) Add(name string, b *pem.Block) error {
 }
 
 func (k *PasswordLessPKCS12Keystore) Write() error {
+	if unix.Access(k.location, unix.W_OK) != nil {
+		return nil
+	}
+
 	out, err := os.OpenFile(k.location, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644)
 	if err != nil {
 		return err
@@ -155,7 +177,5 @@ func (k *PasswordLessPKCS12Keystore) Write() error {
 }
 
 func (k *PasswordLessPKCS12Keystore) Len() int {
-	data, _ := os.ReadFile(k.location)
-	certs, _ := pkcs12.DecodeTrustStore(data, "")
-	return len(certs) + len(k.entries)
+	return len(k.entries)
 }

keystore_test.go

@@ -97,7 +97,8 @@ func testKeystore(t *testing.T, context spec.G, it spec.S) {
 		})
 
 		it("can be written", func() {
-			ks := libjvm.NewPasswordLessPKCS12Keystore(path)
+			ks, err := libjvm.NewPasswordLessPKCS12Keystore(path)
+			Expect(err).ToNot(HaveOccurred())
 			Expect(ks.Len()).To(Equal(1))
 			cert, err := os.ReadFile(filepath.Join("testdata", "cert.pem"))
 			Expect(err).ToNot(HaveOccurred())