Migrate Apple HMAC to CryptoKit #122893
Closed
Migrate Apple HMAC to CryptoKit #122893
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
vcsjones
commented
Jan 5, 2026
| let algorithm: PAL_HashAlgorithm | ||
| let length: Int | ||
| var key: SymmetricKey? | ||
| var hmac: Any? |
Member
Author
There was a problem hiding this comment.
I am not super happy with the Any? here but it's kind of needed because HMAC takes a type parameter, and we can't put a protocol in there. That is, HMAC<any HashFunction> won't work because the type cannot be existential. It must have a concrete type, like SHA256. So that would mean HmacBox would also itself need to be generic.
But that poses a problem. Imagine if HmacBox were generic. When we unbox it from p/invoke, we wouldn't know what to pass:
Unmanaged<HmacBox<???>>.fromOpaque(ctx).takeUnretainedValue()Unless we had every HMAC p/invoke boundary say "Oh BTW it's SHA-256" so we would know what to unbox it to.
So Any? would have to do.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the Apple platform HMAC implementation from C-based CommonCrypto to Swift-based CryptoKit, continuing the modernization effort started in PR #120953. The changes replace the C implementation with Swift code that leverages CryptoKit's native HMAC support for MD5, SHA-1, SHA-256, SHA-384, and SHA-512 algorithms.
Key changes:
- Complete rewrite of HMAC operations in Swift using CryptoKit's HMAC types
- Removal of C-based CommonCrypto implementation files
- Addition of Swift calling conventions to C# interop layer
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| pal_swiftbindings.swift | Adds HmacBox class and implements all HMAC operations (Create, Init, Update, Final, Current, Clone, OneShot, Free) using CryptoKit's HMAC types |
| pal_swiftbindings.h | Adds external function declarations for new Swift HMAC implementations |
| pal_hmac.h | Removed - C header no longer needed after migration to Swift |
| pal_hmac.c | Removed - C implementation replaced by Swift code |
| pal_digest.h | Removes unused DigestCtx typedef |
| entrypoints.c | Removes pal_hmac.h include as it's no longer needed |
| CMakeLists.txt | Removes pal_hmac.c from build sources |
| Interop.Hmac.cs | Adds UnmanagedCallConv attributes with CallConvSwift to all HMAC P/Invoke methods for Swift interop |
Comment on lines
+589
to
+593
| guard let ctx, let pbKey, cbKey >= 0 else { | ||
| return 0 | ||
| } | ||
|
|
||
| let keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) |
There was a problem hiding this comment.
The validation logic in HmacInit does not allow for a zero-length key, but this is a valid scenario in HMAC. When cbKey is 0, pbKey can be NULL, but the current guard statement rejects this case. The old C implementation explicitly handled this by checking if (cbKey != 0 && pbKey == NULL), which allows pbKey to be NULL when cbKey is 0. This change breaks API compatibility for zero-length keys.
Suggested change
| guard let ctx, let pbKey, cbKey >= 0 else { | |
| return 0 | |
| } | |
| let keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) | |
| guard let ctx, cbKey >= 0, cbKey == 0 || pbKey != nil else { | |
| return 0 | |
| } | |
| let keyData: Data | |
| if cbKey == 0 { | |
| keyData = Data() | |
| } else { | |
| keyData = Data(bytesNoCopy: pbKey!, count: Int(cbKey), deallocator: .none) | |
| } |
Comment on lines
+662
to
+678
| break | ||
| case .sha1: | ||
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha256: | ||
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha384: | ||
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha512: | ||
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break |
There was a problem hiding this comment.
The HmacFinal function unnecessarily breaks after each case in the switch statement. In Swift, switch cases don't fall through by default, so the break statements are redundant and should be removed for cleaner code.
Suggested change
| break | |
| case .sha1: | |
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| break | |
| case .sha256: | |
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| break | |
| case .sha384: | |
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| break | |
| case .sha512: | |
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| break | |
| case .sha1: | |
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| case .sha256: | |
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| case .sha384: | |
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | |
| case .sha512: | |
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | |
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } |
Comment on lines
+740
to
+741
| cloneBox.key = box.key | ||
| cloneBox.hmac = box.hmac |
There was a problem hiding this comment.
The cloning implementation is problematic for value-type HMAC objects. Since HMAC is a struct in Swift, the assignment cloneBox.hmac = box.hmac creates a shallow copy. However, when the original HMAC is finalized or updated, this doesn't create an independent clone. The HMAC type in CryptoKit is a struct with value semantics, but storing it as Any and then copying it may not preserve the independent state properly when both instances are later mutated through forced casts.
Comment on lines
+612
to
+637
| switch box.algorithm { | ||
| case .md5: | ||
| var hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha1: | ||
| var hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha256: | ||
| var hmac = (box.hmac as! HMAC<SHA256>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha384: | ||
| var hmac = (box.hmac as! HMAC<SHA384>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha512: | ||
| var hmac = (box.hmac as! HMAC<SHA512>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 |
There was a problem hiding this comment.
The HmacUpdate function contains significant code duplication across all five algorithm cases. Each case follows the exact same pattern: extract hmac, update it, store it back, and return 1. This should be refactored to eliminate the duplication, possibly by using a generic helper function or by restructuring how HMAC instances are stored and updated.
Suggested change
| switch box.algorithm { | |
| case .md5: | |
| var hmac = (box.hmac as! HMAC<Insecure.MD5>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| case .sha1: | |
| var hmac = (box.hmac as! HMAC<Insecure.SHA1>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| case .sha256: | |
| var hmac = (box.hmac as! HMAC<SHA256>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| case .sha384: | |
| var hmac = (box.hmac as! HMAC<SHA384>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| case .sha512: | |
| var hmac = (box.hmac as! HMAC<SHA512>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| func updateHmac<T: HashFunction>(_ type: T.Type) -> Int32 { | |
| var hmac = (box.hmac as! HMAC<T>) | |
| hmac.update(data: data) | |
| box.hmac = hmac | |
| return 1 | |
| } | |
| switch box.algorithm { | |
| case .md5: | |
| return updateHmac(Insecure.MD5.self) | |
| case .sha1: | |
| return updateHmac(Insecure.SHA1.self) | |
| case .sha256: | |
| return updateHmac(SHA256.self) | |
| case .sha384: | |
| return updateHmac(SHA384.self) | |
| case .sha512: | |
| return updateHmac(SHA512.self) |
Comment on lines
+813
to
+837
| private func HmacReset(box: HmacBox, key: SymmetricKey) -> Int32 { | ||
| switch box.algorithm { | ||
| case .md5: | ||
| box.hmac = HMAC<Insecure.MD5>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| case .sha1: | ||
| box.hmac = HMAC<Insecure.SHA1>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| case .sha256: | ||
| box.hmac = HMAC<SHA256>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| case .sha384: | ||
| box.hmac = HMAC<SHA384>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| case .sha512: | ||
| box.hmac = HMAC<SHA512>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| default: | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
The HmacReset function contains significant code duplication. Each case follows the exact same pattern with only the HMAC type varying. This same pattern appears throughout the file and should be addressed through architectural improvements.
Suggested change
| private func HmacReset(box: HmacBox, key: SymmetricKey) -> Int32 { | |
| switch box.algorithm { | |
| case .md5: | |
| box.hmac = HMAC<Insecure.MD5>(key: key) | |
| box.key = key | |
| return 1 | |
| case .sha1: | |
| box.hmac = HMAC<Insecure.SHA1>(key: key) | |
| box.key = key | |
| return 1 | |
| case .sha256: | |
| box.hmac = HMAC<SHA256>(key: key) | |
| box.key = key | |
| return 1 | |
| case .sha384: | |
| box.hmac = HMAC<SHA384>(key: key) | |
| box.key = key | |
| return 1 | |
| case .sha512: | |
| box.hmac = HMAC<SHA512>(key: key) | |
| box.key = key | |
| return 1 | |
| default: | |
| return 0 | |
| } | |
| private func createHmac(for algorithm: PAL_HashAlgorithm, key: SymmetricKey) -> Any? { | |
| switch algorithm { | |
| case .md5: | |
| return HMAC<Insecure.MD5>(key: key) | |
| case .sha1: | |
| return HMAC<Insecure.SHA1>(key: key) | |
| case .sha256: | |
| return HMAC<SHA256>(key: key) | |
| case .sha384: | |
| return HMAC<SHA384>(key: key) | |
| case .sha512: | |
| return HMAC<SHA512>(key: key) | |
| default: | |
| return nil | |
| } | |
| } | |
| private func HmacReset(box: HmacBox, key: SymmetricKey) -> Int32 { | |
| guard let hmac = createHmac(for: box.algorithm, key: key) else { | |
| return 0 | |
| } | |
| box.hmac = hmac | |
| box.key = key | |
| return 1 |
Comment on lines
+560
to
+580
| switch hashAlgorithm { | ||
| case .md5: | ||
| pcbHmac.pointee = Int32(Insecure.MD5.byteCount) | ||
| let box = HmacBox(hashAlgorithm, length: Insecure.MD5.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha1: | ||
| let box = HmacBox(hashAlgorithm, length: Insecure.SHA1.byteCount) | ||
| pcbHmac.pointee = Int32(Insecure.SHA1.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha256: | ||
| let box = HmacBox(hashAlgorithm, length: SHA256.byteCount) | ||
| pcbHmac.pointee = Int32(SHA256.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha384: | ||
| let box = HmacBox(hashAlgorithm, length: SHA384.byteCount) | ||
| pcbHmac.pointee = Int32(SHA384.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha512: | ||
| let box = HmacBox(hashAlgorithm, length: SHA512.byteCount) | ||
| pcbHmac.pointee = Int32(SHA512.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() |
There was a problem hiding this comment.
The order of operations setting pcbHmac.pointee is inconsistent across the switch cases. For MD5, the assignment happens before creating the box (line 562), while for SHA1 it happens after (line 567). For consistency and clarity, the order should be uniform across all cases.
Comment on lines
+658
to
+681
| switch box.algorithm { | ||
| case .md5: | ||
| let mac = (box.hmac as! HMAC<Insecure.MD5>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha1: | ||
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha256: | ||
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha384: | ||
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha512: | ||
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| default: | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
The HmacFinal function contains significant code duplication across all algorithm cases. Each case has identical logic except for the specific HMAC type being cast. This duplication should be eliminated through refactoring, similar to the issue in HmacUpdate.
Comment on lines
+699
to
+722
| switch box.algorithm { | ||
| case .md5: | ||
| let hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha1: | ||
| let hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha256: | ||
| let hmac = (box.hmac as! HMAC<SHA256>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha384: | ||
| let hmac = (box.hmac as! HMAC<SHA384>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha512: | ||
| let hmac = (box.hmac as! HMAC<SHA512>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| default: | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
The HmacCurrent function contains significant code duplication. Each case has identical logic except for the specific HMAC type being cast. This same pattern appears in HmacUpdate, HmacFinal, HmacReset, and HmacOneShot, indicating a systemic issue with the architecture.
Comment on lines
+561
to
+817
| case .md5: | ||
| pcbHmac.pointee = Int32(Insecure.MD5.byteCount) | ||
| let box = HmacBox(hashAlgorithm, length: Insecure.MD5.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha1: | ||
| let box = HmacBox(hashAlgorithm, length: Insecure.SHA1.byteCount) | ||
| pcbHmac.pointee = Int32(Insecure.SHA1.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha256: | ||
| let box = HmacBox(hashAlgorithm, length: SHA256.byteCount) | ||
| pcbHmac.pointee = Int32(SHA256.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha384: | ||
| let box = HmacBox(hashAlgorithm, length: SHA384.byteCount) | ||
| pcbHmac.pointee = Int32(SHA384.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha512: | ||
| let box = HmacBox(hashAlgorithm, length: SHA512.byteCount) | ||
| pcbHmac.pointee = Int32(SHA512.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| default: | ||
| pcbHmac.pointee = 0 | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacInit") | ||
| public func AppleCryptoNative_HmacInit(ctx: UnsafeMutableRawPointer?, pbKey: UnsafeMutableRawPointer?, cbKey: Int32) -> Int32 { | ||
| guard let ctx, let pbKey, cbKey >= 0 else { | ||
| return 0 | ||
| } | ||
|
|
||
| let keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) | ||
| let key = SymmetricKey(data: keyData) | ||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| return HmacReset(box: box, key: key) | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacUpdate") | ||
| public func AppleCryptoNative_HmacUpdate(ctx: UnsafeMutableRawPointer?, pbData: UnsafeMutableRawPointer?, cbData: Int32) -> Int32 { | ||
| guard let ctx, let pbData, cbData >= 0 else { | ||
| return 0 | ||
| } | ||
|
|
||
| if cbData == 0 { | ||
| return 1 | ||
| } | ||
|
|
||
| let data = Data(bytesNoCopy: pbData, count: Int(cbData), deallocator: .none) | ||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| var hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha1: | ||
| var hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha256: | ||
| var hmac = (box.hmac as! HMAC<SHA256>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha384: | ||
| var hmac = (box.hmac as! HMAC<SHA384>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha512: | ||
| var hmac = (box.hmac as! HMAC<SHA512>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| default: | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacFinal") | ||
| public func AppleCryptoNative_HmacFinal(ctx: UnsafeMutableRawPointer?, pOutput: UnsafeMutablePointer<UInt8>?) -> Int32 { | ||
| guard let ctx, let pOutput else { | ||
| return -1 | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
|
|
||
| guard let key = box.key else { | ||
| return -2 | ||
| } | ||
|
|
||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: box.length) | ||
| let copied: Bool | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| let mac = (box.hmac as! HMAC<Insecure.MD5>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha1: | ||
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha256: | ||
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha384: | ||
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha512: | ||
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| default: | ||
| return 0 | ||
| } | ||
|
|
||
| if !copied { | ||
| return 0 | ||
| } | ||
|
|
||
| return HmacReset(box: box, key: key) | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacCurrent") | ||
| public func AppleCryptoNative_HmacCurrent(ctx: UnsafeMutableRawPointer?, pOutput: UnsafeMutablePointer<UInt8>?) -> Int32 { | ||
| guard let ctx, let pOutput else { | ||
| return -1 | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: box.length) | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| let hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha1: | ||
| let hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha256: | ||
| let hmac = (box.hmac as! HMAC<SHA256>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha384: | ||
| let hmac = (box.hmac as! HMAC<SHA384>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha512: | ||
| let hmac = (box.hmac as! HMAC<SHA512>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| default: | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacFree") | ||
| public func AppleCryptoNative_HmacFree(ctx: UnsafeMutableRawPointer?) { | ||
| if let ctx { | ||
| Unmanaged<HmacBox>.fromOpaque(ctx).release() | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacClone") | ||
| public func AppleCryptoNative_HmacClone(ctx: UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer? { | ||
| guard let ctx else { | ||
| return nil | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| let cloneBox = HmacBox(box.algorithm, length: box.length) | ||
| cloneBox.key = box.key | ||
| cloneBox.hmac = box.hmac | ||
| return Unmanaged.passRetained(cloneBox).toOpaque() | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacOneShot") | ||
| public func AppleCryptoNative_HmacOneShot( | ||
| algorithm: Int32, | ||
| pbKey: UnsafeMutableRawPointer?, | ||
| cbKey: Int32, | ||
| pbBuf: UnsafeMutableRawPointer?, | ||
| cbBuf: Int32, | ||
| pOutput: UnsafeMutablePointer<UInt8>?, | ||
| cbOutput: Int32, | ||
| pcbDigest: UnsafeMutablePointer<Int32>? | ||
| ) -> Int32 { | ||
| guard let pcbDigest, let pOutput, cbKey >= 0, cbBuf >= 0 else { | ||
| return -1 | ||
| } | ||
|
|
||
| guard let hashAlgorithm = PAL_HashAlgorithm(rawValue: algorithm) else { | ||
| return -1 | ||
| } | ||
|
|
||
| let data: Data | ||
| let keyData: Data | ||
|
|
||
| if let pbBuf, cbBuf > 0 { | ||
| data = Data(bytesNoCopy: pbBuf, count: Int(cbBuf), deallocator: .none) | ||
| } else { | ||
| data = Data() | ||
| } | ||
|
|
||
| if let pbKey, cbKey > 0 { | ||
| keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) | ||
| } else { | ||
| keyData = Data() | ||
| } | ||
|
|
||
| let key = SymmetricKey(data: keyData) | ||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: Int(cbOutput)) | ||
|
|
||
| switch hashAlgorithm { | ||
| case .md5: | ||
| pcbDigest.pointee = Int32(Insecure.MD5.byteCount) | ||
| return HMAC<Insecure.MD5>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha1: | ||
| pcbDigest.pointee = Int32(Insecure.SHA1.byteCount) | ||
| return HMAC<Insecure.SHA1>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha256: | ||
| pcbDigest.pointee = Int32(SHA256.byteCount) | ||
| return HMAC<SHA256>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha384: | ||
| pcbDigest.pointee = Int32(SHA384.byteCount) | ||
| return HMAC<SHA384>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha512: | ||
| pcbDigest.pointee = Int32(SHA512.byteCount) | ||
| return HMAC<SHA512>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| default: | ||
| return -1 | ||
| } | ||
| } | ||
|
|
||
| private func HmacReset(box: HmacBox, key: SymmetricKey) -> Int32 { | ||
| switch box.algorithm { | ||
| case .md5: | ||
| box.hmac = HMAC<Insecure.MD5>(key: key) | ||
| box.key = key |
There was a problem hiding this comment.
The HMAC implementation introduces support for HMAC<Insecure.MD5> (e.g., in AppleCryptoNative_HmacCreate, AppleCryptoNative_HmacUpdate, AppleCryptoNative_HmacFinal, and related helpers), which relies on the MD5 hash function that is considered cryptographically weak for security-sensitive authentication. An attacker who can influence protocol design or choose algorithms may be able to downgrade integrity protections to MD5-based HMACs, making it easier to meet compliance exemptions or target weaker deployments over time. Prefer deprecating MD5-based HMACs for new usages and steering callers toward HMAC<SHA256> or stronger algorithms, keeping MD5 only behind explicit legacy/compatibility paths if it must remain at all.
Comment on lines
+566
to
+822
| let box = HmacBox(hashAlgorithm, length: Insecure.SHA1.byteCount) | ||
| pcbHmac.pointee = Int32(Insecure.SHA1.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha256: | ||
| let box = HmacBox(hashAlgorithm, length: SHA256.byteCount) | ||
| pcbHmac.pointee = Int32(SHA256.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha384: | ||
| let box = HmacBox(hashAlgorithm, length: SHA384.byteCount) | ||
| pcbHmac.pointee = Int32(SHA384.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| case .sha512: | ||
| let box = HmacBox(hashAlgorithm, length: SHA512.byteCount) | ||
| pcbHmac.pointee = Int32(SHA512.byteCount) | ||
| return Unmanaged.passRetained(box).toOpaque() | ||
| default: | ||
| pcbHmac.pointee = 0 | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacInit") | ||
| public func AppleCryptoNative_HmacInit(ctx: UnsafeMutableRawPointer?, pbKey: UnsafeMutableRawPointer?, cbKey: Int32) -> Int32 { | ||
| guard let ctx, let pbKey, cbKey >= 0 else { | ||
| return 0 | ||
| } | ||
|
|
||
| let keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) | ||
| let key = SymmetricKey(data: keyData) | ||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| return HmacReset(box: box, key: key) | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacUpdate") | ||
| public func AppleCryptoNative_HmacUpdate(ctx: UnsafeMutableRawPointer?, pbData: UnsafeMutableRawPointer?, cbData: Int32) -> Int32 { | ||
| guard let ctx, let pbData, cbData >= 0 else { | ||
| return 0 | ||
| } | ||
|
|
||
| if cbData == 0 { | ||
| return 1 | ||
| } | ||
|
|
||
| let data = Data(bytesNoCopy: pbData, count: Int(cbData), deallocator: .none) | ||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| var hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha1: | ||
| var hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha256: | ||
| var hmac = (box.hmac as! HMAC<SHA256>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha384: | ||
| var hmac = (box.hmac as! HMAC<SHA384>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| case .sha512: | ||
| var hmac = (box.hmac as! HMAC<SHA512>) | ||
| hmac.update(data: data) | ||
| box.hmac = hmac | ||
| return 1 | ||
| default: | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacFinal") | ||
| public func AppleCryptoNative_HmacFinal(ctx: UnsafeMutableRawPointer?, pOutput: UnsafeMutablePointer<UInt8>?) -> Int32 { | ||
| guard let ctx, let pOutput else { | ||
| return -1 | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
|
|
||
| guard let key = box.key else { | ||
| return -2 | ||
| } | ||
|
|
||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: box.length) | ||
| let copied: Bool | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| let mac = (box.hmac as! HMAC<Insecure.MD5>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha1: | ||
| let mac = (box.hmac as! HMAC<Insecure.SHA1>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha256: | ||
| let mac = (box.hmac as! HMAC<SHA256>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha384: | ||
| let mac = (box.hmac as! HMAC<SHA384>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| case .sha512: | ||
| let mac = (box.hmac as! HMAC<SHA512>).finalize() | ||
| copied = mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } | ||
| break | ||
| default: | ||
| return 0 | ||
| } | ||
|
|
||
| if !copied { | ||
| return 0 | ||
| } | ||
|
|
||
| return HmacReset(box: box, key: key) | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacCurrent") | ||
| public func AppleCryptoNative_HmacCurrent(ctx: UnsafeMutableRawPointer?, pOutput: UnsafeMutablePointer<UInt8>?) -> Int32 { | ||
| guard let ctx, let pOutput else { | ||
| return -1 | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: box.length) | ||
|
|
||
| switch box.algorithm { | ||
| case .md5: | ||
| let hmac = (box.hmac as! HMAC<Insecure.MD5>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha1: | ||
| let hmac = (box.hmac as! HMAC<Insecure.SHA1>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha256: | ||
| let hmac = (box.hmac as! HMAC<SHA256>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha384: | ||
| let hmac = (box.hmac as! HMAC<SHA384>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| case .sha512: | ||
| let hmac = (box.hmac as! HMAC<SHA512>) | ||
| let mac = hmac.finalize() | ||
| return mac.withUnsafeBytes { $0.copyBytes(to: destination) == $0.count } ? 1 : 0 | ||
| default: | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacFree") | ||
| public func AppleCryptoNative_HmacFree(ctx: UnsafeMutableRawPointer?) { | ||
| if let ctx { | ||
| Unmanaged<HmacBox>.fromOpaque(ctx).release() | ||
| } | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacClone") | ||
| public func AppleCryptoNative_HmacClone(ctx: UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer? { | ||
| guard let ctx else { | ||
| return nil | ||
| } | ||
|
|
||
| let box = Unmanaged<HmacBox>.fromOpaque(ctx).takeUnretainedValue() | ||
| let cloneBox = HmacBox(box.algorithm, length: box.length) | ||
| cloneBox.key = box.key | ||
| cloneBox.hmac = box.hmac | ||
| return Unmanaged.passRetained(cloneBox).toOpaque() | ||
| } | ||
|
|
||
| @_silgen_name("AppleCryptoNative_HmacOneShot") | ||
| public func AppleCryptoNative_HmacOneShot( | ||
| algorithm: Int32, | ||
| pbKey: UnsafeMutableRawPointer?, | ||
| cbKey: Int32, | ||
| pbBuf: UnsafeMutableRawPointer?, | ||
| cbBuf: Int32, | ||
| pOutput: UnsafeMutablePointer<UInt8>?, | ||
| cbOutput: Int32, | ||
| pcbDigest: UnsafeMutablePointer<Int32>? | ||
| ) -> Int32 { | ||
| guard let pcbDigest, let pOutput, cbKey >= 0, cbBuf >= 0 else { | ||
| return -1 | ||
| } | ||
|
|
||
| guard let hashAlgorithm = PAL_HashAlgorithm(rawValue: algorithm) else { | ||
| return -1 | ||
| } | ||
|
|
||
| let data: Data | ||
| let keyData: Data | ||
|
|
||
| if let pbBuf, cbBuf > 0 { | ||
| data = Data(bytesNoCopy: pbBuf, count: Int(cbBuf), deallocator: .none) | ||
| } else { | ||
| data = Data() | ||
| } | ||
|
|
||
| if let pbKey, cbKey > 0 { | ||
| keyData = Data(bytesNoCopy: pbKey, count: Int(cbKey), deallocator: .none) | ||
| } else { | ||
| keyData = Data() | ||
| } | ||
|
|
||
| let key = SymmetricKey(data: keyData) | ||
| let destination = UnsafeMutableRawBufferPointer(start: pOutput, count: Int(cbOutput)) | ||
|
|
||
| switch hashAlgorithm { | ||
| case .md5: | ||
| pcbDigest.pointee = Int32(Insecure.MD5.byteCount) | ||
| return HMAC<Insecure.MD5>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha1: | ||
| pcbDigest.pointee = Int32(Insecure.SHA1.byteCount) | ||
| return HMAC<Insecure.SHA1>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha256: | ||
| pcbDigest.pointee = Int32(SHA256.byteCount) | ||
| return HMAC<SHA256>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha384: | ||
| pcbDigest.pointee = Int32(SHA384.byteCount) | ||
| return HMAC<SHA384>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| case .sha512: | ||
| pcbDigest.pointee = Int32(SHA512.byteCount) | ||
| return HMAC<SHA512>.authenticationCode(for: data, using: key).withUnsafeBytes { mac in | ||
| return mac.copyBytes(to: destination) == mac.count ? 1 : -1 | ||
| } | ||
| default: | ||
| return -1 | ||
| } | ||
| } | ||
|
|
||
| private func HmacReset(box: HmacBox, key: SymmetricKey) -> Int32 { | ||
| switch box.algorithm { | ||
| case .md5: | ||
| box.hmac = HMAC<Insecure.MD5>(key: key) | ||
| box.key = key | ||
| return 1 | ||
| case .sha1: | ||
| box.hmac = HMAC<Insecure.SHA1>(key: key) | ||
| box.key = key | ||
| return 1 |
There was a problem hiding this comment.
The HMAC implementation also introduces HMAC<Insecure.SHA1> (e.g., in AppleCryptoNative_HmacCreate, AppleCryptoNative_HmacUpdate, AppleCryptoNative_HmacFinal, and helpers), which depends on SHA-1, a hash function that is no longer considered secure for modern integrity protection. If applications use these SHA-1–based HMACs for security-critical purposes (e.g., request signing, token validation), they may face reduced resistance to collision-style attacks and fail modern cryptographic policy/compliance requirements. Consider phasing out SHA-1-based HMACs in favor of HMAC<SHA256> or stronger algorithms, or clearly confining them to documented legacy-only scenarios.
This was referenced Jan 5, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This migrates MD5, SHA-1, and SHA-2 to CryptoKit for HMAC on Apple platforms. This is a continuation of the work done at #120953.