Re: implement subject alternative names support for SSL connections

On 09/04/2014 10:33 AM, Alexey Klyukin wrote:
> On Wed, Sep 3, 2014 at 11:50 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>
>> * It's ugly that the caller does the malloc and memcpy, and the
>> certificate_name_entry_validate_match function then modifies its name
>> argument. Move the malloc+memcpy inside the function.
>
> For the case of CN the caller has to do the malloc anyway, since
> X509_NAME_get_text_by_NID expects the already allocated buffer.
> This means that 'movable' malloc + memcpy occurs only once, and while
> it would still make sense to move it into the function, it would also
> mean we would do an unnecessary malloc for the case of CN.

Hmm. Perhaps we should use X509_NAME_get_index_by_NID + 
X509_NAME_get_entry instead of X509_NAME_get_text_by_NID. You could then 
pass the ASN1_STRING object to the 
certificate_name_entry_validate_match() function, and have it do the 
ASN1_STRING_length() and ASN1_STRING_data() calls too.

>>> The tricky part is the error
>>> message if no match was found: initially, it only listed a single
>>> common name, but now tracking all DNS names just for the sake of the
>>> error message makes the code more bloated, so I'm wondering if simply
>>> stating that there was no match, as implemented in the attached patch,
>>> would be good enough?
>>
>> Hmm. It would still be nice to say something about the certificate that was
>> received. How about:
>>
>>    server certificate with common name "%s" does not match host name "%s"
>
> We cannot guarantee at this point that the common name is present in
> the certificate. And if it is not, which name should we pick instead?
> One way to solve this is to take the first non-NULL name, but it if
> there is no common name, but then the wording of the error message
> would depend on the availability of CN.
> Another is to show all available names, but I do not like collecting
> them just for the sake of displaying in the error message.
> And last one is to just show the error without mentioning names,
> that's what I've chosen to be the most consistent.

I think we should:

1. Check if there's a common name, and if so, print that
2. Check if there is exactly one SAN, and if so, print that
3. Just print an error without mentioning names.

There's a lot of value in printing the name if possible, so I'd really 
like to keep that. But I agree that printing all the names if there are 
several would get complicated and the error message could become very 
long. Yeah, the error message might need to be different for cases 1 and 
2. Or maybe phrase it "server certificate's name \"%s\" does not match 
host name \"%s\"", which would be reasonable for both 1. and 2.

- Heikki