search

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
256 stars 69 forks source link

Should update target JKS/PKCS when password changed #433

Open Shawcs opened 2 months ago

Shawcs commented 2 months ago

Hi !

I noticed some problems around password for generated p12 and JKS file.

short description:

For JKS you can alway open it with a no password and the password you setup in the bundle

For p12 and JKS if the password is updated after the bundle creation the created resources in destination namespace are not re generated and keep the old password, even if the bundle update it's generation.

Reproduction of the problem

  1. create a bundle with jks and p12 with a password

    apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
name: my-bundle
spec:
sources:
- inLine: |
    -----BEGIN CERTIFICATE-----
    MIIDojCCAYqgAwIBAgIQV5ocp05c1d2ULNLEDrdCpTANBgkqhkiG9w0BAQsFADBH
    MQswCQYDVQQGEwJDSDEnMCUGA1UEChMeQmFucXVlIExvbWJhcmQgT2RpZXIgZXQg
    Q2llIFNBMQ8wDQYDVQQDEwZMTyBEUEkwHhcNMjQwMzA3MDAwMDAwWhcNMjUwMzA3
    MjM1OTU5WjAVMRMwEQYDVQQDEwpnaXRodWIuY29tMFkwEwYHKoZIzj0CAQYIKoZI
    zj0DAQcDQgAEQrPuGOisrWzPTzsVzujNAMvKeM1GRDs18c2N5R6LemewOMjO0Ep1
    yESxF/xn4Zj7tlsTeMT5zz4Li1DQN/K1zKOBhjCBgzAdBgNVHQ4EFgQUO2g/NDr1
    RzTK76ZOPZq9Xm56zJ8wDgYDVR0PAQH/BAQDAgeAMAwGA1UdEwEB/wQCMAAwHQYD
    VR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMCUGA1UdEQQeMByCCmdpdGh1Yi5j
    b22CDnd3dy5naXRodWIuY29tMA0GCSqGSIb3DQEBCwUAA4ICAQBEaD1pszjmbtjc
    nE0s43FN2oU/S23Wf331M6Ae72F9B3ceqY/X0oPIHsOtpA7gSDOzjs4sNrHq34qn
    q3SRpmiDepmY4Ba2+gXNg5dWeul2e7ht22L/jYR8oT0pGClWkLiveijBT3Veqbxu
    jT3lxYGd1ey20feg4rQIw61GgaZ2dyHjlpj8FrCxSSLnULIM4db+04+2PXbTGl9J
    O+UQzbDI34KoKKWCCvCDFziCTG3rblv3RcGOCUcAXL1WpJIPlhhSYK+Dvv1Op2/C
    bd8LU10GmzZpZ/aR4SZDymggc32xvXEfPSrM36qgDcbg7Vb3mm+uZUgWKHmh1pnA
    1DeiFM9PT9GGN8m5ioMsbYIbNZUUw2jt8Gbz0CWSQlJWAfo0LEbVnIH4TG10m6Ix
    zqDgzL7QYe7XpGqY1LuwNidozLvRUaZkhDud7XlgHYYQwrP4z6/ekawett0LdvSL
    JdSRFvE7pO4K2kXxFr8YzeET0jrZ+JeVX8WYbsfUlo0UdMVnViIFrgbTd8v6Mlev
    kBG18BDEDA8hYS1JsSmpeiy/c/WsIzihgln25RcaaUPZBszu/yQp3WwtNVJojqnF
    16LisgMdnA/1gj801KmoTYUfQFTPVkITGjYVogYGZ1B9FYNwl7ymulCRrVNOzqs/
    G3VnreyXrukwRbMI/MR4ccXl5/n+Hw==
    -----END CERTIFICATE-----
target:
additionalFormats:
  jks:
    key: trust.jks
    password: firstpassword
  pkcs12:
    key: certs.p12
    password: firstpassword
namespaceSelector:
  matchLabels:
    kubernetes.io/metadata.name: infra
configMap:
  key: configmap-key.pem

  2. From the generated configmap download the jks and p12 and try to read them

    keytool -v -list -storetype jks -keystore trust.jks --storepass firstpassword

    and

    echo "" | keytool -v -list -storetype jks -keystore trust.jks

    (this is like entering the command and hit enter when the keytool prompt for jks password) in both cas the keytool list the jks content. Is that normal that we can open the JKS with no password (and not empty password) ?

For the p12:

openssl pkcs12 -info -in certs_firstpass.p12 -password pass:firstpassword

  1. update the bundle password

    apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
name: my-bundle
spec:
sources:
- inLine: |
    -----BEGIN CERTIFICATE-----
    MIIDojCCAYqgAwIBAgIQV5ocp05c1d2ULNLEDrdCpTANBgkqhkiG9w0BAQsFADBH
    MQswCQYDVQQGEwJDSDEnMCUGA1UEChMeQmFucXVlIExvbWJhcmQgT2RpZXIgZXQg
    Q2llIFNBMQ8wDQYDVQQDEwZMTyBEUEkwHhcNMjQwMzA3MDAwMDAwWhcNMjUwMzA3
    MjM1OTU5WjAVMRMwEQYDVQQDEwpnaXRodWIuY29tMFkwEwYHKoZIzj0CAQYIKoZI
    zj0DAQcDQgAEQrPuGOisrWzPTzsVzujNAMvKeM1GRDs18c2N5R6LemewOMjO0Ep1
    yESxF/xn4Zj7tlsTeMT5zz4Li1DQN/K1zKOBhjCBgzAdBgNVHQ4EFgQUO2g/NDr1
    RzTK76ZOPZq9Xm56zJ8wDgYDVR0PAQH/BAQDAgeAMAwGA1UdEwEB/wQCMAAwHQYD
    VR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMCUGA1UdEQQeMByCCmdpdGh1Yi5j
    b22CDnd3dy5naXRodWIuY29tMA0GCSqGSIb3DQEBCwUAA4ICAQBEaD1pszjmbtjc
    nE0s43FN2oU/S23Wf331M6Ae72F9B3ceqY/X0oPIHsOtpA7gSDOzjs4sNrHq34qn
    q3SRpmiDepmY4Ba2+gXNg5dWeul2e7ht22L/jYR8oT0pGClWkLiveijBT3Veqbxu
    jT3lxYGd1ey20feg4rQIw61GgaZ2dyHjlpj8FrCxSSLnULIM4db+04+2PXbTGl9J
    O+UQzbDI34KoKKWCCvCDFziCTG3rblv3RcGOCUcAXL1WpJIPlhhSYK+Dvv1Op2/C
    bd8LU10GmzZpZ/aR4SZDymggc32xvXEfPSrM36qgDcbg7Vb3mm+uZUgWKHmh1pnA
    1DeiFM9PT9GGN8m5ioMsbYIbNZUUw2jt8Gbz0CWSQlJWAfo0LEbVnIH4TG10m6Ix
    zqDgzL7QYe7XpGqY1LuwNidozLvRUaZkhDud7XlgHYYQwrP4z6/ekawett0LdvSL
    JdSRFvE7pO4K2kXxFr8YzeET0jrZ+JeVX8WYbsfUlo0UdMVnViIFrgbTd8v6Mlev
    kBG18BDEDA8hYS1JsSmpeiy/c/WsIzihgln25RcaaUPZBszu/yQp3WwtNVJojqnF
    16LisgMdnA/1gj801KmoTYUfQFTPVkITGjYVogYGZ1B9FYNwl7ymulCRrVNOzqs/
    G3VnreyXrukwRbMI/MR4ccXl5/n+Hw==
    -----END CERTIFICATE-----
target:
additionalFormats:
  jks:
    key: trust.jks
    password: secondpassword
  pkcs12:
    key: certs.p12
    password: secondpassword
namespaceSelector:
  matchLabels:
    kubernetes.io/metadata.name: infra
configMap:
  key: configmap-key.pem
status:
conditions:
- lastTransitionTime: '2024-09-10T14:16:47Z'
  message: 'Successfully synced Bundle to namespaces that match this label selector: kubernetes.io/metadata.name=infra'
  observedGeneration: 1
  reason: Synced
  status: 'True'
  type: Synced

    here the status will update in: 

    status:
conditions:
- lastTransitionTime: '2024-09-10T14:16:47Z'
  message: 'Successfully synced Bundle to namespaces that match this label selector: kubernetes.io/metadata.name=infra'
  observedGeneration: 2
  reason: Synced
  status: 'True'
  type: Synced

  2. refresh and download again the p12 and jks from the configmap

  3. Open them again with password (I renamed the file this time with a "second" to avoid conflicts) : for p12

    
openssl pkcs12 -info -in certs_second.p12 -password pass:secondpassword
MAC: sha1, Iteration 1
MAC length: 20, salt length: 8
Mac verify error: invalid password?

openssl pkcs12 -info -in certs_second.p12 -password pass:firstpassword <- works


for JKS
```bash
keytool -v -list -storetype jks -keystore trust_second.jks --storepass 'secondpassword'
keytool error: java.io.IOException: Keystore was tampered with, or password was incorrect
java.io.IOException: Keystore was tampered with, or password was incorrect
        at java.base/sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:813)
        at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:221)
        at java.base/java.security.KeyStore.load(KeyStore.java:1473)
        at java.base/sun.security.tools.keytool.Main.doCommands(Main.java:954)
        at java.base/sun.security.tools.keytool.Main.run(Main.java:423)
        at java.base/sun.security.tools.keytool.Main.main(Main.java:416)
Caused by: java.security.UnrecoverableKeyException: Password verification failed
        at java.base/sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:811)
        ... 5 more

keytool -v -list -storetype jks -keystore trust_second.jks --storepass 'firstpassword' <- works
echo "" | keytool -v -list -storetype jks -keystore trust_second.jks <- works

Suggestion

For me there is two options:

erikgb commented 2 months ago

Is that normal that we can open the JKS with no password (and not empty password) ?

I think this is the way JKS/keytool works. The password is just an integrity check, and when the password is omitted, the integrity check is just skipped.

But I think a truststore should be updated if the password changes. Even if cert-manager does not yet support this, ref. https://github.com/cert-manager/cert-manager/issues/3450. But in cert-manager solving this is more complex, since the password is externalized. And not inlined, as in trust-manager.

/kind bug

arsenalzp commented 1 month ago

It seems this was my task - introduction of security trust stores by passwords Can the issue be in that reconcile function doesn't compare old and new password fields?

erikgb commented 1 month ago

It seems it was my task. Can the issue be in that reconcile function doesn't compare old and new password fields?

We should probably add a new hash annotation for the truststore password (if any). And reconcile the target if hash doesn't match . Similar to what we do with bundle data.

arsenalzp commented 1 month ago

How is this issue urgent? I can tackle this. P.S. I see tests regarding this issue have been merged

arsenalzp commented 1 month ago

/assign

arsenalzp commented 1 month ago

Hello, I tried to fix this issue, password changes are reflecting rebuilding of additional formats targets. However, it brought an issue with Patch of password hash field.