TS-4459 - Force domain names in cert to lower on insert into lookup tree

[reveller]

This commit resolves domain names in a cert which need to be lower cased before being inserted into the lookup table.

This was patched by Michael Janiszewski at GoDaddy.

[jpeach]

I don't think that we should do the malloc and lowercasing on every lookup.

How about this ... when the caller indexes the certificate, check whether the name is lowercased. If not, insert 2 entries; one lowercased and one with the case provided. Then on the lookup you ought to hit one of the entries but you don't care which one.

[mjaniszewski-godaddy]

The point regarding malloc/lowercase on each lookup is a good one, but indexing two separate versions of the cert won't cover all cases.

Let's say a cert has a common name of EXAMPLE.COM, and we index said cert under both EXAMPLE.COM and example.com. In this model, requests coming in for EXAMPLE.com/Example.COM/etc. won't match. According to the relevant RFC (http://tools.ietf.org/html/rfc6125#section-6.4.1), we should be matching in a case insensitive manner, regardless of the case present in the cert CN/SubjectAltName.

Would you be ok with a check on lookup to determine if the hostname needs to be modified, along with a conditional malloc/lowercase/free based on that? That should avoid the hit in the vast majority of cases.

[jpeach]

Yep. We use TS_MAX_HOST_NAME_LEN in various places, so we already have a fixed upper limit on host names. So how about we always lowercase when inserting, and lower case on lookup if the presented case fails? We don't need to malloc, just use a stack temporary to do the lowercasing.

[zwoop]

Adding @shinrich for review too please.

[shinrich]

I'd been watching this, but @jpeach had already made the relevant comments. I like using a stack buffer of size TS_MAX_HOST_NAME_LEN. Avoids allocation pain. Keeps things simple. Otherwise, this looks like a good change.

[zwoop]

@reveller Any update? :)

[reveller]

I just spoke to the developer about this yesterday and told him he either needs to update it or I am going to close this for now. He has indicated he will update it soon and remove the allocations.