aboutsummaryrefslogtreecommitdiff
path: root/src/backend
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2012-02-23 15:48:09 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2012-02-23 15:48:09 -0500
commite6fcb03dc0de1771f7d408b5df1738272e6f98e5 (patch)
treec786e27946eb8c619c9c8098a8539fb3adbdf410 /src/backend
parent54e2b6488bb294691a525a272d6f614ac2046282 (diff)
downloadpostgresql-e6fcb03dc0de1771f7d408b5df1738272e6f98e5.tar.gz
postgresql-e6fcb03dc0de1771f7d408b5df1738272e6f98e5.zip
Remove arbitrary limitation on length of common name in SSL certificates.
Both libpq and the backend would truncate a common name extracted from a certificate at 32 bytes. Replace that fixed-size buffer with dynamically allocated string so that there is no hard limit. While at it, remove the code for extracting peer_dn, which we weren't using for anything; and don't bother to store peer_cn longer than we need it in libpq. This limit was not so terribly unreasonable when the code was written, because we weren't using the result for anything critical, just logging it. But now that there are options for checking the common name against the server host name (in libpq) or using it as the user's name (in the server), this could result in undesirable failures. In the worst case it even seems possible to spoof a server name or user name, if the correct name is exactly 32 bytes and the attacker can persuade a trusted CA to issue a certificate in which that string is a prefix of the certificate's common name. (To exploit this for a server name, he'd also have to send the connection astray via phony DNS data or some such.) The case that this is a realistic security threat is a bit thin, but nonetheless we'll treat it as one. Back-patch to 8.4. Older releases contain the faulty code, but it's not a security problem because the common name wasn't used for anything interesting. Reported and patched by Heikki Linnakangas Security: CVE-2012-0867
Diffstat (limited to 'src/backend')
-rw-r--r--src/backend/libpq/be-secure.c59
1 files changed, 38 insertions, 21 deletions
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 4e8faa4f35c..5d11470cbb6 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -73,6 +73,7 @@
#include "libpq/libpq.h"
#include "tcop/tcopprot.h"
+#include "utils/memutils.h"
#ifdef USE_SSL
@@ -964,44 +965,54 @@ aloop:
port->count = 0;
- /* get client certificate, if available. */
+ /* Get client certificate, if available. */
port->peer = SSL_get_peer_certificate(port->ssl);
- if (port->peer == NULL)
- {
- strlcpy(port->peer_dn, "(anonymous)", sizeof(port->peer_dn));
- strlcpy(port->peer_cn, "(anonymous)", sizeof(port->peer_cn));
- }
- else
+
+ /* and extract the Common Name from it. */
+ port->peer_cn = NULL;
+ if (port->peer != NULL)
{
- X509_NAME_oneline(X509_get_subject_name(port->peer),
- port->peer_dn, sizeof(port->peer_dn));
- port->peer_dn[sizeof(port->peer_dn) - 1] = '\0';
- r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
- NID_commonName, port->peer_cn, sizeof(port->peer_cn));
- port->peer_cn[sizeof(port->peer_cn) - 1] = '\0';
- if (r == -1)
- {
- /* Unable to get the CN, set it to blank so it can't be used */
- port->peer_cn[0] = '\0';
- }
- else
+ int len;
+
+ len = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
+ NID_commonName, NULL, 0);
+ if (len != -1)
{
+ char *peer_cn;
+
+ peer_cn = MemoryContextAlloc(TopMemoryContext, len + 1);
+ r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
+ NID_commonName, peer_cn, len + 1);
+ peer_cn[len] = '\0';
+ if (r != len)
+ {
+ /* shouldn't happen */
+ pfree(peer_cn);
+ close_SSL(port);
+ return -1;
+ }
+
/*
* Reject embedded NULLs in certificate common name to prevent
* attacks like CVE-2009-4034.
*/
- if (r != strlen(port->peer_cn))
+ if (len != strlen(peer_cn))
{
ereport(COMMERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("SSL certificate's common name contains embedded null")));
+ pfree(peer_cn);
close_SSL(port);
return -1;
}
+
+ port->peer_cn = peer_cn;
}
}
+
ereport(DEBUG2,
- (errmsg("SSL connection from \"%s\"", port->peer_cn)));
+ (errmsg("SSL connection from \"%s\"",
+ port->peer_cn ? port->peer_cn : "(anonymous)")));
/* set up debugging/info callback */
SSL_CTX_set_info_callback(SSL_context, info_cb);
@@ -1027,6 +1038,12 @@ close_SSL(Port *port)
X509_free(port->peer);
port->peer = NULL;
}
+
+ if (port->peer_cn)
+ {
+ pfree(port->peer_cn);
+ port->peer_cn = NULL;
+ }
}
/*