fix: add normalisation to signatures before verifying

beatt83 · Apr 23, 2024 · c65a47f · c65a47f
1 parent f4d3861
commit c65a47f
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 2 deletions.
diff --git a/Sources/JSONWebAlgorithms/Signatures/EC/Verifiers/ES256KVerifier.swift b/Sources/JSONWebAlgorithms/Signatures/EC/Verifiers/ES256KVerifier.swift
@@ -30,7 +30,8 @@ public struct ES256KVerifier: Verifier {
         else { throw CryptoError.notValidPublicKey }
         let publicKey = try secp256k1.Signing.PublicKey(dataRepresentation: [0x04] + x + y, format: .uncompressed)
         let hash = SHA256.hash(data: data)
-        guard try publicKey.isValidSignature(getSignature(signature), for: hash) else {
+        let objSignature = try getSignature(signature).normalize
+        guard publicKey.isValidSignature(objSignature, for: hash) else {
             guard ES256KVerifier.bouncyCastleFailSafe else {
                 return false
             }
@@ -44,7 +45,7 @@ public struct ES256KVerifier: Verifier {
     private func transcodeBCSignatureToBitcoin(signature: Data) throws -> secp256k1.Signing.ECDSASignature {
         let signature = try getSignature(signature)
         let signatureInvertedRS = invertR_S(signatureData: signature.dataRepresentation)
-        return try .init(dataRepresentation: signatureInvertedRS)
+        return try .init(dataRepresentation: signatureInvertedRS).normalize
     }
 }
 
@@ -59,3 +60,26 @@ private func getSignature(_ data: Data) throws -> secp256k1.Signing.ECDSASignatu
         throw CryptoError.invalidSignature
     }
 }
+
+private extension secp256k1.Signing.ECDSASignature {
+    /// Convert a signature into a normal signature.
+    var normalize: secp256k1.Signing.ECDSASignature {
+        get throws {
+            let context = secp256k1.Context.rawRepresentation
+            var signature = secp256k1_ecdsa_signature()
+            var resultSignature = secp256k1_ecdsa_signature()
+
+            dataRepresentation.copyToUnsafeMutableBytes(of: &signature.data)
+
+            guard secp256k1_ecdsa_signature_normalize(
+                context,
+                &resultSignature,
+                &signature
+            ) != 0 else {
+                return self
+            }
+
+            return try secp256k1.Signing.ECDSASignature(dataRepresentation: resultSignature.dataValue)
+        }
+    }
+}
diff --git a/Tests/JWATests/ES256KTests.swift b/Tests/JWATests/ES256KTests.swift
@@ -156,4 +156,21 @@ final class Secp256k1Tests: XCTestCase {
             key: publicKey.jwkRepresentation
         ))
     }
