FakeKS restored to self-signed. Add FakeChainedKS. #125

ignasi35 · 2018-10-05T12:13:52Z

Using a keyStore with multiple privateKeyPair entries to create an SSLContext with a default KeyManagerFactory has the consequence that you can't set what key should be used. There are mechanisms to setup SNI or to allow connection-specific key selection. Until those are necessary and implemented, using a single-key KeyStore is the simplest option.

Note: we could create a collection of tools to create several KeyStore's (one for the CA, another for the server, a third one for client-side SSL auth) but that's not the purpose of this PR.

Fixes #115

dwijnand

LGTM. One question.

dwijnand · 2018-10-05T12:19:54Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeKeyStore.scala

+    val SignatureAlgorithmName = "SHA256withRSA"
+    val KeyPairAlgorithmName = "RSA"
+    val KeyPairKeyLength = 2048 // 2048 is the NIST acceptable key length until 2030
+    val KeystoreType = "PKCS12"


Is this change intentional?

Yes. kyetool keeps alerting jks is proprietary and recommending a migration:

$ keytool -keystore generated.keystore -list Enter keystore password: ***************** WARNING WARNING WARNING ***************** * The integrity of the information stored in your keystore * * has NOT been verified! In order to verify its integrity, * * you must provide your keystore password. * ***************** WARNING WARNING WARNING ***************** Keystore type: jks Keystore provider: SUN Your keystore contains 4 entries playgeneratedtrusted, Sep 20, 2018, trustedCertEntry, Certificate fingerprint (SHA1): F3:7E:F7:12:BC:73:EB:A7:CF:AF:C7:49:CD:3A:B1:C1:F0:1B:75:98 playgenerated, Sep 20, 2018, PrivateKeyEntry, Certificate fingerprint (SHA1): F3:7E:F7:12:BC:73:EB:A7:CF:AF:C7:49:CD:3A:B1:C1:F0:1B:75:98 playgeneratedcatrusted, Sep 20, 2018, trustedCertEntry, Certificate fingerprint (SHA1): 4C:F3:C9:5E:D8:F1:83:46:BD:89:7C:1F:7D:BD:24:EB:94:9C:E8:8E playgeneratedca, Sep 20, 2018, PrivateKeyEntry, Certificate fingerprint (SHA1): 4C:F3:C9:5E:D8:F1:83:46:BD:89:7C:1F:7D:BD:24:EB:94:9C:E8:8E Warning: The JKS keystore uses a proprietary format. It is recommended to migrate to PKCS12 which is an industry standard format using "keytool -importkeystore -srckeystore generated.keystore -destkeystore generated.keystore -deststoretype pkcs12".

vs

$ keytool -keystore selfsigned.keystore -list Enter keystore password: ***************** WARNING WARNING WARNING ***************** * The integrity of the information stored in your keystore * * has NOT been verified! In order to verify its integrity, * * you must provide your keystore password. * ***************** WARNING WARNING WARNING ***************** Keystore type: jks Keystore provider: SUN Your keystore contains 1 entry sslconfig-selfsigned, Oct 5, 2018, PrivateKeyEntry,

Hmm, the trust entry on the selfsigned.keystore isn't listed. Is it really added?

ignasi35

This is an early commit of changes in "FakeXyz tools for dev mode and testkit".

There's still code duplication and API improvements we could make.

ignasi35 · 2018-10-05T12:16:09Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeChainedKeyStore.scala

+
+    val DistinguishedName = "CN=localhost, OU=Unit Testing, O=Mavericks, L=SSL Config Base 1, ST=Cyberspace, C=CY"
+    val keyPassword: Array[Char] = EMPTY_PASSWORD
+  }


The above two objects keep the certificate-specific constants. Below are the Keystore-specific constants. Future refactors could extract this data structures into case classes, etc... and extract duplicate code from FakeKeyStore+FakeChainedKeyStore.

ignasi35 · 2018-10-05T12:16:25Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeChainedKeyStore.scala

+    val KeystoreType = "PKCS12"
+    val SignatureAlgorithmOID: ObjectIdentifier = AlgorithmId.sha256WithRSAEncryption_oid
+    val keystorePassword: Array[Char] = EMPTY_PASSWORD
+  }


This set of constants is duplicate in FakeKeyStore.

Can we make them reference each other? Or even remove the duplication at all?

ignasi35 · 2018-10-05T12:18:51Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeChainedKeyStore.scala

+    certInfo.set(CertificateAlgorithmId.NAME + "." + CertificateAlgorithmId.ALGORITHM, actualAlgorithm)
+    val newCert = new X509CertImpl(certInfo)
+    newCert.sign(keyPair.getPrivate, KeystoreSettings.SignatureAlgorithmName)
+    newCert


The two methods above have a lot of duplication amongst themselves and with some other code in FakeKeyStore

ignasi35 · 2018-10-05T12:21:02Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeKeyStore.scala

+    val KeyPairKeyLength = 2048 // 2048 is the NIST acceptable key length until 2030
+    val KeystoreType = "PKCS12"
+    val SignatureAlgorithmOID: ObjectIdentifier = AlgorithmId.sha256WithRSAEncryption_oid
+    val keystorePassword: Array[Char] = EMPTY_PASSWORD


Some cleanup.

ignasi35 · 2018-10-05T12:25:03Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeKeyStore.scala

+    val SignatureAlgorithmName = "SHA256withRSA"
+    val KeyPairAlgorithmName = "RSA"
+    val KeyPairKeyLength = 2048 // 2048 is the NIST acceptable key length until 2030
+    val KeystoreType = "PKCS12"


KeyStore's no longer default to JKS. Instead the default is PKCS12

ignasi35 · 2018-10-05T12:32:34Z

With this change the naming conventions change slightly:

FakeKeyStore generates selfsigned.keystore. Contains selfsigned-*"
FakeChainedKeyStore generates chained.keystore Contains {ca,user}-*"

marcospereira

Some questions:

Can we avoid adding the duplications before merging?
How hard is to add tests for FakeChainedKeyStore?
Would it be simpler to have a parameter (chained: Boolean = false?) and handle the different behavior instead of having a new object/class?

marcospereira · 2018-10-05T13:44:53Z

ssl-config-core/src/main/scala/com/typesafe/sslconfig/ssl/FakeChainedKeyStore.scala

+    val KeystoreType = "PKCS12"
+    val SignatureAlgorithmOID: ObjectIdentifier = AlgorithmId.sha256WithRSAEncryption_oid
+    val keystorePassword: Array[Char] = EMPTY_PASSWORD
+  }


Can we make them reference each other? Or even remove the duplication at all?

ignasi35 · 2018-10-05T13:53:37Z

I added the duplication to focus on having something we can uses ASAP.

IMHO all FakeXyz tools shohhuld be moved out of ssl-config-core and into a new ssl-config-testkit.

Need to log off now, but will be back tonight to improve the code.

## Purpose Update ssl-config to version 0.3.5 ## References * lightbend/ssl-config#125 * lagom/lagom#1610

wsargent · 2018-11-01T21:33:09Z

Note: we could create a collection of tools to create several KeyStore's (one for the CA, another for the server, a third one for client-side SSL auth) but that's not the purpose of this PR.

https://github.com/tersesystems/securitybuilder#x509certificatecreator :-)

FakeKS restorred to self-signed. Add FakeChainedKS

857dd41

ignasi35 requested review from dwijnand and marcospereira October 5, 2018 12:14

dwijnand approved these changes Oct 5, 2018

View reviewed changes

ignasi35 commented Oct 5, 2018

View reviewed changes

ignasi35 added status:backlog review and removed status:backlog labels Oct 5, 2018

marcospereira reviewed Oct 5, 2018

View reviewed changes

dwijnand merged commit d271f76 into lightbend:master Oct 5, 2018

ignasi35 mentioned this pull request Oct 5, 2018

Cleanup FakeXyz code #126

Open

marcospereira mentioned this pull request Oct 5, 2018

Update ssl-config to version 0.3.5 playframework/playframework#8677

Merged

ignasi35 pushed a commit to playframework/playframework that referenced this pull request Oct 8, 2018

Update ssl-config to version 0.3.5 (#8677)

af319e0

## Purpose Update ssl-config to version 0.3.5 ## References * lightbend/ssl-config#125 * lagom/lagom#1610

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FakeKS restored to self-signed. Add FakeChainedKS. #125

FakeKS restored to self-signed. Add FakeChainedKS. #125

ignasi35 commented Oct 5, 2018 •

edited

Loading

dwijnand left a comment

dwijnand Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 left a comment

ignasi35 Oct 5, 2018

ignasi35 Oct 5, 2018

marcospereira Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 Oct 5, 2018

ignasi35 commented Oct 5, 2018 •

edited

Loading

marcospereira left a comment

marcospereira Oct 5, 2018

ignasi35 commented Oct 5, 2018

wsargent commented Nov 1, 2018

FakeKS restored to self-signed. Add FakeChainedKS. #125

FakeKS restored to self-signed. Add FakeChainedKS. #125

Conversation

ignasi35 commented Oct 5, 2018 • edited Loading

dwijnand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ignasi35 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ignasi35 commented Oct 5, 2018 • edited Loading

marcospereira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ignasi35 commented Oct 5, 2018

wsargent commented Nov 1, 2018

ignasi35 commented Oct 5, 2018 •

edited

Loading

ignasi35 commented Oct 5, 2018 •

edited

Loading