public inbox for [email protected]  
help / color / mirror / Atom feed
From: Jacob Champion <[email protected]>
To: [email protected] <[email protected]>
To: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Support for NSS as a libpq TLS backend
Date: Wed, 13 Jan 2021 17:07:53 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CAA8=A7-suZg90M7Fgyo1Rfymkrpdh_Z547RKXrbHirsAmojQSw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Tue, 2020-10-27 at 21:07 +0100, Daniel Gustafsson wrote:
> > On 20 Oct 2020, at 21:15, Andres Freund <[email protected]> wrote:
> > 
> > > +static SECStatus
> > > +pg_cert_auth_handler(void *arg, PRFileDesc * fd, PRBool checksig, PRBool isServer)
> > > +{
> > > +	SECStatus	status;
> > > +	Port	   *port = (Port *) arg;
> > > +	CERTCertificate *cert;
> > > +	char	   *peer_cn;
> > > +	int			len;
> > > +
> > > +	status = SSL_AuthCertificate(CERT_GetDefaultCertDB(), port->pr_fd, checksig, PR_TRUE);
> > > +	if (status == SECSuccess)
> > > +	{
> > > +		cert = SSL_PeerCertificate(port->pr_fd);
> > > +		len = strlen(cert->subjectName);
> > > +		peer_cn = MemoryContextAllocZero(TopMemoryContext, len + 1);
> > > +		if (strncmp(cert->subjectName, "CN=", 3) == 0)
> > > +			strlcpy(peer_cn, cert->subjectName + strlen("CN="), len + 1);
> > > +		else
> > > +			strlcpy(peer_cn, cert->subjectName, len + 1);
> > > +		CERT_DestroyCertificate(cert);
> > > +
> > > +		port->peer_cn = peer_cn;
> > > +		port->peer_cert_valid = true;
> > 
> > Hm. We either should have something similar to
> > 
> > 			/*
> > 			 * Reject embedded NULLs in certificate common name to prevent
> > 			 * attacks like CVE-2009-4034.
> > 			 */
> > 			if (len != strlen(peer_cn))
> > 			{
> > 				ereport(COMMERROR,
> > 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
> > 						 errmsg("SSL certificate's common name contains embedded null")));
> > 				pfree(peer_cn);
> > 				return -1;
> > 			}
> > here, or a comment explaining why not.
> 
> We should, but it's proving rather difficult as there is no equivalent API call
> to get the string as well as the expected length of it.

I'm going to try to tackle this part next. It looks like NSS uses RFC
4514 (or something like it) backslash-quoting, which this code either
needs to undo or bypass before performing a comparison.

--Jacob


view thread (181+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Support for NSS as a libpq TLS backend
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox