GNOME Bugzilla – Bug 785234
gcr encodes CKA_EC_POINT as a raw bigint instead of DER encoded as required by PKCS#11
Last modified: 2019-02-22 11:57:45 UTC
While working on gnome-keyring ECDSA support, I noticed that I was getting wrongly formatted PKCS#11 objects. The problem is that the gcr just puts into CKA_EC_POINT the bignum if (!read_buffer_mpi (buffer, offset, builder, CKA_EC_POINT)) but PKCS#11 says it should be DER-encoding of ANSI X9.62 ECPoint value Q This is fairly easy to fix in the code, but I was not able to find the way how to update the data, which are used to compare against the encoded values. I manually checked that the only changes are the lengths and this specific attribute. The patch is in github PR: https://github.com/GNOME/gcr/pull/5
Created attachment 356240 [details] [review] proposed patch
Review of attachment 356240 [details] [review]: This patch seems to touch only the decoding behavior of ASN.1 to PKCS#11, not the other way around (PKCS#11 to ASN.1): https://git.gnome.org/browse/gcr/tree/gcr/gcr-subject-public-key.c#n774 Would it make sense to update it as well?
Certainly yes. But I see it hard to orient in the gcr code, especially in the parts about the certificates, that are used around this code. I will probably not be able to handle that now, but I will try to find some time for that.
Created attachment 364502 [details] [review] Properly encode and decode CKA_EC_POINT in spk This is an addition to the previous patch to handle the encoding and decoding also in the spk level. All the tests are passing for me now. Reviewable also on github (both patches rebased to current master) https://github.com/GNOME/gcr/compare/master...Jakuje:ecdsa-pkcs11
Review of attachment 356240 [details] [review]: Looks fine. ::: egg/pk.asn @@ +120,3 @@ +-- bogus attribute used to encode Q into PKCS#11 CKA_EC_POINT +ECKeyQ ::= OCTET STRING nit: can't we reuse "ECPoint" defined above?
Review of attachment 364502 [details] [review]: Looks good, thanks!
Created attachment 364504 [details] [review] Properly encode and decode CKA_EC_POINT Sure. I did not notice the ECPoint (mainly because it was not in gnome-keyring). Good catch. Thanks for review. I squashed previous commits + and removed the ECKeyQ
Thank you, pushed it.