Bug#887637: rsyslog-gnutls: TLS server does not send intermediate certificates, breaking verification

[Rainer Gerhards]

Arne,

Rainer from upstream here. Thanks for the patch. I will see that I can
integrate it into upstream source.

Question, due to GDPR: is it OK for you to be set as the author inside
git? If so, that means we can not remove your personal information at
a later time as this would break the master branch. If you do not like
this, I can commit anonymously.

More info available here:
https://github.com/rsyslog/rsyslog/blob/master/CONTRIBUTING.md

Rainer

2018-01-22 11:25 GMT+01:00 Arne Nordmark :
> On Thu, 18 Jan 2018 16:27:35 +0100 Arne Nordmark 
> wrote:
>>
>> gtlsLoadOurCertKey() uses gnutls_x509_crt_import() on the file data, and
>> this function only handles one cert.
>>
>
> If one uses gnutls_x509_crt_list_import() instead, intermediate certs could
> be supported. With the attached patch,
> the server sends all certificates in the file.
>
> Arne
>
>
>

Bug#887637: rsyslog-gnutls: TLS server does not send intermediate certificates, breaking verification

[Michael Biebl]

Hi Arne

On Mon, 22 Jan 2018 11:25:42 +0100 Arne Nordmark 
wrote:
> On Thu, 18 Jan 2018 16:27:35 +0100 Arne Nordmark  
> wrote:
>  >
>  > gtlsLoadOurCertKey() uses gnutls_x509_crt_import() on the file data, 
> and this function only handles one cert.
>  >
> 
> If one uses gnutls_x509_crt_list_import() instead, intermediate certs 
> could be supported. With the attached patch,
> the server sends all certificates in the file.
> 

Would be great, if you can test the latest upstream version currently
availabe in Debian and if this is still an issue, report this upstream at
https://github.com/rsyslog/rsyslog/issues

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#887637: rsyslog-gnutls: TLS server does not send intermediate certificates, breaking verification

[Arne Nordmark]

On Thu, 18 Jan 2018 16:27:35 +0100 Arne Nordmark  
wrote:

>
> gtlsLoadOurCertKey() uses gnutls_x509_crt_import() on the file data, 
and this function only handles one cert.

>

If one uses gnutls_x509_crt_list_import() instead, intermediate certs 
could be supported. With the attached patch,

the server sends all certificates in the file.

Arne



--- a/runtime/nsd_gtls.c
+++ b/runtime/nsd_gtls.c
@@ -173,6 +173,7 @@
 	gnutls_datum_t data = { NULL, 0 };
 	uchar *keyFile;
 	uchar *certFile;
+	int lenRcvd;
 
 	ISOBJ_TYPE_assert(pThis, nsd_gtls);
 
@@ -192,9 +193,12 @@
 
 	/* try load certificate */
 	CHKiRet(readFile(certFile, ));
-	CHKgnutls(gnutls_x509_crt_init(>ourCert));
+	pThis->nOurCerts=sizeof(pThis->pOurCerts);
+	lenRcvd=gnutls_x509_crt_list_import(pThis->pOurCerts, >nOurCerts, , GNUTLS_X509_FMT_PEM,0);
+	if (lenRcvd<0) {
+		CHKgnutls(lenRcvd);
+	}
 	pThis->bOurCertIsInit = 1;
-	CHKgnutls(gnutls_x509_crt_import(pThis->ourCert, , GNUTLS_X509_FMT_PEM));
 	free(data.data);
 	data.data = NULL;
 
@@ -210,7 +214,9 @@
 		if(data.data != NULL)
 			free(data.data);
 		if(pThis->bOurCertIsInit) {
-			gnutls_x509_crt_deinit(pThis->ourCert);
+			for (int i=0; inOurCerts; ++i) {
+gnutls_x509_crt_deinit(pThis->pOurCerts[i]);
+			}
 			pThis->bOurCertIsInit = 0;
 		}
 		if(pThis->bOurKeyIsInit) {
@@ -255,8 +261,8 @@
 #else
 	st->type = GNUTLS_CRT_X509;
 #endif
-	st->ncerts = 1;
-	st->cert.x509 = >ourCert;
+	st->ncerts = pThis->nOurCerts;
+	st->cert.x509 = pThis->pOurCerts;
 	st->key.x509 = pThis->ourKey;
 	st->deinit_all = 0;
 
@@ -1204,7 +1210,9 @@
 	}
 
 	if(pThis->bOurCertIsInit)
-		gnutls_x509_crt_deinit(pThis->ourCert);
+  for (int i=0; inOurCerts; ++i) {
+			gnutls_x509_crt_deinit(pThis->pOurCerts[i]);
+  }
 	if(pThis->bOurKeyIsInit)
 		gnutls_x509_privkey_deinit(pThis->ourKey);
 	if(pThis->bHaveSess)
--- a/runtime/nsd_gtls.h
+++ b/runtime/nsd_gtls.h
@@ -25,6 +25,7 @@
 #include "nsd.h"
 
 #define NSD_GTLS_MAX_RCVBUF 8 * 1024 /* max size of buffer for message reception */
+#define NSD_GTLS_MAX_CERT 10 /* max number of certs in our chain */
 
 typedef enum {
 	gtlsRtry_None = 0,	/**< no call needs to be retried */
@@ -56,7 +57,8 @@
  * set to 1 and changed to 0 after the first report. It is changed back to 1 after
  * one successful authentication. */
 	permittedPeers_t *pPermPeers; /* permitted peers */
-	gnutls_x509_crt_t ourCert;	/**< our certificate, if in client mode (unused in server mode) */
+	gnutls_x509_crt_t pOurCerts[NSD_GTLS_MAX_CERT];	/**< our certificate, if in client mode (unused in server mode) */
+	unsigned int nOurCerts;  /* number of certificates in our chain */
 	gnutls_x509_privkey_t ourKey;	/**< our private key, if in client mode (unused in server mode) */
 	short	bOurCertIsInit;	/**< 1 if our certificate is initialized and must be deinit on destruction */
 	short	bOurKeyIsInit;	/**< 1 if our private key is initialized and must be deinit on destruction */

Bug#887637: rsyslog-gnutls: TLS server does not send intermediate certificates, breaking verification

[Arne Nordmark]

Package: rsyslog-gnutls
Version: 8.24.0-1
Severity: normal

The setup consists of a TLS-enabled rsyslog server and TLS-enbled rsyslog 
clients without using client certificate authentication.

When DefaultNetstreamDriverCertFile on the server specifies a file with a 
single cert (which is signed by a top level cert available to the clients),
clients can connect.

When DefaultNetstreamDriverCertFile on the server specifies a file with a cert 
followed by an intermediate cert (which is signed by a top level cert available 
to the clients),
clients fail to connect.

Using "openssl s_client" reveals that only the server cert is sent, not the 
intermediate cert, and thus clients will fail
server cert verification since the intermediate certificate is not available.

The relevant code is in runtime/nsd_gtls.c. Interestingly enough there are two 
separate functions that read the certificate:

gtlsAddOurCert() uses gnutls_certificate_set_x509_key_file(), which will handle 
intermediate certs correctly.

gtlsLoadOurCertKey() uses gnutls_x509_crt_import() on the file data, and this 
function only handles one cert.

The later function seems meant to be used in clients to read the client 
certificate when using client authentication,
but is also called in gtlsInitSession(). If one changes gtlsInitSession() to 
read
#if HAVE_GNUTLS_CERTIFICATE_SET_RETRIEVE_FUNCTION && 0
thus disabling the call to gtlsLoadOurCertKey(),
the server will present the intermediate cert and clients will be able to 
connect.

Arne

-- System Information:
Debian Release: 9.3
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.9.0-5-amd64 (SMP w/4 CPU cores)
Locale: LANG=sv_SE.UTF-8, LC_CTYPE=sv_SE.UTF-8 (charmap=UTF-8), 
LANGUAGE=sv_SE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages rsyslog-gnutls depends on:
ii  libc62.24-11+deb9u1
ii  libgnutls30  3.5.8-5+deb9u3
ii  rsyslog  8.24.0-1

rsyslog-gnutls recommends no packages.

Versions of packages rsyslog-gnutls suggests:
ii  gnutls-bin  3.5.8-5+deb9u3

-- no debconf information