Hi Erich,
On Tue, Jan 31, 2012 at 9:34 AM, Erich E. Hoover ehoover@mines.edu wrote:
On Tue, Jan 31, 2012 at 10:23 AM, Erich E. Hoover ehoover@mines.edu wrote:
On Tue, Jan 31, 2012 at 10:04 AM, Juan Lang juan.lang@gmail.com wrote:
Sorry I didn't spot this earlier. Without this, someone who registers a certificate common name with an embedded NULL, like "codeweavers.com\0.badguy", could fool crypt32 into accepting it for a domain it isn't registered to, codeweavers.com in my example.
It looks like you've just changed it to allow more than one NULL at the end... It seems to me that the matching code already handles the case of an embedded NULL, since it goes through the allowed_len characters and manually checks each byte (rather than using a routine like strcmp() which stops at NULLs).
Well, sort of. The byte-by-byte comparison takes place component by component. The boundary between each component is defined by the presence of a '.'. That's why, in my example, I have an embedded NULL immediately prior to a '.'. After the end of each component is found, it's passed to match_domain_component. In the current git version, each component strips a NULL, hence an embedded NULL is accepted. In the version I sent, only trailing NULL(s) are removed.
You're right that I allow multiple trailing NULLs rather than just one, but that difference seems immaterial. The key one is to prevent NULLs immediately prior to dots. --Juan