-
Notifications
You must be signed in to change notification settings - Fork 9.3k
tls: add Ed25519 certificate support #9452
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ import okhttp3.tls.internal.der.CertificateAdapters.generalNameIpAddress | |
| import okhttp3.tls.internal.der.Extension | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers.BASIC_CONSTRAINTS | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers.ED25519 | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers.ORGANIZATIONAL_UNIT_NAME | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers.SHA256_WITH_ECDSA | ||
| import okhttp3.tls.internal.der.ObjectIdentifiers.SHA256_WITH_RSA_ENCRYPTION | ||
|
|
@@ -341,6 +342,16 @@ class HeldCertificate( | |
| keySize = 2048 | ||
| } | ||
|
|
||
| /** | ||
| * Configure the certificate to generate an Ed25519 key. Ed25519 provides about 128 bits of | ||
| * security and fast signing/verification. Requires Java 15 or newer at runtime. | ||
| */ | ||
| fun ed25519() = | ||
| apply { | ||
| keyAlgorithm = "Ed25519" | ||
| keySize = 0 | ||
| } | ||
|
|
||
| fun build(): HeldCertificate { | ||
| // Subject keys & identity. | ||
| val subjectKeyPair = keyPair ?: generateKeyPair() | ||
|
|
@@ -480,14 +491,21 @@ class HeldCertificate( | |
| } | ||
|
|
||
| private fun signatureAlgorithm(signedByKeyPair: KeyPair): AlgorithmIdentifier = | ||
| when (signedByKeyPair.private) { | ||
| is RSAPrivateKey -> { | ||
| when { | ||
| signedByKeyPair.private is RSAPrivateKey -> { | ||
| AlgorithmIdentifier( | ||
| algorithm = SHA256_WITH_RSA_ENCRYPTION, | ||
| parameters = null, | ||
| ) | ||
| } | ||
|
|
||
| signedByKeyPair.private.algorithm == "Ed25519" || signedByKeyPair.private.algorithm == "EdDSA" -> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Is this check complete? or are there sub algorithms? |
||
| AlgorithmIdentifier( | ||
| algorithm = ED25519, | ||
| parameters = null, | ||
| ) | ||
| } | ||
|
|
||
| else -> { | ||
| AlgorithmIdentifier( | ||
| algorithm = SHA256_WITH_ECDSA, | ||
|
|
@@ -498,7 +516,7 @@ class HeldCertificate( | |
|
|
||
| private fun generateKeyPair(): KeyPair = | ||
| KeyPairGenerator.getInstance(keyAlgorithm).run { | ||
| initialize(keySize, SecureRandom()) | ||
| if (keySize > 0) initialize(keySize, SecureRandom()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably worth a comment since this affects other code paths. |
||
| generateKeyPair() | ||
| } | ||
|
|
||
|
|
@@ -581,9 +599,10 @@ class HeldCertificate( | |
|
|
||
| // The private key doesn't tell us its type but it's okay because the certificate knows! | ||
| val keyType = | ||
| when (certificate.publicKey) { | ||
| is ECPublicKey -> "EC" | ||
| is RSAPublicKey -> "RSA" | ||
| when { | ||
| certificate.publicKey is ECPublicKey -> "EC" | ||
| certificate.publicKey is RSAPublicKey -> "RSA" | ||
| certificate.publicKey.algorithm == "Ed25519" || certificate.publicKey.algorithm == "EdDSA" -> certificate.publicKey.algorithm | ||
| else -> throw IllegalArgumentException("unexpected key type: ${certificate.publicKey}") | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import assertk.assertThat | |
| import assertk.assertions.containsExactly | ||
| import assertk.assertions.isCloseTo | ||
| import assertk.assertions.isEqualTo | ||
| import assertk.assertions.isIn | ||
| import assertk.assertions.isNull | ||
| import assertk.assertions.matches | ||
| import java.math.BigInteger | ||
|
|
@@ -262,6 +263,56 @@ class HeldCertificateTest { | |
| assertThat(leaf.certificate.sigAlgName).isEqualTo("SHA256WITHECDSA", ignoreCase = true) | ||
| } | ||
|
|
||
| @Test | ||
| fun ed25519() { | ||
| platform.assumeNotAndroid() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is just in the tests, but if called by an app, we should have a useful error message. Something like Ed25519 requires JDK 15+ / a provider that supports it |
||
| val heldCertificate = | ||
| HeldCertificate | ||
| .Builder() | ||
| .commonName("cash.app") | ||
| .ed25519() | ||
| .build() | ||
| assertThat(heldCertificate.certificate.sigAlgName.lowercase()).isIn("ed25519", "eddsa") | ||
| assertThat(heldCertificate.keyPair.private.algorithm.lowercase()).isIn("ed25519", "eddsa") | ||
| assertThat(heldCertificate.keyPair.public.algorithm.lowercase()).isIn("ed25519", "eddsa") | ||
| } | ||
|
|
||
| @Test | ||
| fun ed25519RoundTrip() { | ||
| platform.assumeNotAndroid() | ||
| val original = | ||
| HeldCertificate | ||
| .Builder() | ||
| .commonName("cash.app") | ||
| .ed25519() | ||
| .build() | ||
| val pem = original.certificatePem() + original.privateKeyPkcs8Pem() | ||
| val decoded = decode(pem) | ||
| assertThat(decoded.certificate.encoded).isEqualTo(original.certificate.encoded) | ||
| assertThat(decoded.keyPair.private.encoded).isEqualTo(original.keyPair.private.encoded) | ||
| assertThat(decoded.keyPair.public.encoded).isEqualTo(original.keyPair.public.encoded) | ||
| } | ||
|
|
||
| @Test | ||
| fun ed25519SignedByEcdsa() { | ||
| platform.assumeNotAndroid() | ||
| val root = | ||
| HeldCertificate | ||
| .Builder() | ||
| .certificateAuthority(0) | ||
| .ecdsa256() | ||
| .build() | ||
| val leaf = | ||
| HeldCertificate | ||
| .Builder() | ||
| .ed25519() | ||
| .signedBy(root) | ||
| .build() | ||
| assertThat(root.certificate.sigAlgName).isEqualTo("SHA256WITHECDSA", ignoreCase = true) | ||
| assertThat(leaf.certificate.sigAlgName).isEqualTo("SHA256WITHECDSA", ignoreCase = true) | ||
| assertThat(leaf.keyPair.private.algorithm.lowercase()).isIn("ed25519", "eddsa") | ||
| } | ||
|
|
||
| @Test | ||
| fun decodeEcdsa256() { | ||
| // The certificate + private key below was generated programmatically: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given differing providers, I'm worried about case sensitivity, can you audit how it's represented in providers, and maybe make it case insensitive, if it's not guaranteed by a spec.