Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for all possible private keys and all possible lengths of message digest? #53

Closed
concise opened this issue Aug 9, 2015 · 3 comments

Comments

@concise
Copy link

concise commented Aug 9, 2015

For a complete implementation of ECDSA signature scheme, I think the signing functions can be changed to accept a private_key pointer to a buffer of length uECC_N_BYTES (a newly defined macro for the byte length of curve_n) instead of uECC_BYTES, because in some cases the bit length of curve_n (for curves like secp160r1, secp224k1, and secp521r1) is not in the set { 80, 96, 112, 128, 192, 256 }. We can not represent all possible private keys if we only use uECC_BYTES octets. That means, some users of a valid private key cannot even use this C library to generate ECDSA signatures.

For a similar reason, I think the functions may accept an additional argument indicating the length of the buffer pointed by message_hash. Otherwise, by forcing the message digest to be uECC_BYTES octets long, users cannot use this library directly to produce an ECDSA signature on secp192r1 curve with SHA-256 in the standard way.

int uECC_sign(const uint8_t private_key[uECC_BYTES], // cannot represent all keys
              const uint8_t message_hash[uECC_BYTES], // may need to be truncated by the user
              uint8_t signature[uECC_BYTES*2]);

int uECC_sign_deterministic(const uint8_t private_key[uECC_BYTES], // cannot represent all keys
                            const uint8_t message_hash[uECC_BYTES], // may need to be truncated by the user
                            uECC_HashContext *hash_context,
                            uint8_t signature[uECC_BYTES*2]);

int uECC_verify(const uint8_t public_key[uECC_BYTES*2],
                const uint8_t hash[uECC_BYTES], // may need to be truncated by the user
                const uint8_t signature[uECC_BYTES*2]);

This is some sort of a design choice. By not supporting all possible combinations of inputs, the library codebase can be a little bit simpler and easier to maintain.

@concise
Copy link
Author

concise commented Aug 9, 2015

Actually, "secp192r1 with SHA-256" is not a good counter example, because that is actually "supported" by the current implementation; just discarding everything after the 24th octets will be fine. However, the combinations like "secp256r1 with SHA-1" is not directly supported by micro-ecc now. (in which case the user has to manually prepend 12 zero bytes to the SHA-1 hash)

kmackay added a commit that referenced this issue Sep 19, 2015
Previously, callers would need to manually convert the hash value
appropriately if it was not the same length as curve_n. Now, callers
just pass in the full hash value and the length; uECC will convert
the hash as appropriate.
kmackay added a commit that referenced this issue Sep 19, 2015
@kmackay
Copy link
Owner

kmackay commented Sep 19, 2015

Now supported on the runtime branch.

kmackay added a commit that referenced this issue Oct 13, 2015
Previously, callers would need to manually convert the hash value
appropriately if it was not the same length as curve_n. Now, callers
just pass in the full hash value and the length; uECC will convert
the hash as appropriate.
kmackay added a commit that referenced this issue Oct 13, 2015
@kmackay
Copy link
Owner

kmackay commented Oct 13, 2015

Now supported in master

@kmackay kmackay closed this as completed Oct 13, 2015
aeruder pushed a commit to aeruder/micro-ecc that referenced this issue Dec 23, 2015
This was changed in commit

    0283b54 Convert hash to int in sign/verify (kmackay#53)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants