Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem about SelfCertSignerFactory class #713

Open
WindzCUHK opened this issue Jun 20, 2019 · 6 comments
Open

Problem about SelfCertSignerFactory class #713

WindzCUHK opened this issue Jun 20, 2019 · 6 comments

Comments

@WindzCUHK
Copy link
Contributor

WindzCUHK commented Jun 20, 2019

Problems

  1. The CA certificate generated in SelfCertSignerFactory is not registered to ZTS truststore at runtime.
    The client certificate issued by ZTS cannot not be verified by ZTS.
    https://github.com/yahoo/athenz/blob/0c46baf5dad884b139653d649f8bfa546f96ff73/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/SelfCertSignerFactory.java#L75-L76

Suggested solution 1

  1. add the CA certificate generated in SelfCertSignerFactory to ZTS truststore during the start up phase

Suggested solution 2

  1. generate the CA certificate in advance with openssl
  2. add the CA certificate in ZTS truststore
  3. use the KeyStoreCertSigner class to issue client certificates with pre-generated CA certificate storing in a keystore
    1. https://github.com/WindzCUHK/athenz/tree/dockerize.v2/docker/acceptance-test/KeyStoreCertSigner

Suggested solution 3

  1. generate a CA certificate with openssl with the same private key and subject name
    1. openssl config file used
  2. add the generated CA certificate to ZTS truststore
  3. use the same private key and subject name in the SelfCertSignerFactory class
  4. As the CA certificate generated in runtime has the same private key and subject name with the CA certificate generated by openssl, client certificate generated by SelfCertSigner class should be valid when validating with the CA certificate generated by openssl which stored in the ZTS truststore.

Problem with suggested solution 3

  • The client certificate works as expected when using curl.

  • However, the client certificate cannot work with golang binaries (e.g. athenz-cli).

  • details
    As the byte encoding is difference while decoded string is equal (Issuer of the client certificate V.S. Subject name of the CA certificate generated with openssl storing in the ZTS truststore), golang is not sending the client certificate to the ZTS server as a service identity, and causing 401 with No credentials in ZTS's log.
    https://github.com/golang/go/blob/master/src/crypto/tls/handshake_client.go#L957-L961

    • CertificateRequest in JAVA debug log
    *** CertificateRequest
    Cert Types: RSA, DSS, ECDSA
    Supported Signature Algorithms: SHA512withECDSA, SHA512withRSA, SHA384withECDSA, SHA384withRSA, SHA256withECDSA, SHA256withRSA, SHA256withDSA, SHA224withECDSA, SHA224withRSA, SHA224withDSA, SHA1withECDSA, SHA1withRSA, SHA1withDSA
    Cert Authorities:
    <CN=localhost, OU=Testing Domain, O=Athenz, ST=CA, C=US>
    <CN=Self Signed Athenz CA, O=Athenz, C=US>
    

Questions

  1. Is there any better solutions to solve this issue?
  2. As the Crypto.java is not supporting unencrypted PKCS #8 private key, the loadPrivateKey() method returns null and causes error in SelfCertSigner. Is it intended?
@havetisyan
Copy link
Collaborator

HI @WindzCUHK

  1. So the selfcertsignerfactory is primarily for testing purposes so haven't really looked into automated the steps. Looking at your specified options - I've typically done 3 but I've never seen the issue you're describing (or maybe I've always used curl but that sounds interesting why/how the values could be different).

Anyway, suggested solution (2) sounds like a good option. Though I would make sure to include the Self part in the factory/class name to make sure the user knows these are Self Signed certs and thus not really for production use.

  1. Have you tried passing just "" for the password?

@WindzCUHK
Copy link
Contributor Author

WindzCUHK commented Jul 16, 2019

@havetisyan

Thank you for answering. Below a few follow up questions:

  1. Do Athenz suggest to use a CA certificate loaded from file (keystore) for certificate signing in production?
  2. Should we make a PR about the KeyStoreCertSigner class to the master branch?
  3. For password, do you mean setting athenz.zts.self_signer_private_key_password= in the property file instead of comment it out?

@havetisyan
Copy link
Collaborator

  1. For certificate signing the recommended solution is to use a signer that stores the private key in HSM (like the one that was just open sourced: https://github.com/yahoo/crypki).

For example, in our production environment, our Paranoids Eng team is the team running our certificate signer. It'a s complete separate service with team managing it and as such they provide the CA certificates. We just include that CA cert in your truststore and configure jetty to start up with that truststore.

  1. Sure, you can call SelfCertKeyStoreSigner and include it in the master branch. it's just another example of self cert signer that can be used for dev environments.

  2. Yes, if you set it to "", doesn't that work?

@WindzCUHK
Copy link
Contributor Author

@havetisyan

Thank you for the answer.

  1. As for k8s deployment, I think it is a common practice to store private keys as a secret in production. Hence, deploying Athenz in k8s, it may be easier for the user to mount the private key in secret as a PEM file or as a keystore inside the pod, and we provide a certificate signer implementation that can load the file/keystore directly. Are there any concerns in this use case?
  2. Thank you. We will consider having a PR later on.
  3. Yes, Crypto.loadPrivateKey() returns null.
    Explicit conversion on the private key format is required.
    https://github.com/WindzCUHK/athenz/blob/db3c6b62345ad1b5b2356fab7ff903e7754ee539/docker/setup-scripts/6.1.create-zts-cert-signer-pair.sh#L25-L26

@havetisyan
Copy link
Collaborator

  1. Using a private key as a secret in production is ok for a standard application. it's not recommended for your certificate signer which uses that as the ROOT CA private key. Your private key in production is compromised, your application is compromised. Your ROOT CA private key is compromised, your entire production environment is compromised since you can now generate certs for any identity. So this is why I recommend you have a solution where the private key is stored in HSM and not as a file somewhere on your system.

  2. if that's a use case you need to support, I would suggest you look at providing a new method in Crypto class that supports those. You can either create a new method or just extend the existing method to handle it when the password is passed as null.

@WindzCUHK
Copy link
Contributor Author

@havetisyan

  1. Thank you for the explanation. We will lower the priority of SelfCertKeyStoreSigner PR currently.
  2. The Crypto.loadPrivateKey() method supports PKCS1 unencrypted key currently.
    https://github.com/yahoo/athenz/blob/6d8a149a511baddfd3930dfd21afde19c132442f/libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/util/Crypto.java#L586-L590
    Is there any concern PKCS8 unencrypted key is not supported? It is instanceof PrivateKeyInfo.
  3. And, is it better to throw exception like unsupported key type when the private key format is not supported inside the method rather than null?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants