Merge pull request #7 from vapor/tn-ecdsa-size-fix

Fix ECDSA signature size
vapor · Feb 21, 2020 · 77553e6 · 77553e6
2 parents 1183373 + e5e0794
commit 77553e6
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 20 deletions.
diff --git a/Sources/JWTKit/Signing/ECDSA/ECDSAKey.swift b/Sources/JWTKit/Signing/ECDSA/ECDSAKey.swift
@@ -1,8 +1,26 @@
 import CJWTKitBoringSSL
 
 public final class ECDSAKey: OpenSSLKey {
-    public static func generate() throws -> ECDSAKey {
-        guard let c = CJWTKitBoringSSL_EC_KEY_new_by_curve_name(NID_X9_62_prime256v1) else {
+
+    public enum Curve {
+        case p256
+        case p384
+        case p521
+
+        var cName: Int32 {
+            switch self {
+            case .p256:
+                return NID_X9_62_prime256v1
+            case .p384:
+                return NID_secp384r1
+            case .p521:
+                return NID_secp521r1
+            }
+        }
+    }
+
+    public static func generate(curve: Curve = .p521) throws -> ECDSAKey {
+        guard let c = CJWTKitBoringSSL_EC_KEY_new_by_curve_name(curve.cName) else {
             throw JWTError.signingAlgorithmFailure(ECDSAError.newKeyByCurveFailure)
         }
         guard CJWTKitBoringSSL_EC_KEY_generate_key(c) != 0 else {

diff --git a/Sources/JWTKit/Signing/ECDSA/ECDSASigner.swift b/Sources/JWTKit/Signing/ECDSA/ECDSASigner.swift
@@ -4,6 +4,18 @@ internal struct ECDSASigner: JWTAlgorithm, OpenSSLSigner {
     let key: ECDSAKey
     let algorithm: OpaquePointer
     let name: String
+
+    var curveResultSize: Int {
+        let curveName = CJWTKitBoringSSL_EC_GROUP_get_curve_name(CJWTKitBoringSSL_EC_KEY_get0_group(key.c))
+        switch curveName {
+        case NID_X9_62_prime256v1, NID_secp384r1:
+            return 32
+        case NID_secp521r1:
+            return 66
+        default:
+            fatalError("Unsupported ECDSA key curve: \(curveName)")
+        }
+    }
 
     func sign<Plaintext>(_ plaintext: Plaintext) throws -> [UInt8]
         where Plaintext: DataProtocol
@@ -20,19 +32,16 @@ internal struct ECDSASigner: JWTAlgorithm, OpenSSLSigner {
 
         // serialize r+s values
         // see: https://tools.ietf.org/html/rfc7515#appendix-A.3
-        var rBytes = [UInt8](repeating: 0, count: 32)
-        var sBytes = [UInt8](repeating: 0, count: 32)
-        let rCount = Int(CJWTKitBoringSSL_BN_bn2bin(CJWTKitBoringSSL_ECDSA_SIG_get0_r(signature), &rBytes))
-        let sCount = Int(CJWTKitBoringSSL_BN_bn2bin(CJWTKitBoringSSL_ECDSA_SIG_get0_s(signature), &sBytes))
-
-        // BN_bn2bin can return < 32 bytes which will result in the data
-        // being zero-padded on the wrong side
-        return .init(
-            [UInt8](repeating: 0, count: 32 - rCount) +
-            rBytes[0..<rCount] +
-            [UInt8](repeating: 0, count: 32 - sCount) +
-            sBytes[0..<sCount]
-        )
+        let r = CJWTKitBoringSSL_ECDSA_SIG_get0_r(signature)
+        let s = CJWTKitBoringSSL_ECDSA_SIG_get0_s(signature)
+        let rsSize = self.curveResultSize
+        var rBytes = [UInt8](repeating: 0, count: rsSize)
+        var sBytes = [UInt8](repeating: 0, count: rsSize)
+        let rCount = Int(CJWTKitBoringSSL_BN_bn2bin(r, &rBytes))
+        let sCount = Int(CJWTKitBoringSSL_BN_bn2bin(s, &sBytes))
+        // zero-padding may be on wrong side after write
+        return rBytes.prefix(rCount).zeroPrefixed(upTo: rsSize)
+            + sBytes.prefix(sCount).zeroPrefixed(upTo: rsSize)
     }
 
     func verify<Signature, Plaintext>(
@@ -46,21 +55,22 @@ internal struct ECDSASigner: JWTAlgorithm, OpenSSLSigner {
         // parse r+s values
         // see: https://tools.ietf.org/html/rfc7515#appendix-A.3
         let signatureBytes = signature.copyBytes()
-        guard signatureBytes.count == 64 else {
+        let rsSize = self.curveResultSize
+        guard signatureBytes.count == rsSize * 2 else {
             return false
         }
 
         let signature = CJWTKitBoringSSL_ECDSA_SIG_new()
         defer { CJWTKitBoringSSL_ECDSA_SIG_free(signature) }
 
-        try signatureBytes[0..<32].withUnsafeBufferPointer { r in
-            try signatureBytes[32..<64].withUnsafeBufferPointer { s in
+        try signatureBytes.prefix(rsSize).withUnsafeBufferPointer { r in
+            try signatureBytes.suffix(rsSize).withUnsafeBufferPointer { s in
                 // passing bignums to this method transfers ownership
                 // (they will be freed when the signature is freed)
                 guard CJWTKitBoringSSL_ECDSA_SIG_set0(
                     signature,
-                    CJWTKitBoringSSL_BN_bin2bn(r.baseAddress, 32, nil),
-                    CJWTKitBoringSSL_BN_bin2bn(s.baseAddress, 32, nil)
+                    CJWTKitBoringSSL_BN_bin2bn(r.baseAddress, rsSize, nil),
+                    CJWTKitBoringSSL_BN_bin2bn(s.baseAddress, rsSize, nil)
                 ) == 1 else {
                     throw JWTError.signingAlgorithmFailure(ECDSAError.signFailure)
                 }
@@ -75,3 +85,13 @@ internal struct ECDSASigner: JWTAlgorithm, OpenSSLSigner {
         ) == 1
     }
 }
+
+private extension Collection where Element == UInt8 {
+    func zeroPrefixed(upTo count: Int) -> [UInt8] {
+        if self.count < count {
+            return [UInt8](repeating: 0, count: count - self.count) + self
+        } else {
+            return .init(self)
+        }
+    }
+}