cert-manager / trust-manager

trust-manager is an operator for distributing trust bundles across a Kubernetes cluster.
https://cert-manager.io/docs/projects/trust-manager/
Apache License 2.0
233 stars 64 forks source link

Bundle generating empty truststore.p12 when no password is provided #296

Closed arjunprasad2143 closed 4 months ago

arjunprasad2143 commented 5 months ago

I used the below config to create a pkcs12 additional format bundle without password. But the created truststore is empty.

apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
  name: truststore
spec:
  sources:
    - secret:
        key: root-certs.pem
        name: trusted-certs
  target:
    additionalFormats:
      pkcs12:
        key: truststore.p12
        password: ''
    secret:
      key: root-certs.pem

The secret contains the required bundle, but the truststore.p12 is empty. The result remains the same when I use a configmap source.

SgtCoDFish commented 4 months ago

I tested this a bit locally and I think what's happening is simply that keytool can't parse the file. Still worth a fix, but we're not making an empty p12 - we're just making one that keytool can't read.

Our tests do check that a passwordless file can be read - but obviously they're using our pkcs12 library which can read passwordless files.

The library says this:

Passwordless encodes PKCS#12 files without any encryption or MACs. A lot of software has trouble reading such files, so it's probably only useful for creating Java trust stores using Encoder.EncodeTrustStore or Encoder.EncodeTrustStoreEntries.

I've confirmed that the following patch creates a passwordless PKCS#12 file that keytool can read:

diff --git a/pkg/bundle/sync.go b/pkg/bundle/sync.go
index 1966af0..ea36eaf 100644
--- a/pkg/bundle/sync.go
+++ b/pkg/bundle/sync.go
@@ -320,7 +320,13 @@ func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) {
        })
    }

-   return pkcs12.LegacyRC2.EncodeTrustStoreEntries(entries, e.password)
+   encoder := pkcs12.LegacyRC2
+
+   if e.password == "" {
+       encoder = pkcs12.Passwordless
+   }
+
+   return encoder.EncodeTrustStoreEntries(entries, e.password)
 }

 // syncConfigMapTarget syncs the given data to the target ConfigMap in the given namespace.

And I've also confirmed that the resulting file is readable by our tests. I'd like to check with the team if we're happy to make that change in general.

erikgb commented 4 months ago

Thanks for the investigation, @SgtCoDFish! I submitted the PR adding PKCS#12 support, and I should have tested the truststore with keytool. Java applications probably represent the far majority of consumers of PKCS#12 truststores in trust-manager, and it would be a pity if the Java keytoolcannot read it. I am highly positive about your suggested change! 👍