From: | Alexey Klyukin <alexk(at)hintbits(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: implement subject alternative names support for SSL connections |
Date: | 2014-09-11 17:46:46 |
Message-ID: | CAAS3tyKtt-twcfM8OobMsv6m_VoZaDS-KwLtvumToOLdCQ09Ag@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Sep 8, 2014 at 8:04 PM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> On 09/05/2014 07:30 PM, Alexey Klyukin wrote:
>> The error does not state whether the names comes from the CN or from
>> the SAN section.
>
>
> I'd reword that slightly, to:
>
> psql: server certificate for "example.com" (and 2 other names) does not
> match host name "example-foo.com"
>
> I never liked the current wording, "server name "foo"" very much. I think
> it's important to use the word "server certificate" in the error message, to
> make it clear where the problem is.
+1
>
> For translations, that message should be "pluralized", but there is no
> libpq_ngettext macro equivalent to ngettext(), like libpq_gettext. I wonder
> if that was left out on purpose, or if we just haven't needed that in libpq
> before. Anyway, I think we need to add that for this.
Well, the translation guidelines in the documentation suggest avoiding
plurals if possible, but I don't like rephrasing the sentence with
'names total: %d', so attached is my attempt to add the function.
I have also moved the one-time library binding code to the function of its own.
>
>> This version also checks for the availability of the subject name, it
>> looks like RFC 5280 does not require it at all.
>>
>> 4.1.2.6. Subject
>>
>> The subject field identifies the entity associated with the public
>> key stored in the subject public key field. The subject name MAY be
>> carried in the subject field and/or the subjectAltName extension.
>
>
> Ok.
>
>> The pattern of allocating the name in the subroutine and then
>> reporting it (and releasing when necessary) in the main function is a
>> little bit ugly, but it looks to me the least ugly of anything else I
>> could come up (i.e. extracting those names at the time the error
>> message is shown).
>
>
> I reworked that a bit, see attached. What do you think?
Thank you, I like your approach of unconditionally allocating and
freeing memory, it makes the code easier to read.
2 minor changes I've made in v7 (in addition to adding the libpq_ngettext):
- the verify_peer_name_matches_certificate does not try to return -1
(since it returns bool, it would be misinterpreted as true)
- removed the !got_error && !found_match check from the for loop
header to the loop body per style comment from Tom in the beginning of
this thread.
>
> I think this is ready for commit, except for two things:
>
> 1. The pluralization of the message for translation
>
> 2. I still wonder if we should follow the RFC 6125 and not check the Common
> Name at all, if SANs are present. I don't really understand the point of
> that rule, and it seems unlikely to pose a security issue in practice,
> because surely a CA won't sign a certificate with a bogus/wrong CN, because
> an older client that doesn't look at the SANs at all would use the CN
> anyway. But still... what does a Typical Web Browser do?
>
At least Firefox and Safari seem to follow RFC strictly, according to
some anecdotical evidences (I haven't got enough time to dig into the
source code yet):
http://superuser.com/questions/230136/primary-common-name-in-subjectaltname
https://bugzilla.mozilla.org/show_bug.cgi?id=238142
Chrome seem to follow them, so the major open-source browsers are
ignoring the Common Name if the SubjectAltName is present.
Still, I see no win in ignoring CN (except for the shorter code), it
just annoys some users that forgot to put the CN name also in the SAN
section, so my 5 cents is that we should check both.
Regards,
--
Alexey Klyukin
Attachment | Content-Type | Size |
---|---|---|
san_ssl_v7.diff | text/plain | 11.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Arthur Silva | 2014-09-11 17:54:36 | Re: Memory Alignment in Postgres |
Previous Message | Andres Freund | 2014-09-11 17:17:42 | Re: [REVIEW] Re: Compression of full-page-writes |