mirror of
https://github.com/postgres/postgres.git
synced 2025-11-10 17:42:29 +03:00
Don't reflect unescaped cert data to the logs
Commit3a0e385048introduced a new path for unauthenticated bytes from the client certificate to be printed unescaped to the logs. There are a handful of these already, but it doesn't make sense to keep making the problem worse. \x-escape any unprintable bytes. The test case introduces a revoked UTF-8 certificate. This requires the addition of the `-utf8` flag to `openssl req`. Since the existing certificates all use an ASCII subset, this won't modify the existing certificates' subjects if/when they get regenerated; this was verified experimentally with $ make sslfiles-clean $ make sslfiles Unfortunately the test can't be run in the CI yet due to a test timing issue; see55828a6b60. Author: Jacob Champion <jchampion@timescale.com> Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w@mail.gmail.com
This commit is contained in:
@@ -27,12 +27,14 @@
|
||||
#include <netinet/tcp.h>
|
||||
#include <arpa/inet.h>
|
||||
|
||||
#include "common/string.h"
|
||||
#include "libpq/libpq.h"
|
||||
#include "miscadmin.h"
|
||||
#include "pgstat.h"
|
||||
#include "storage/fd.h"
|
||||
#include "storage/latch.h"
|
||||
#include "tcop/tcopprot.h"
|
||||
#include "utils/builtins.h"
|
||||
#include "utils/memutils.h"
|
||||
|
||||
/*
|
||||
@@ -1080,16 +1082,16 @@ dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata)
|
||||
}
|
||||
|
||||
/*
|
||||
* Examines the provided certificate name, and if it's too long to log, modifies
|
||||
* and truncates it. The return value is NULL if no truncation was needed; it
|
||||
* otherwise points into the middle of the input string, and should not be
|
||||
* freed.
|
||||
* Examines the provided certificate name, and if it's too long to log or
|
||||
* contains unprintable ASCII, escapes and truncates it. The return value is
|
||||
* always a new palloc'd string. (The input string is still modified in place,
|
||||
* for ease of implementation.)
|
||||
*/
|
||||
static char *
|
||||
truncate_cert_name(char *name)
|
||||
prepare_cert_name(char *name)
|
||||
{
|
||||
size_t namelen = strlen(name);
|
||||
char *truncated;
|
||||
char *truncated = name;
|
||||
|
||||
/*
|
||||
* Common Names are 64 chars max, so for a common case where the CN is the
|
||||
@@ -1099,19 +1101,20 @@ truncate_cert_name(char *name)
|
||||
*/
|
||||
#define MAXLEN 71
|
||||
|
||||
if (namelen <= MAXLEN)
|
||||
return NULL;
|
||||
|
||||
/*
|
||||
* Keep the end of the name, not the beginning, since the most specific
|
||||
* field is likely to give users the most information.
|
||||
*/
|
||||
truncated = name + namelen - MAXLEN;
|
||||
truncated[0] = truncated[1] = truncated[2] = '.';
|
||||
if (namelen > MAXLEN)
|
||||
{
|
||||
/*
|
||||
* Keep the end of the name, not the beginning, since the most specific
|
||||
* field is likely to give users the most information.
|
||||
*/
|
||||
truncated = name + namelen - MAXLEN;
|
||||
truncated[0] = truncated[1] = truncated[2] = '.';
|
||||
namelen = MAXLEN;
|
||||
}
|
||||
|
||||
#undef MAXLEN
|
||||
|
||||
return truncated;
|
||||
return pg_clean_ascii(truncated, 0);
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -1154,21 +1157,24 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
|
||||
{
|
||||
char *subject,
|
||||
*issuer;
|
||||
char *sub_truncated,
|
||||
*iss_truncated;
|
||||
char *sub_prepared,
|
||||
*iss_prepared;
|
||||
char *serialno;
|
||||
ASN1_INTEGER *sn;
|
||||
BIGNUM *b;
|
||||
|
||||
/*
|
||||
* Get the Subject and Issuer for logging, but don't let maliciously
|
||||
* huge certs flood the logs.
|
||||
* huge certs flood the logs, and don't reflect non-ASCII bytes into it
|
||||
* either.
|
||||
*/
|
||||
subject = X509_NAME_to_cstring(X509_get_subject_name(cert));
|
||||
sub_truncated = truncate_cert_name(subject);
|
||||
sub_prepared = prepare_cert_name(subject);
|
||||
pfree(subject);
|
||||
|
||||
issuer = X509_NAME_to_cstring(X509_get_issuer_name(cert));
|
||||
iss_truncated = truncate_cert_name(issuer);
|
||||
iss_prepared = prepare_cert_name(issuer);
|
||||
pfree(issuer);
|
||||
|
||||
/*
|
||||
* Pull the serial number, too, in case a Subject is still ambiguous.
|
||||
@@ -1181,14 +1187,13 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
|
||||
appendStringInfoChar(&str, '\n');
|
||||
appendStringInfo(&str,
|
||||
_("Failed certificate data (unverified): subject \"%s\", serial number %s, issuer \"%s\"."),
|
||||
sub_truncated ? sub_truncated : subject,
|
||||
serialno ? serialno : _("unknown"),
|
||||
iss_truncated ? iss_truncated : issuer);
|
||||
sub_prepared, serialno ? serialno : _("unknown"),
|
||||
iss_prepared);
|
||||
|
||||
BN_free(b);
|
||||
OPENSSL_free(serialno);
|
||||
pfree(issuer);
|
||||
pfree(subject);
|
||||
pfree(iss_prepared);
|
||||
pfree(sub_prepared);
|
||||
}
|
||||
|
||||
/* Store our detail message to be logged later. */
|
||||
|
||||
Reference in New Issue
Block a user