agens-no / EllipticCurveKeyPair

Sign, verify, encrypt and decrypt using the Secure Enclave
Other
709 stars 115 forks source link

Document (in comments or notes) signature algorithm #11

Closed swizzlr closed 7 years ago

swizzlr commented 7 years ago

ecdsaSignatureDigestX962SHA256 is used to sign data; however, it appears to sign data that has already been digested into sha-256.

We should document what's going on here (double digest?) and also how to verify it in OpenSSL. The demo app outputs some weird code to do with hexdumps and simple sha256 verification, as opposed to OpenSSL's -ecdsa-with-SHA1 (I think this is something I don't personally understand about crypto, so it's my problem, but I'd like to elucidate it further).

hfossli commented 7 years ago

I see... Good thinking. I’m first able to check this in details on monday. I have a family of four to prioritize in the weekend.

swizzlr commented 7 years ago

Oh please ignore, these are mostly for my reference.

hfossli commented 7 years ago

I love it. Keep it coming 😊

hfossli commented 7 years ago

You ask about this, right? openssl dgst -sha256 -verify key.pem -signature signature.dat dataToSign.dat?

I am not sure, but I guess it does sha256 on the digest only and then goes on and verifies the signature. Openssl knows what kind of key this is and which cryptogtaphic algorithm to use because of headers in key. Just my guess. And if I am to guess more I think secp256r1 doesn’t do any sha256 itself it just expects 256 bits and in order to get there we need to sha256 first. It could probably be another hashing algorithm as well.

swizzlr commented 7 years ago

Yeah, that's my understanding too. Turns out signature algorithms like specifying the digest algorithm too.

wuf810 commented 7 years ago

I’ve been wondering about this as well. I thought I was also doing an additional hash but what you say makes sense.

However what I’m not sure of is when to use ecdsaSignatureMessageX962SHA256. I’m assuming to is for signing clear messages? And again assuming in this case no need for a SHA256 hash first?

swizzlr commented 7 years ago

@wuf810 no, I think the algorithm is coupled with the hash, but the expected input is a sha-256 hash.

hfossli commented 7 years ago

Huh. I hadn't actually noticed there are two different options.

ecdsaSignatureMessageX962SHA256
ECDSA algorithm, signature is in DER x9.62 encoding, SHA-256 digest is generated from input data of any size.

ecdsaSignatureDigestX962SHA256
ECDSA algorithm, signature is in DER x9.62 encoding, input data is message digest created by SHA256 algorithm.

I'm fine with this change

        @available(iOS 10.0, *)
        private func signUsingNewApi(_ digest: Data, privateKey: PrivateKey) throws -> Data {
            Helper.logToConsoleIfExecutingOnMainThread()
-           let digestToSign = digest.sha256()
            var error : Unmanaged<CFError>?
+           let result = SecKeyCreateSignature(privateKey.underlying, .ecdsaSignatureMessageX962SHA256, digest as CFData, &error)
-           let result = SecKeyCreateSignature(privateKey.underlying, .ecdsaSignatureDigestX962SHA256, digestToSign as CFData, &error)
            guard let signature = result else {
                throw Error.fromError(error?.takeRetainedValue(), message: "Could not create signature.")
            }
            return signature as Data
        }
hfossli commented 7 years ago

I see the private key supports

supports algid:sign:ECDSA:RFC4754 for operation 0
supports algid:sign:ECDSA:digest-X962 for operation 0
supports algid:sign:ECDSA:digest-X962:SHA1 for operation 0
supports algid:sign:ECDSA:digest-X962:SHA224 for operation 0
supports algid:sign:ECDSA:digest-X962:SHA256 for operation 0
supports algid:sign:ECDSA:digest-X962:SHA384 for operation 0
supports algid:sign:ECDSA:digest-X962:SHA512 for operation 0
supports algid:sign:ECDSA:message-X962:SHA1 for operation 0
supports algid:sign:ECDSA:message-X962:SHA224 for operation 0
supports algid:sign:ECDSA:message-X962:SHA256 for operation 0
supports algid:sign:ECDSA:message-X962:SHA384 for operation 0
supports algid:sign:ECDSA:message-X962:SHA512 for operation 0
supports algid:encrypt:ECIES:ECDH:KDFX963:SHA1:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDH:KDFX963:SHA224:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDH:KDFX963:SHA256:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDH:KDFX963:SHA384:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDH:KDFX963:SHA512:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDHC:KDFX963:SHA1:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDHC:KDFX963:SHA224:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDHC:KDFX963:SHA256:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDHC:KDFX963:SHA384:AESGCM for operation 3
supports algid:encrypt:ECIES:ECDHC:KDFX963:SHA512:AESGCM for operation 3

So I guess we could take the algorithm as a parameter if that's desired.

wuf810 commented 7 years ago

Sounds good to me…removes an unnecessary step which is always good :-)

swizzlr commented 7 years ago

Could probably strip out the sha-256 file too I guess?

wuf810 commented 7 years ago

Is that not needed for the secKeyRawSign code (pre iOS 10 signing?).

hfossli commented 7 years ago

Yep, that's needed for iOS 9. Also, I think it will make the code worse if we take the algorithm as a parameter and at the same time wants to support iOS 9...

Please let me know if you would like to drop support for iOS 9 #12

hfossli commented 7 years ago

Please review this pull request https://github.com/agens-no/EllipticCurveKeyPair/pull/13.

hfossli commented 7 years ago

Check out the new method signature on sign, verify, encrypt and decrypt on master. Is there anything more that should be done in this issue? Or has it been resolved?

swizzlr commented 7 years ago

I think it has been!