+
+    func testNotNormalizedBouncyCastleSignatureValidation() async throws {
+        let payload = try "eyJhbGciOiJFUzI1NksifQ.eyJpc3MiOiJkaWQ6cHJpc206MGYzN2Y2YmRmZWNlMzQzNzJmMzE3YmM5NDQyNzY5YzI0Yzk1ZmNkYjQzZDAzOTNiOTZiOGQ3YWEwODBlZDBiNzpDcmtCQ3JZQkVqb0tCbUYxZEdndE1SQUVTaTRLQ1hObFkzQXlOVFpyTVJJaEF2TWkxYUZZaTdOUlFWQ00zU2s2TjBvQkhIOXlabUhUcGtOT0tyd1QzYWdVRWpzS0IybHpjM1ZsTFRFUUFrb3VDZ2x6WldOd01qVTJhekVTSVFLSkp2UkloYjZLWG9iTnhWQnhaS2ZyMkdCcnNsc0lUb1doSVFCUDEyMS1yaEk3Q2dkdFlYTjBaWEl3RUFGS0xnb0pjMlZqY0RJMU5tc3hFaUVDRXI2QkJMbFVEcjFMcHdJZ1JLcHZZQ1BYUkRfWFh5SFpTbWdVTXMxZlpVMCIsInN1YiI6ImRpZDpwcmlzbTpiYWQyNDgzNDA2NjU5MmIwMDgwYjlmMWYyZjkxZjA1ZmNiZDU0MzQxMDlkZTIwMTM3YmRiOWE0ZWE4N2E4NjQ4OkN0OEJDdHdCRW5RS0gyRjFkR2hsYm5ScFkyRjBhVzl1WVhWMGFHVnVkR2xqWVhScGIyNUxaWGtRQkVKUENnbHpaV053TWpVMmF6RVNJSDNWNzdRb3dXWWZpN1BHdFZTSjBIZVdjcVlwdmxzanNydUN1ZUhITDByZUdpRHl4OWVBN0lQdVgyLUExMkxHZktFczJZTGJoUm1ILUdWbzFNUndZdUYta0JKa0NnOXRZWE4wWlhKdFlYTjBaWEpMWlhrUUFVSlBDZ2x6WldOd01qVTJhekVTSUgzVjc3UW93V1lmaTdQR3RWU0owSGVXY3FZcHZsc2pzcnVDdWVISEwwcmVHaUR5eDllQTdJUHVYMi1BMTJMR2ZLRXMyWUxiaFJtSC1HVm8xTVJ3WXVGLWtBIiwibmJmIjoxNzEzODYyMjk4LCJ2YyI6eyJjcmVkZW50aWFsU3ViamVjdCI6eyJlbWFpbEFkZHJlc3MiOiJjb3Jwb3JhdGVAZG9tYWluLmNvbSIsImRyaXZpbmdDbGFzcyI6MSwiZHJpdmluZ0xpY2Vuc2VJRCI6IkVTLTEyMzQ1Njc4OTAiLCJpZCI6ImRpZDpwcmlzbTpiYWQyNDgzNDA2NjU5MmIwMDgwYjlmMWYyZjkxZjA1ZmNiZDU0MzQxMDlkZTIwMTM3YmRiOWE0ZWE4N2E4NjQ4OkN0OEJDdHdCRW5RS0gyRjFkR2hsYm5ScFkyRjBhVzl1WVhWMGFHVnVkR2xqWVhScGIyNUxaWGtRQkVKUENnbHpaV053TWpVMmF6RVNJSDNWNzdRb3dXWWZpN1BHdFZTSjBIZVdjcVlwdmxzanNydUN1ZUhITDByZUdpRHl4OWVBN0lQdVgyLUExMkxHZktFczJZTGJoUm1ILUdWbzFNUndZdUYta0JKa0NnOXRZWE4wWlhKdFlYTjBaWEpMWlhrUUFVSlBDZ2x6WldOd01qVTJhekVTSUgzVjc3UW93V1lmaTdQR3RWU0owSGVXY3FZcHZsc2pzcnVDdWVISEwwcmVHaUR5eDllQTdJUHVYMi1BMTJMR2ZLRXMyWUxiaFJtSC1HVm8xTVJ3WXVGLWtBIiwiZGF0ZU9mSXNzdWFuY2UiOiIyMDIzLTAxLTAxVDAyOjAyOjAyWiJ9LCJ0eXBlIjpbIlZlcmlmaWFibGVDcmVkZW50aWFsIl0sIkBjb250ZXh0IjpbImh0dHBzOlwvXC93d3cudzMub3JnXC8yMDE4XC9jcmVkZW50aWFsc1wvdjEiXSwiY3JlZGVudGlhbFN0YXR1cyI6eyJzdGF0dXNQdXJwb3NlIjoiUmV2b2NhdGlvbiIsInN0YXR1c0xpc3RJbmRleCI6NCwiaWQiOiJodHRwOlwvXC8xOTIuMTY4LjEuNDQ6ODAwMFwvcHJpc20tYWdlbnRcL2NyZWRlbnRpYWwtc3RhdHVzXC9jMDkxOGViNi1lZGIzLTRjMTUtYWM4OS0yZDk5MTZmMjFmYjUjNCIsInR5cGUiOiJTdGF0dXNMaXN0MjAyMUVudHJ5Iiwic3RhdHVzTGlzdENyZWRlbnRpYWwiOiJodHRwOlwvXC8xOTIuMTY4LjEuNDQ6ODAwMFwvcHJpc20tYWdlbnRcL2NyZWRlbnRpYWwtc3RhdHVzXC9jMDkxOGViNi1lZGIzLTRjMTUtYWM4OS0yZDk5MTZmMjFmYjUifX19".tryToData()
+
+        let publicKeyRaw = Data(base64Encoded: "BIkm9EiFvopehs3FUHFkp+vYYGuyWwhOhaEhAE/XbX6u9FcLWxguRJYRCVcuN7AHDo8wePDUVDI9UrvMSaKXbiw=")!
+        let publicKey = try secp256k1.Signing.PublicKey(dataRepresentation: publicKeyRaw, format: .uncompressed)
+        let sigantureBase64 = "gKPlzHeuO0NryO1f_iHzxKUtyyD6e_woWNoK2QsEBGuhQw-tkJpKVvBzK0F-z7bdG0nj6xWZBmlJ7ctsVexYPg"
+        let signatureRaw = try Base64URL.decode(sigantureBase64)
+
+        ES256KVerifier.bouncyCastleFailSafe = true
+
+        XCTAssertTrue(try ES256KVerifier().verify(
+            data: payload,
+            signature: signatureRaw,
+            key: publicKey.jwkRepresentation
+        ))
+    }
 }