Switch Apple's digest functionality to CryptoKit#120953
Switch Apple's digest functionality to CryptoKit#120953akoeplinger merged 14 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Outdated
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Outdated
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Outdated
Show resolved
Hide resolved
|
@akoeplinger added you as a reviewer largely for awareness and to better understand if we are ready to take a "break iOS 12" change for NET 11 or if we should sit on this longer. |
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Outdated
Show resolved
Hide resolved
|
After thinking about this more, I don't think the copy semantics work the way I an expecting them to.
I think for SHA-2 we should stick with CommonCrypto to make clone and current work. For SHA-3 we can implement with CryptoKit, but we will probably need to omit GetCurrentDigest and Clone. At least until CryptoKit has a clear way to "fork" a hash in a documented way. |
|
Okay, after doing some more digging and testing - using a |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates Apple platform's digest/hashing implementation from CommonCrypto to CryptoKit, positioning the codebase for future SHA-3 support while dropping support for iOS/tvOS 12.
Key Changes:
- Replaces C-based CommonCrypto digest implementation with Swift-based CryptoKit implementation
- Updates P/Invoke signatures to use Swift calling conventions
- Implements digest operations (one-shot, create, update, final, reset, clone, current) using CryptoKit's HashFunction protocol
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pal_swiftbindings.swift | Adds HashBox wrapper class and implements all digest functions using CryptoKit hash types (MD5, SHA1, SHA256, SHA384, SHA512) |
| pal_swiftbindings.h | Adds extern C declarations for the new Swift-implemented digest functions |
| pal_digest.h | Removes function declarations that are now implemented in Swift |
| pal_digest.c | Removes entire C implementation of digest functions based on CommonCrypto |
| CMakeLists.txt | Removes pal_digest.c from build sources |
| Interop.Digest.cs | Adds Swift calling convention attributes to P/Invoke declarations |
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Show resolved
Hide resolved
...libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Digest.cs
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Assuming we're good with the min-ver bump, LGTM. |
src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift
Outdated
Show resolved
Hide resolved
|
@akoeplinger are there any other pipelines we should run for confidence in mobile Apple platforms? I am not sure what is included in the "smoke" ones. |
Easiest way is to check the Helix workitems, right now it's running mostly System.Runtime.Tests. |
|
/azp run runtime-ioslikesimulator |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
runtime-ioslikesimulator is green, merging :) |
This changes our implementation of hashing to be based on CryptoKit instead of CommonCrypto. This will set us up on a path for success to eventually add handling for SHA-3.
A couple of notes on implementation: