Implement Elliptic Curve (EC) crypto support#3038
Conversation
…, and HKDF support to crypto library
floitsch
left a comment
floitsch
left a comment
There was a problem hiding this comment.
First round :)
Looks good though. Nothing serious.
|
|
||
| /** | ||
| Generates a new EC key pair for the given $curve. | ||
| Common curves: "secp256r1", "secp384r1", "secp521r1". |
There was a problem hiding this comment.
How do I know which curves are available?
Should we make constants for them?
There was a problem hiding this comment.
Good point! Added named constants to EcKey:
static CURVE-SECP256R1 ::= "secp256r1"
static CURVE-SECP384R1 ::= "secp384r1"
static CURVE-SECP521R1 ::= "secp521r1"The generate docstring now references them as well.
| The $password is optional and used for encrypted keys. | ||
| */ | ||
| static parse-private key/io.Data --password/string?="" -> EcKey: | ||
| der := ec-get-private-key-der_ key (password or "") |
There was a problem hiding this comment.
The default value already ensures that it is never null.
| der := ec-get-private-key-der_ key (password or "") | |
| der := ec-get-private-key-der_ key password |
| The $key must be DER or PEM. | ||
| The $password is optional and used for encrypted keys. | ||
| */ | ||
| static parse-private key/io.Data --password/string?="" -> EcKey: |
There was a problem hiding this comment.
| static parse-private key/io.Data --password/string?="" -> EcKey: | |
| static parse-private key/io.Data --password/string="" -> EcKey: |
| The $ikm is the input keying material. | ||
| The $salt is optional. | ||
| The $info is optional context information. | ||
| The $length is the desired length of the output key. |
There was a problem hiding this comment.
A lot of Toit code uses 'size' instead of 'length'. Maybe the crypto libs are different.
have a look at the existing code and decide by yourself.
I guess the length is in bytes?
There was a problem hiding this comment.
Checked the existing crypto code — size is the predominant convention. Renamed --length to --size throughout and explicitly documented that it is in bytes.
| The $hasher-creator is a lambda that creates an HMAC object for a given key. | ||
| */ | ||
| hkdf --ikm/ByteArray --salt/ByteArray? --info/ByteArray? --length/int [hasher-creator] -> ByteArray: | ||
| // Step 1: Extract |
There was a problem hiding this comment.
| // Step 1: Extract |
| return result; | ||
| } | ||
|
|
||
| PRIMITIVE(ec_get_public_key_der) { |
There was a problem hiding this comment.
Looks very similar to ec_get_private_key_der. Can we use a shared helper to avoid the code duplication?
There was a problem hiding this comment.
Done! Extracted ec_get_der_helper as a shared static helper, modeled after the existing rsa_export_der pattern. Both ec_get_private_key_der and ec_get_public_key_der are now thin wrappers around it.
| } | ||
|
|
||
| size_t glen = (ec_prv->grp.pbits + 7) / 8; | ||
| ByteArray* result = process->allocate_byte_array(glen); |
There was a problem hiding this comment.
Move the allocation to the top of the function.
| curves.do: | curve | | ||
| print "Testing curve: $curve" | ||
|
|
||
| // Test generation |
There was a problem hiding this comment.
Finish comments with ".". Here and in the rest of the file.
| // Test generation | |
| // Test generation. |
| print " Signature: $(signature.size) bytes" | ||
|
|
||
| // Test verification | ||
| if key-pair.verify message signature: |
There was a problem hiding this comment.
We typically just write expect (key-pair.verify message signature). But i can live with the verbose output.
| // 1. Tamper with tag (last bytes) | ||
| tampered-tag := ciphertext.copy | ||
| tampered-tag[tampered-tag.size - 1] ^= 0xFF | ||
| if (catch: key-pair.decrypt-ecies tampered-tag) != ec.Ecies.AUTHENTICATION-FAILURE: |
There was a problem hiding this comment.
expect-throws would deal with that.
here and below.
There was a problem hiding this comment.
All fixed!
- Terminated all comments with
.throughout the file. - Replaced the verbose
if/elseverification pattern withexpect (key-pair.verify message signature). - Replaced all
catch: ... != AUTHENTICATION-FAILUREpatterns withexpect-throws.
…e, and use Defer for mbedtls cleanup
…DF-derived nonces
2 participants
Adds EC key generation, ECDSA signing/verification, ECDH key exchange, and ECIES encryption to the crypto module.
What's included