[Swan-dev] weird sanitization in idtoa()+dntoa()

[Andrew Cagney]

I think most of the weirdness is gone:
- str_{id,dn}() always sanitize things ready for logging; and are
implemented using the jam_*() primitives
- jam_{id,dn}() can be used for metachar escaping and shell variables
There are still a few idtoa() calls lurking but I think they can
change to str_id().

The gotya seems to be dntoa().  It's output gets fed to
CERT_AsciiToName() and that expects RFC1485 (that RFC's been obsoleted
/ rewritten).  I'm not sure how well dntoa() handles all this (it
seems to quote characters different to the RFC, but I'm just guessing
by reading the code :-()

In https://bugzilla.mozilla.org/show_bug.cgi?id=1342137 there's an
example of a UTF-8 string been fed into CERT_AsciiToName(), so the
need to handle !isprint() is real.  It would be nice if we could test
all this.



On Mon, 9 Sep 2019 at 11:33, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>
>
>
> On Sun, 8 Sep 2019 at 14:02, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>>
>> First, does anyone know a way to first create a DN containing !isprint() and then get it past NSS - pluto should only look at certs after the've been NSS approved and NSS could be rejecting bad DNs (anyone?) which could make the below largely academic.
>>
>> Now some background on DN/ID sanitizing.  I think things can be reduced to:
>>
>> %.*s, len, ptr - dumps a raw string into a buffer stopping at len or NUL - this is what "raw text" means in the below
>> escape_metachar() replaces shell meta characters ('"...) with \NNN but leaves !isprint() alone
>> sanitize_string() replaces !isprint() with \NNN
>>
>> dntoa() - dumps the DN as raw text
>> str_dn() - calls dntoa() and then calls sanitize_string()
>> idtoa() (and str_id) - dumps an ID into a buffer; but then scribbles on the result replacing !isprint() with '?'; since it calls dntoa() it applies to the raw DN
>> jam_id_escaped (it was called something else before): calls idtoa() so !isprint()->''?' and then escape_metachar() so '$... -> \.
>>
>> hopefully I've got this right (and I've managed to spread the confusion :-)
>>
>> So how is it used?
>>
>> First there's logging.   Here I don't think it matters (just as long as !isprint() is sanitized).
>> For no obvious reason we've got idtoa() using '?' and str_dn() using \NNN (like sanitize_string()) - so I figure changing everything to the \NNN is harmless.
>>
>> Next there's shell variables and here's where things get weird:
>>
>> - .._DN='...': the code calls dntoa()+escape_metachar() which means that while shell metachars are escaped !isprint() is not - so, in theory the shell variable could contain escape characters ...
>> - ..._ID='...': the code calls jam_id_escaped() which means metachars get escaped and !isprint() becomes '?' - the shell variable doesn't match the content
>>
>
> It turns out that ever since 0c9e7831570fbe1c641df16baf51446b55e63a7e in '13, escaped-metachars have been printed as (\t):
>    \0311
> instead of:
>    \011
> which strongly suggests that either:
> - the escaping isn't happening in the real world
> - it is, but the shell variables are only ever printed (they can't meaningfully be compared or used)
>
>
>> Anyone?  My guess is that it doesn't matter but !isprint() should probably be suppressed. The obvious choice is:
>> - !isprint() -> \NNN but that will trip up on shell escapes - \ would end up being escaped :-)
>> - metachar -> \meta - fine
>> but remember !isprint() is off script so the choice is arbitrary.
>>
>> Thoughts?
>>
>> Andrew
>>
>>
>>
>>
>>
>>
>>