[Opendnssec-develop] HSMs use UTF-8 characters

[Rick van Rein]

Hi Sion / Jakob / Jerry,

> There is very low risk

Indeed, there is a positive risk

> and I cannot see any realistic attacks to the current implemention. The only component creating keys in the repositories are OpenDNSSEC itself

The PKCS #11 interface is a shared resource, so OpenDNSSEC is the only component as long as attackers play by our rules.

Also, programming errors such as unnoticed cut-offs of UTF-8 multiple-byte characters could cause the named issues.  Just type a funny character in the right place and you’ve triggered it.

> and we control that code.

Hardly: I’m pretty sure nobody using libhsm has considered these potential problems, and that is precisely why I want to make it explicit.  Both documentation and code are currently ignoring anything that is not ASCII.

Also, when printing literal codes to an UTF-8 output, we may upset such code.  You’d be trusting terminals, screen / tmux, and much more to behave properly when presented with downright wrong data.  Let’s not take that risk.

> The remaining attack vectors would be imported keys, token labels and PINs.

Only textual fields, actually, and indeed also as part of imported keys.

> Is fixing this worth the effort? If the fix is easy, go ahead. But change always introducing risk as well.

The fix is easy, much easier than the systematic ignorance of checking for NULL return values and race condition problems ;-)
For the libhsm-using application it is simply a matter of
 - renaming (char *) to (wchar_t *)
 - using special formats in printf/scanf
 - perhaps using wcslen() instead of strlen() and such, as driven by type errors
Within libhsm, I would map with mbstowcs() to convert, and possibly perform an RFC2279 syntax check.

> Does this cover the CKA_ID also? So could someone, potentially, import a
> key that can then not be used (or worse)?

CKA_ID is a binary code, so it should never interpreted as text in any representation; but CKA_LABEL is its hex representation; this is generated by the keygen but never sought for.

> I do not think we use this information for anything else or for display
> so the impact code wise might be small.

Yes, probably.

> For what version are we considering this for? I would think 2.x'ish.

The audit I’m doing is against 1.4 and will be backported as well as possible to 1.3.

-Rick