Don't parse EC parameters and point #58
Conversation
| @@ -167,9 +162,9 @@ let to_json : type a . a t -> Yojson.Safe.json = fun attribute -> | |||
| | CKA_MODIFIABLE, param -> | |||
| p_bool "CKA_MODIFIABLE" param | |||
| | CKA_EC_PARAMS, param -> | |||
| p_ec_params "CKA_EC_PARAMS" param | |||
| p_data "CKA_EC_PARAMS" param | |||
There was a problem hiding this comment.
This is binary data. If I understand it correctly, this means that this will be serialized as hex? Can you add a couple unit tests for this?
There was a problem hiding this comment.
What should be tested exactly? The fact that it uses p_data or the fact the p_data serializes its second argument to hex?
There was a problem hiding this comment.
I was thinking checking that of_yojson (Assoc [CKA_EC_PARAMS, String "0x4142}") is [pack CKA_EC_PARAMS "AB"], and similarly for to_yojson.
src_dll/pkcs11_fake.c
Outdated
| @@ -184,6 +184,12 @@ CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hObject, | |||
| get_single_attribute_value(pAttribute, &value, sizeof(value), &ret); | |||
| } | |||
| break; | |||
| case CKA_EC_PARAMS: | |||
| { | |||
| CK_BYTE value[] = {}; | |||
There was a problem hiding this comment.
Can you add a valid value here? You can find some valid encodings here for example.
And also can you add a corresponding test in test_get_attribute_value in test_driver.ml?
| let decode_ec_point cs = | ||
| let grammar = Key_parsers.Asn1.EC.point_grammar in | ||
| let codec = Asn.codec Asn.ber grammar in | ||
| match Asn.decode codec cs with |
There was a problem hiding this comment.
If I understand it correctly, this was the only use of Key_parsers. Can you remove it from _tags, opam, .merlin and META?
|
Two small remarks mostly to self:
|
bbc2
force-pushed
the
do-not-parse-ec-parameters
branch
2 times, most recently
from
f606163 to
5d977fe
Compare
|
Should be good now. |
|
Looks good, thanks. If you rebase your branch onto |
In particular, don't return CKA_CS_UNKNOWN if the parsing of EC parameters or point failed. The use of CKA_CS_UNKNOWN is reserved for codes unknown to the library. This removes the dependency on key-parsers.
bbc2
force-pushed
the
do-not-parse-ec-parameters
branch
from
5d977fe to
f7eb214
Compare
|
I just did that. |
|
Thanks! ✨ |
In particular, don't return CKA_CS_UNKNOWN if the parsing of EC parameters or point failed. The use of CKA_CS_UNKNOWN is reserved for codes unknown to the library.