From 78eeff035f41b6262f0bd9ae3b57f01fe8346b72 Mon Sep 17 00:00:00 2001 From: Nobushige Asahi Date: Thu, 20 Feb 2020 15:37:12 +0900 Subject: [PATCH 1/4] bugfix: fix too complicated expression for the compiler --- WebAuthnKit/Sources/Format/Bytes.swift | 7 ++++++- WebAuthnKit/Sources/Format/CBOR.swift | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/WebAuthnKit/Sources/Format/Bytes.swift b/WebAuthnKit/Sources/Format/Bytes.swift index dffcfb4..9c6bf4d 100644 --- a/WebAuthnKit/Sources/Format/Bytes.swift +++ b/WebAuthnKit/Sources/Format/Bytes.swift @@ -39,7 +39,12 @@ public class Bytes { while b.count > 8 { b.removeFirst() } - let result1 = UInt64((UInt64(b[0]) << 56) | (UInt64(b[1]) << 48) | (UInt64(b[2]) << 40) | (UInt64(b[3]) << 32)) + let A = (UInt64(b[0]) << 56) + let B = (UInt64(b[1]) << 48) + let C = (UInt64(b[2]) << 40) + let D = (UInt64(b[3]) << 32) + let result1 = UInt64( A | B | C | D ) + let result2 = UInt64((UInt64(b[4]) << 24) | (UInt64(b[5]) << 16) | (UInt64(b[6]) << 8) | UInt64(b[7])) return result1 | result2 } diff --git a/WebAuthnKit/Sources/Format/CBOR.swift b/WebAuthnKit/Sources/Format/CBOR.swift index df84f95..e381634 100644 --- a/WebAuthnKit/Sources/Format/CBOR.swift +++ b/WebAuthnKit/Sources/Format/CBOR.swift @@ -504,7 +504,11 @@ internal class CBORReader { case 4: result = Int64((UInt32(b2[0]) << 24) | (UInt32(b2[1]) << 16) | (UInt32(b2[2]) << 8) | UInt32(b2[3])) case 8: - let result1 = Int64((UInt64(b2[0]) << 56) | (UInt64(b2[1]) << 48) | (UInt64(b2[2]) << 40) | (UInt64(b2[3]) << 32)) + let A = (UInt64(b2[0]) << 56) + let B = (UInt64(b2[1]) << 48) + let C = (UInt64(b2[2]) << 40) + let D = (UInt64(b2[3]) << 32) + let result1 = Int64( A | B | C | D ) let result2 = Int64((UInt64(b2[4]) << 24) | (UInt64(b2[5]) << 16) | (UInt64(b2[6]) << 8) | UInt64(b2[7])) result = result1 | result2 default: From cca688ba573818ac02653ff22ca904826766b397 Mon Sep 17 00:00:00 2001 From: Nobushige Asahi Date: Thu, 20 Feb 2020 16:03:07 +0900 Subject: [PATCH 2/4] bugfix: fix for Authr-MakeCred-Resp-1 Test in ConformanceTool --- WebAuthnKit/Sources/Authenticator/Attestation.swift | 10 +++++----- WebAuthnKit/Sources/Format/CBOR.swift | 4 ++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/WebAuthnKit/Sources/Authenticator/Attestation.swift b/WebAuthnKit/Sources/Authenticator/Attestation.swift index e48f2c6..04d97dc 100644 --- a/WebAuthnKit/Sources/Authenticator/Attestation.swift +++ b/WebAuthnKit/Sources/Authenticator/Attestation.swift @@ -54,13 +54,13 @@ public class AttestationObject { public func toBytes() -> Optional<[UInt8]> { - let dict = SimpleOrderedDictionary() - dict.addBytes("authData", self.authData.toBytes()) - dict.addString("fmt", "packed") - dict.addStringKeyMap("attStmt", self.attStmt) + let dict = SimpleOrderedDictionary() + dict.addString(0x01, "packed") + dict.addBytes(0x02, self.authData.toBytes()) + dict.addStringKeyMap(0x03, self.attStmt) return CBORWriter() - .putStringKeyMap(dict) + .putIntKeyMap(dict) .getResult() } diff --git a/WebAuthnKit/Sources/Format/CBOR.swift b/WebAuthnKit/Sources/Format/CBOR.swift index e381634..f5bc769 100644 --- a/WebAuthnKit/Sources/Format/CBOR.swift +++ b/WebAuthnKit/Sources/Format/CBOR.swift @@ -607,6 +607,10 @@ internal class CBORWriter { _ = self.putDouble(value as! Double) } else if value is Bool { _ = self.putBool(value as! Bool) + } else if value is SimpleOrderedDictionary { + _ = self.putStringKeyMap(value as! SimpleOrderedDictionary) + } else if value is [Any] { + _ = self.putArray(value as! [Any]) } else { fatalError("unsupported value type \(value)") } From ae9880a25c55a8c8b5720dc3c813ac05c6b514a2 Mon Sep 17 00:00:00 2001 From: Nobushige Asahi Date: Fri, 21 Feb 2020 16:28:14 +0900 Subject: [PATCH 3/4] bugfix: Change the public key for making attestation to one in authData. because it's defined in self attestation spec. --- .../Authenticator/Internal/KeySupport.swift | 22 ++++++++++++++----- .../Internal/SelfAttestation.swift | 11 ++-------- ...rnalAuthenticatorGetAssertionSession.swift | 5 +++-- ...alAuthenticatorMakeCredentialSession.swift | 7 +++--- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/WebAuthnKit/Sources/Authenticator/Internal/KeySupport.swift b/WebAuthnKit/Sources/Authenticator/Internal/KeySupport.swift index 6a43f62..047b871 100644 --- a/WebAuthnKit/Sources/Authenticator/Internal/KeySupport.swift +++ b/WebAuthnKit/Sources/Authenticator/Internal/KeySupport.swift @@ -12,8 +12,9 @@ import EllipticCurveKeyPair public protocol KeySupport { var selectedAlg: COSEAlgorithmIdentifier { get } - func createKeyPair(label: String) -> Optional - func sign(data: [UInt8], label: String) -> Optional<[UInt8]> + func createKeyPair() -> Optional + func sign(data: [UInt8]) -> Optional<[UInt8]> + func setup(label: String) } public class KeySupportChooser { @@ -38,9 +39,14 @@ public class KeySupportChooser { } } +enum ECDSAKeySupportError : Error { + case noKeyPair +} + public class ECDSAKeySupport : KeySupport { public let selectedAlg: COSEAlgorithmIdentifier + public var pair: EllipticCurveKeyPair.Manager? init(alg: COSEAlgorithmIdentifier) { self.selectedAlg = alg @@ -66,9 +72,13 @@ public class ECDSAKeySupport : KeySupport { return EllipticCurveKeyPair.Manager(config: config) } - public func sign(data: [UInt8], label: String) -> Optional<[UInt8]> { + public func setup(label: String) { + self.pair = self.createPair(label: label) + } + + public func sign(data: [UInt8]) -> Optional<[UInt8]> { do { - let pair = self.createPair(label: label) + guard let pair = self.pair else { throw ECDSAKeySupportError.noKeyPair } let signature = try pair.sign(Data(bytes: data), hash: .sha256) return signature.bytes } catch let error { @@ -77,10 +87,10 @@ public class ECDSAKeySupport : KeySupport { } } - public func createKeyPair(label: String) -> Optional { + public func createKeyPair() -> Optional { WAKLogger.debug(" createKeyPair") do { - let pair = self.createPair(label: label) + guard let pair = self.pair else { throw ECDSAKeySupportError.noKeyPair } let publicKey = try pair.publicKey().data().DER.bytes if publicKey.count != 91 { WAKLogger.debug(" length of pubKey should be 91: \(publicKey.count)") diff --git a/WebAuthnKit/Sources/Authenticator/Internal/SelfAttestation.swift b/WebAuthnKit/Sources/Authenticator/Internal/SelfAttestation.swift index f416fd4..3622585 100644 --- a/WebAuthnKit/Sources/Authenticator/Internal/SelfAttestation.swift +++ b/WebAuthnKit/Sources/Authenticator/Internal/SelfAttestation.swift @@ -14,7 +14,7 @@ public class SelfAttestation { authData: AuthenticatorData, clientDataHash: [UInt8], alg: COSEAlgorithmIdentifier, - keyLabel: String + keySupport: KeySupport ) -> Optional { WAKLogger.debug(" create") @@ -22,15 +22,8 @@ public class SelfAttestation { var dataToBeSigned = authData.toBytes() dataToBeSigned.append(contentsOf: clientDataHash) - guard let keySupport = - KeySupportChooser().choose([alg]) else { - WAKLogger.debug(" key-support not found") - return nil - } - guard let sig = keySupport.sign( - data: dataToBeSigned, - label: keyLabel + data: dataToBeSigned ) else { WAKLogger.debug(" failed to sign") return nil diff --git a/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorGetAssertionSession.swift b/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorGetAssertionSession.swift index 47bd413..59774f2 100644 --- a/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorGetAssertionSession.swift +++ b/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorGetAssertionSession.swift @@ -172,8 +172,9 @@ public class InternalAuthenticatorGetAssertionSession : AuthenticatorGetAssertio self.stop(by: .unsupported) return } - - guard let signature = keySupport.sign(data: data, label: cred.keyLabel) else { + + keySupport.setup(label: cred.keyLabel) + guard let signature = keySupport.sign(data: data) else { self.stop(by: .unknown) return } diff --git a/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorMakeCredentialSession.swift b/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorMakeCredentialSession.swift index acd54c8..9c4974e 100644 --- a/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorMakeCredentialSession.swift +++ b/WebAuthnKit/Sources/Authenticator/Internal/Session/InternalAuthenticatorMakeCredentialSession.swift @@ -186,8 +186,9 @@ public class InternalAuthenticatorMakeCredentialSession : AuthenticatorMakeCrede ) // TODO should remove fron KeyPair too? - - guard let publicKeyCOSE = keySupport.createKeyPair(label: credSource.keyLabel) else { + keySupport.setup(label: credSource.keyLabel) + + guard let publicKeyCOSE = keySupport.createKeyPair() else { self.stop(by: .unknown) return } @@ -223,7 +224,7 @@ public class InternalAuthenticatorMakeCredentialSession : AuthenticatorMakeCrede authData: authenticatorData, clientDataHash: hash, alg: keySupport.selectedAlg, - keyLabel: credSource.keyLabel + keySupport: keySupport ) else { WAKLogger.debug(" failed to build attestation object") self.stop(by: .unknown) From 8301b1306cc2157510cf1698ed2c6efacabe9c20 Mon Sep 17 00:00:00 2001 From: Nobushige Asahi Date: Fri, 21 Feb 2020 16:30:30 +0900 Subject: [PATCH 4/4] bugfix: Change source of client hash. According to spec, challenge in request is client data hash. --- .../Sources/Client/Operation/ClientCreateOperation.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WebAuthnKit/Sources/Client/Operation/ClientCreateOperation.swift b/WebAuthnKit/Sources/Client/Operation/ClientCreateOperation.swift index 3f9f509..a4a81e7 100644 --- a/WebAuthnKit/Sources/Client/Operation/ClientCreateOperation.swift +++ b/WebAuthnKit/Sources/Client/Operation/ClientCreateOperation.swift @@ -253,7 +253,7 @@ public class ClientCreateOperation: AuthenticatorMakeCredentialSessionDelegate { ) session.makeCredential( - hash: self.clientDataHash, + hash: self.options.challenge, rpEntity: rpEntity, userEntity: options.user, requireResidentKey: requireResidentKey,