RSA Support #88
RSA Support #88
Conversation
Sources/WebAuthn/Ceremonies/Shared/COSE/COSEAlgorithmIdentifier.swift
Outdated
Show resolved
Hide resolved
Sources/WebAuthn/Ceremonies/Shared/COSE/COSEAlgorithmIdentifier.swift
Outdated
Show resolved
Hide resolved
|
I just committed a new version |
Sources/WebAuthn/Ceremonies/Shared/COSE/COSEAlgorithmIdentifier.swift
Outdated
Show resolved
Hide resolved
Co-authored-by: Dimitri Bouniol <dimitribouniol@mochidev.com>
…r.swift Co-authored-by: Dimitri Bouniol <dimitribouniol@mochidev.com>
Co-authored-by: Dimitri Bouniol <dimitribouniol@mochidev.com>
|
Since #99 moved everything over to Swift Testing, can we make use of parameterized tests to test both ECDSA and RSA keys in the same tests? This should help when we add more key types in the future as well, since the tests are pretty identical. This means this PR will be blocked behind the following, but I think we can get them in quickly to unblock it: |
|
I have fixed the requested issues. |
|
@dscreve Thanks for this! I’ll take a look at merging in the new tests in ~12h so we can get this over the finish line. |
|
@dscreve I don't have write permissions to your branch, but could you pull If this repo's remote is named git fetch upstream
git pull upstream dimitri/rsa-fixes
git push |
I executed the given commands while being on the main branch (not the RSA-Support branch). |
|
The point would be to get those commits onto this branch so they resolve the conflicts — could you checkout your RSA branch and re-run them? |
Done |
| @@ -24,7 +24,7 @@ let package = Package( | |||
| ], | |||
| dependencies: [ | |||
| .package(url: "https://github.com/unrelentingtech/SwiftCBOR.git", from: "0.4.7"), | |||
| .package(url: "https://github.com/apple/swift-crypto.git", "2.0.0" ..< "4.0.0"), | |||
| .package(url: "https://github.com/apple/swift-crypto.git", "3.8.1" ..< "4.0.0"), | |||
There was a problem hiding this comment.
@0xTim Do you happen to know if there is an earlier version that supports RSA? My local package.resolved file initially didn't work, but I already lost the version it was locked to by the time I realized I could search from there. I think https://github.com/apple/swift-crypto/releases/tag/3.8.1 is the earliest we can go, but perhaps you know of an earlier version.
There was a problem hiding this comment.
I don't think there is
(And to be honest I'd be happy to just start with the latest release anyway)
| @@ -153,11 +152,12 @@ struct EC2PublicKey: PublicKey, Sendable { | |||
| .isValidSignature(ecdsaSignature, for: data) else { | |||
| throw WebAuthnError.invalidSignature | |||
| } | |||
| default: | |||
There was a problem hiding this comment.
I still think we may want to list out the unsupported types here to catch new key sizes in the future, but the list may be a long one, so just a nit.
There was a problem hiding this comment.
I'm less worried about this. The want to throw an unsupported and any new supported ones will need to be added to the switch to make tests pass
| @Test(arguments: [ | ||
| TestKeyConfiguration.ecdsa, | ||
| TestKeyConfiguration.rsa, |
RSA Support