cryptosense / pkcs11

OCaml bindings for the PKCS#11 cryptographic API
BSD 2-Clause "Simplified" License
23 stars 6 forks source link

Encode CKA_ID as hex string in json #101

Closed NathanReb closed 6 years ago

NathanReb commented 6 years ago

CKA_ID is associated with an arbitrary byte string, potentially containing non ascii characters leading to invalid json encoding.

NathanReb commented 6 years ago

Just fixed the tests!

NathanReb commented 6 years ago

Hmm this is a breaking change, I'll update CHANGES.md!

emillon commented 6 years ago

I usually do that just before releasing, but that works as well.

Is there an easy way to stay compatible with the former format, for just a single release?

NathanReb commented 6 years ago

I guess we could try to parse it as hex encoded and if it fails, fallback to a raw string? Does it sound good to you?

emillon commented 6 years ago

Yes, that would work. Also, I'm not sure that parsing as a raw string used to work, since IIRC "%S" is used when dumping but the raw string is returned in the other direction (that would mean that escape characters are left).

Anyway, the fix is the important part, so if compatibility is a problem, we can just declare a breaking change here.

NathanReb commented 6 years ago

The problem with that solution is that if you had serialized CKA_ID "0x00" before the fix, you'll get CKA_ID "\x00" when deserializing it with the fixed version.

I'm not sure this is likely to happen but in that case, it would be a breaking change and you wouldn't know about it :/

NathanReb commented 6 years ago

I have a patch for pseudo compatibility if you want it so let me know what you prefer!

emillon commented 6 years ago

OK, let's keep it simple then!

emillon commented 6 years ago

Thanks!