On Wed, Aug 20, 2014 at 11:53 AM, Heikki Linnakangas <
[email protected]> wrote:
> On 07/25/2014 07:10 PM, Alexey Klyukin wrote:
>
>> Greetings,
>>
>> I'd like to propose a patch for checking subject alternative names entry
>> in
>> the SSL certificate for DNS names during SSL authentication.
>>
>
> Thanks! I just ran into this missing feature last week, while working on
> my SSL test suite. So +1 for having the feature.
>
> This patch needs to be rebased over current master branch, thanks to my
> refactoring that moved all OpenSSL-specific stuff to be-secure-openssl.c.
The patch is rebased against fe-secure-openssl.c (that's where
verify_peer_name_matches_certificate appeared in the master branch), I've
changed the condition in the for loop to be less confusing (thanks to
comments from Magnus and Tom), making an explicit break once a match is
detected.
Note that It generates a lot of OpenSSL related warnings on my system (66
total) with clang, complaining about
$X is deprecated: first deprecated in OS X 10.7
[-Wdeprecated-declarations], but it does so for most other SSL functions,
so I don't think it's a problem introduced by this patch.
Sincerely,
Alexey.
diff --git a/src/interfaces/libpq/fe-secure-openssl.c
b/src/interfaces/libpq/fe-secure-openssl.c
new file mode 100644
index f950fc3..b4f6bc9
*** a/src/interfaces/libpq/fe-secure-openssl.c
--- b/src/interfaces/libpq/fe-secure-openssl.c
***************
*** 60,65 ****
--- 60,66 ----
#ifdef USE_SSL_ENGINE
#include <openssl/engine.h>
#endif
+ #include <openssl/x509v3.h>
static bool verify_peer_name_matches_certificate(PGconn *);
static int verify_cb(int ok, X509_STORE_CTX *ctx);
*************** wildcard_certificate_match(const char *p
*** 473,479 ****
/*
! * Verify that common name resolves to peer.
*/
static bool
verify_peer_name_matches_certificate(PGconn *conn)
--- 474,480 ----
/*
! * Verify that common name or any of the alternative dNSNames resolves to
peer.
*/
static bool
verify_peer_name_matches_certificate(PGconn *conn)
*************** verify_peer_name_matches_certificate(PGc
*** 492,499 ****
/*
* Extract the common name from the certificate.
- *
- * XXX: Should support alternate names here
*/
/* First find out the name's length and allocate a buffer for it. */
len = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
--- 493,498 ----
*************** verify_peer_name_matches_certificate(PGc
*** 556,565 ****
result = true;
else
{
printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("server
common name \"%s\" does not match host name \"%s\"\n"),
peer_cn,
conn->pghost);
- result = false;
}
}
--- 555,627 ----
result = true;
else
{
+ int i;
+ int alt_names_total;
+ STACK_OF(GENERAL_NAME) *alt_names;
+
+ /* Get the list and the total number of alternative
names. */
+ alt_names = (STACK_OF(GENERAL_NAME) *)
X509_get_ext_d2i(conn->peer, NID_subject_alt_name, NULL, NULL);
+ if (alt_names != NULL)
+ alt_names_total =
sk_GENERAL_NAME_num(alt_names);
+ else
+ alt_names_total = 0;
+
+ result = false;
+
+ /*
+ * Compare the alternative names dnsNames identifies
against
+ * the originally given hostname.
+ */
+ for (i = 0; i < alt_names_total; i++)
+ {
+ const GENERAL_NAME *name =
sk_GENERAL_NAME_value(alt_names, i);
+ if (name->type == GEN_DNS)
+ {
+ int reported_len;
+ int actual_len;
+ char *dns_namedata,
+ *dns_name;
+
+ reported_len =
ASN1_STRING_length(name->d.dNSName);
+ /* GEN_DNS can be only IA5String,
equivalent to US ASCII */
+ dns_namedata = (char *)
ASN1_STRING_data(name->d.dNSName);
+
+ dns_name = malloc(reported_len + 1);
+ memcpy(dns_name, dns_namedata,
reported_len);
+ dns_name[reported_len] = '\0';
+
+ actual_len = strlen(dns_name);
+ if (actual_len != reported_len)
+ {
+ /*
+ * Reject embedded NULLs in
certificate alternative name to prevent attacks
+ * like CVE-2009-4034.
+ */
+
printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("SSL
certificate's alternative name contains embedded null\n"));
+ free(peer_cn);
+ free(dns_name);
+
sk_GENERAL_NAMES_free(alt_names);
+ return false;
+ }
+ if (pg_strcasecmp(dns_name,
conn->pghost) == 0)
+ result = true;
+
+ free(dns_name);
+ }
+ if (result)
+ break;
+ }
+ if (alt_names != NULL)
+ sk_GENERAL_NAMES_free(alt_names);
+ }
+
+ if (!result)
+ {
+ /* Common name did not match and there are no
alternative names */
printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("server
common name \"%s\" and alternatives do not match host name \"%s\"\n"),
peer_cn,
conn->pghost);
}
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
