On Mon, Aug 25, 2014 at 12:02 PM, Heikki Linnakangas <
[email protected]> wrote:
> On 08/24/2014 03:11 PM, Alexey Klyukin wrote:
>
>> On Wed, Aug 20, 2014 at 11:53 AM, Heikki Linnakangas <
>> [email protected]> wrote:
>>
>>>
>>> The patch doesn't seem to support wildcards in alternative names. Is
> that on purpose?
>
Not really, we just did not have any use case for them, but it seems that
RFC 5280 does not disallow them:
" Finally, the semantics of subject alternative names that include
wildcard characters (e.g., as a placeholder for a set of names) are
not addressed by this specification. Applications with specific
requirements MAY use such names, but they must define the semantics."
I've added support for them in the next iteration of the patch attached to
this email.
>
> It would be good to add a little helper function that does the NULL-check,
> straight comparison, and wildcard check, for a single name. And then use
> that for the Common Name and all the Alternatives. That'll ensure that all
> the same rules apply whether the name is the Common Name or an Alternative
> (assuming that the rules are supposed to be the same; I don't know if
> that's true).
>
Thanks, common code has been moved into a separate new function.
Another question is how should we treat the certificates with no CN and
non-empty SAN?
Current code just bails out right after finding no CN section present , but
the RFC (5280) says:
"
Further, if the only subject identity included in the certificate is
an alternative name form (e.g., an electronic mail address), then the
subject distinguished name MUST be empty (an empty sequence), and the
subjectAltName extension MUST be present.
"
which to me sounds like the possibility of coming across such certificates
in the wild, although I personally see little use in them.
Regards,
--
Alexey Klyukin
diff --git a/src/interfaces/libpq/fe-secure-openssl.c
b/src/interfaces/libpq/fe-secure-openssl.c
new file mode 100644
index f950fc3..394a66f
*** a/src/interfaces/libpq/fe-secure-openssl.c
--- b/src/interfaces/libpq/fe-secure-openssl.c
***************
*** 60,68 ****
--- 60,73 ----
#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);
+ static int certificate_name_entry_validate_match(PGconn *conn,
+
char *name,
+
unsigned int len,
+
bool *match);
static void destroy_ssl_system(void);
static int initialize_SSL(PGconn *conn);
static PostgresPollingStatusType open_client_SSL(PGconn *);
*************** wildcard_certificate_match(const char *p
*** 471,479 ****
return 1;
}
/*
! * Verify that common name resolves to peer.
*/
static bool
verify_peer_name_matches_certificate(PGconn *conn)
--- 476,515 ----
return 1;
}
+ /*
+ * Validate a single certificate name entry and match it against the pghost.
+ * Returns 0 if the certificate name is invalid (contains embedded NULLs), 1
otherwise.
+ */
+ static int
+ certificate_name_entry_validate_match(PGconn *conn, char *name, unsigned int
len, bool *match)
+ {
+ /* There is no guarantee the string returned from the certificate is
NULL-terminated */
+ name[len] = '\0';
+ *match = false;
+ /*
+ * Reject embedded NULLs in certificate common or alternative name to
prevent attacks
+ * like CVE-2009-4034.
+ */
+ if (len != strlen(name))
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("SSL certificate's
common name contains embedded null\n"));
+ return 0;
+ }
+ if (pg_strcasecmp(name, conn->pghost) == 0)
+ /* Exact name match */
+ *match = true;
+ else if (wildcard_certificate_match(name, conn->pghost))
+ /* Matched wildcard certificate */
+ *match = true;
+ else
+ *match = false;
+ return 1;
+ }
+
/*
! * 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),
--- 528,533 ----
*************** verify_peer_name_matches_certificate(PGc
*** 522,540 ****
free(peer_cn);
return false;
}
- peer_cn[len] = '\0';
-
- /*
- * Reject embedded NULLs in certificate common name to prevent attacks
- * like CVE-2009-4034.
- */
- if (len != strlen(peer_cn))
- {
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL
certificate's common name contains embedded null\n"));
- free(peer_cn);
- return false;
- }
/*
* We got the peer's common name. Now compare it against the originally
--- 556,561 ----
*************** verify_peer_name_matches_certificate(PGc
*** 546,566 ****
libpq_gettext("host name must
be specified for a verified SSL connection\n"));
result = false;
}
! else
{
! if (pg_strcasecmp(peer_cn, conn->pghost) == 0)
! /* Exact name match */
! result = true;
! else if (wildcard_certificate_match(peer_cn, conn->pghost))
! /* Matched wildcard certificate */
! 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;
}
}
free(peer_cn);
--- 567,627 ----
libpq_gettext("host name must
be specified for a verified SSL connection\n"));
result = false;
}
! if (certificate_name_entry_validate_match(conn, peer_cn, len, &result)
== 0)
{
! free(peer_cn);
! return false;
! }
! else if (!result)
! {
! int i;
! int san_len;
! STACK_OF(GENERAL_NAME) *peer_san;
!
! /* Get the list and the total number of subject alternative
names (SANs). */
! peer_san = (STACK_OF(GENERAL_NAME) *)
X509_get_ext_d2i(conn->peer, NID_subject_alt_name, NULL, NULL);
! san_len = peer_san ? sk_GENERAL_NAME_num(peer_san) : 0;
!
! /*
! * Compare the alternative names dnsNames identifies against
! * the originally given hostname.
! */
! for (i = 0; i < san_len; i++)
! {
! const GENERAL_NAME *name =
sk_GENERAL_NAME_value(peer_san, i);
! if (name->type == GEN_DNS)
! {
! int reported_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);
! if (certificate_name_entry_validate_match(conn,
dns_name, reported_len, &result) == 0)
! {
! free(peer_cn);
! free(dns_name);
! sk_GENERAL_NAMES_free(peer_san);
! return false;
! }
! free(dns_name);
! }
! if (result)
! break;
! }
! 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);
}
+ if (peer_san != NULL)
+ sk_GENERAL_NAMES_free(peer_san);
}
free(peer_cn);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
