Merge pull request #4692 from alorbach/pr-issue-4686

openssl: Correct gnutlsPriorityString (custom ciphers) behaviour
2025-12-15 10:30:40 +01:00 · 2021-10-11 15:00:24 +02:00 · 2021-10-11 15:00:24 +02:00 · ed512aa18a
commit ed512aa18a
parent d2a8c5be8b edc1b186c1
5 changed files with 197 additions and 105 deletions
--- a/runtime/nsd_ossl.c
+++ b/runtime/nsd_ossl.c
@ -426,36 +426,6 @@ osslGlblInit(void)
 	RETiRet;
 }

-/* Init Anon OpenSSL helpers */
-#if 0 // TODO: DELETE!
-static rsRetVal
-osslAnonInit(void)
-{
-	DEFiRet;
-	pthread_mutex_lock(&anonInit_mut);
-	if (bAnonInit == 1) {
-		/* we are done */
-		FINALIZE;
-	}
-	dbgprintf("osslAnonInit Init Anon OpenSSL helpers\n");
-
-	#if OPENSSL_VERSION_NUMBER >= 0x10002000L
-	/* Enable Support for automatic EC temporary key parameter selection. */
-	SSL_CTX_set_ecdh_auto(ctx, 1);
-	#else
-	dbgprintf("osslAnonInit: openssl to old, cannot use SSL_CTX_set_ecdh_auto."
-		"Using SSL_CTX_set_tmp_ecdh with NID_X9_62_prime256v1/() instead.\n");
-	SSL_CTX_set_tmp_ecdh(ctx, EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
-	#endif
-
-	bAnonInit = 1;
-finalize_it:
-	pthread_mutex_unlock(&anonInit_mut);
-	RETiRet;
-}
-#endif
-
-
 /* try to receive a record from the remote peer. This works with
 * our own abstraction and handles local buffering and EAGAIN.
 * See details on local buffering in Rcv(9 header-comment.
@ -540,10 +510,10 @@ finalize_it:
 }

 static rsRetVal
-osslInitSession(nsd_ossl_t *pThis) /* , nsd_ossl_t *pServer) */
+osslInitSession(nsd_ossl_t *pThis, osslSslState_t osslType) /* , nsd_ossl_t *pServer) */
 {
 	DEFiRet;
-	BIO *client;
+	BIO *conn;
 	char pristringBuf[4096];
 	nsd_ptcp_t *pPtcp = (nsd_ptcp_t*) pThis->pTcp;

@ -564,10 +534,8 @@ osslInitSession(nsd_ossl_t *pThis) /* , nsd_ossl_t *pServer) */
 		if (pThis->DrvrVerifyDepth != 0) {
 			SSL_set_verify_depth(pThis->ssl, pThis->DrvrVerifyDepth);
 		}
-	}
-
-	if (bAnonInit == 1) { /* no mutex needed, read-only after init */
-		/* Allow ANON Ciphers */
+	} else 	if (bAnonInit == 1 && pThis->gnutlsPriorityString == NULL) {
+		/* Allow ANON Ciphers only in ANON Mode and if no custom priority string is defined */
 		#if OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
 		 /* NOTE: do never use: +eNULL, it DISABLES encryption! */
 		strncpy(pristringBuf, "ALL:+COMPLEMENTOFDEFAULT:+ADH:+ECDH:+aNULL@SECLEVEL=0",
@ -585,20 +553,26 @@ osslInitSession(nsd_ossl_t *pThis) /* , nsd_ossl_t *pServer) */
 	}

 	/* Create BIO from ptcp socket! */
-	client = BIO_new_socket(pPtcp->sock, BIO_CLOSE /*BIO_NOCLOSE*/);
-	dbgprintf("osslInitSession: Init client BIO[%p] done\n", (void *)client);
+	conn = BIO_new_socket(pPtcp->sock, BIO_CLOSE /*BIO_NOCLOSE*/);
+	dbgprintf("osslInitSession: Init conn BIO[%p] done\n", (void *)conn);

-	/* Set debug Callback for client BIO as well! */
-	BIO_set_callback(client, BIO_debug_callback);
+	/* Set debug Callback for conn BIO as well! */
+	BIO_set_callback(conn, BIO_debug_callback);

-/* TODO: still needed? Set to NON blocking ! */
-BIO_set_nbio( client, 1 );
-
-	SSL_set_bio(pThis->ssl, client, client);
-	SSL_set_accept_state(pThis->ssl); /* sets ssl to work in server mode. */
+	/* TODO: still needed? Set to NON blocking ! */
+	BIO_set_nbio( conn, 1 );
+	SSL_set_bio(pThis->ssl, conn, conn);

+	if (osslType == osslServer) {
+		/* Server Socket */
+		SSL_set_accept_state(pThis->ssl); /* sets ssl to work in server mode. */
+		pThis->sslState = osslServer; /*set Server state */
+	} else {
+		/* Client Socket */
+		SSL_set_connect_state(pThis->ssl); /*sets ssl to work in client mode.*/
+		pThis->sslState = osslClient; /*set Client state */
+	}
 	pThis->bHaveSess = 1;
-	pThis->sslState = osslServer; /*set Server state */

 	/* we are done */
 	FINALIZE;
@ -1445,8 +1419,9 @@ osslPostHandshakeCheck(nsd_ossl_t *pNsd)

 	#if OPENSSL_VERSION_NUMBER >= 0x10002000L
 	if(SSL_get_shared_curve(pNsd->ssl, -1) == 0) {
-		LogError(0, RS_RET_NO_ERRCODE, "nsd_ossl:"
-		"No shared curve between syslog client and server.");
+		// This is not a failure
+		LogMsg(0, RS_RET_NO_ERRCODE, LOG_INFO, "nsd_ossl: "
+		"Information, no shared curve between syslog client and server");
 	}
 	#endif
 	sslCipher = (const SSL_CIPHER*) SSL_get_current_cipher(pNsd->ssl);
@ -1585,7 +1560,7 @@ AcceptConnReq(nsd_t *pNsd, nsd_t **ppNew)
 	pNew->gnutlsPriorityString = pThis->gnutlsPriorityString;
 	pNew->ctx = pThis->ctx;
 	pNew->ctx_is_copy = 1; // do not free on pNew Destruction
-	CHKiRet(osslInitSession(pNew));
+	CHKiRet(osslInitSession(pNew, osslServer));

 	/* Store nsd_ossl_t* reference in SSL obj */
 	SSL_set_ex_data(pNew->ssl, 0, pThis);
@ -1796,9 +1771,6 @@ Connect(nsd_t *pNsd, int family, uchar *port, uchar *host, char *device)
 	DEFiRet;
 	DBGPRINTF("openssl: entering Connect family=%d, device=%s\n", family, device);
 	nsd_ossl_t* pThis = (nsd_ossl_t*) pNsd;
-	nsd_ptcp_t* pPtcp = (nsd_ptcp_t*) pThis->pTcp;
-	BIO *conn;
-	char pristringBuf[4096];

 	ISOBJ_TYPE_assert(pThis, nsd_ossl);
 	assert(port != NULL);
@ -1813,61 +1785,13 @@ Connect(nsd_t *pNsd, int family, uchar *port, uchar *host, char *device)
 		FINALIZE;
 	}

-	/* Create BIO from ptcp socket! */
-	conn = BIO_new_socket(pPtcp->sock, BIO_CLOSE /*BIO_NOCLOSE*/);
-	dbgprintf("Connect: Init conn BIO[%p] done\n", (void *)conn);
-
 	LogMsg(0, RS_RET_NO_ERRCODE, LOG_INFO, "nsd_ossl: "
 		"TLS Connection initiated with remote syslog server.");
 	/*if we reach this point we are in tls mode */
 	DBGPRINTF("Connect: TLS Mode\n");
-	if(!(pThis->ssl = SSL_new(pThis->ctx))) {
-		pThis->ssl = NULL;
-		osslLastSSLErrorMsg(0, pThis->ssl, LOG_ERR, "Connect");
-		ABORT_FINALIZE(RS_RET_NO_ERRCODE);
-	}

-	// Set SSL_MODE_AUTO_RETRY to SSL obj
-	SSL_set_mode(pThis->ssl, SSL_MODE_AUTO_RETRY);
-
-	if (pThis->authMode != OSSL_AUTH_CERTANON) {
-		dbgprintf("Connect: enable certificate checking (Mode=%d, VerifyDepth=%d)\n",
-			pThis->authMode, pThis->DrvrVerifyDepth);
-		/* Enable certificate valid checking */
-		SSL_set_verify(pThis->ssl, SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, verify_callback);
-		if (pThis->DrvrVerifyDepth != 0) {
-			SSL_set_verify_depth(pThis->ssl, pThis->DrvrVerifyDepth);
-		}
-	}
-
-	if (bAnonInit == 1) { /* no mutex needed, read-only after init */
-		/* Allow ANON Ciphers */
-		#if OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
-		 /* NOTE: do never use: +eNULL, it DISABLES encryption! */
-		strncpy(pristringBuf, "ALL:+COMPLEMENTOFDEFAULT:+ADH:+ECDH:+aNULL@SECLEVEL=0",
-			sizeof(pristringBuf));
-		#else
-		strncpy(pristringBuf, "ALL:+COMPLEMENTOFDEFAULT:+ADH:+ECDH:+aNULL",
-			sizeof(pristringBuf));
-		#endif
-
-		dbgprintf("Connect: setting anon ciphers: %s\n", pristringBuf);
-		if ( SSL_set_cipher_list(pThis->ssl, pristringBuf) == 0 ){
-			dbgprintf("Connect: Error setting ciphers '%s'\n", pristringBuf);
-			ABORT_FINALIZE(RS_RET_SYS_ERR);
-		}
-	}
-
-	/* Set debug Callback for client BIO as well! */
-	BIO_set_callback(conn, BIO_debug_callback);
-
-/* TODO: still needed? Set to NON blocking ! */
-BIO_set_nbio( conn, 1 );
-
-	SSL_set_bio(pThis->ssl, conn, conn);
-	SSL_set_connect_state(pThis->ssl); /*sets ssl to work in client mode.*/
-	pThis->sslState = osslClient; /*set Client state */
-	pThis->bHaveSess = 1;
+	/* Do SSL Session init */
+	CHKiRet(osslInitSession(pThis, osslClient));

 	/* Store nsd_ossl_t* reference in SSL obj */
 	SSL_set_ex_data(pThis->ssl, 0, pThis);
@ -1896,9 +1820,23 @@ SetGnutlsPriorityString(nsd_t *const pNsd, uchar *const gnutlsPriorityString)
 	ISOBJ_TYPE_assert(pThis, nsd_ossl);

 #if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)
+	sbool ApplySettings = 0;
+	if (	(gnutlsPriorityString != NULL && pThis->gnutlsPriorityString == NULL) ||
+		(gnutlsPriorityString != NULL &&
+		strcmp( (const char*)pThis->gnutlsPriorityString, (const char*)gnutlsPriorityString) != 0)
+		) {
+		ApplySettings = 1;
+	}
+
 	pThis->gnutlsPriorityString = gnutlsPriorityString;
-	dbgprintf("gnutlsPriorityString: set to '%s'\n",
-		(gnutlsPriorityString != NULL ? (char*)gnutlsPriorityString : "NULL"));
+	dbgprintf("gnutlsPriorityString: set to '%s' Apply %s\n",
+		(gnutlsPriorityString != NULL ? (char*)gnutlsPriorityString : "NULL"),
+		(ApplySettings == 1? "TRUE" : "FALSE"));
+	if (ApplySettings == 1) {
+		/* Apply Settings if necessary */
+		applyGnutlsPriorityString(pThis);
+	}
+
 #else
 	LogError(0, RS_RET_SYS_ERR, "Warning: TLS library does not support SSL_CONF_cmd API"
 		"(maybe it is too old?). Cannot use gnutlsPriorityString ('%s'). For more see: "
@ -1920,7 +1858,7 @@ applyGnutlsPriorityString(nsd_ossl_t *const pThis)
 	 * generated during SetGntuTLSPriorityString().
 	 */

-	if(pThis->gnutlsPriorityString == NULL) {
+	if(pThis->gnutlsPriorityString == NULL || pThis->ctx == NULL) {
 		FINALIZE;
 	} else {
 		dbgprintf("applying gnutlsPriorityString: '%s'\n", pThis->gnutlsPriorityString);
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@ -1284,12 +1284,14 @@ TESTS +=  \
 	sndrcv_tls_ossl_anon_ipv4.sh \
 	sndrcv_tls_ossl_anon_ipv6.sh \
 	sndrcv_tls_ossl_anon_rebind.sh \
+	sndrcv_tls_ossl_anon_ciphers.sh \
 	sndrcv_tls_ossl_serveranon_ossl_clientanon.sh \
 	sndrcv_tls_ossl_servercert_ossl_clientanon.sh \
 	sndrcv_tls_ossl_certvalid.sh \
 	sndrcv_tls_ossl_certvalid_action_level.sh \
 	sndrcv_tls_ossl_certvalid_expired.sh \
 	sndrcv_tls_ossl_certvalid_tlscommand.sh \
+	sndrcv_tls_ossl_certvalid_ciphers.sh \
 	imtcp-tls-ossl-x509valid.sh \
 	imtcp-tls-ossl-x509name.sh \
 	imtcp-tls-ossl-x509fingerprint.sh \
@ -2146,10 +2148,12 @@ EXTRA_DIST= \
 	sndrcv_tls_ossl_anon_ipv4.sh \
 	sndrcv_tls_ossl_anon_ipv6.sh \
 	sndrcv_tls_ossl_anon_rebind.sh \
+	sndrcv_tls_ossl_anon_ciphers.sh \
 	sndrcv_tls_ossl_certvalid.sh \
 	sndrcv_tls_ossl_certvalid_action_level.sh \
 	sndrcv_tls_ossl_certvalid_expired.sh \
 	sndrcv_tls_ossl_certvalid_tlscommand.sh \
+	sndrcv_tls_ossl_certvalid_ciphers.sh \
 	imtcp-tls-ossl-x509valid.sh \
 	imtcp-tls-ossl-x509name.sh \
 	imtcp-tls-ossl-x509fingerprint.sh \
--- a/tests/imtcp-tls-ossl-basic-tlscommands.sh
+++ b/tests/imtcp-tls-ossl-basic-tlscommands.sh
@ -3,6 +3,9 @@
 # This file is part of the rsyslog project, released under ASL 2.0
 . ${srcdir:=.}/diag.sh init
 export NUMMESSAGES=10
+# uncomment for debugging support:
+#export RSYSLOG_DEBUG="debug nostdout noprintmutexaction"
+#export RSYSLOG_DEBUGLOG="$RSYSLOG_DYNNAME.receiver.debuglog"
 generate_conf
 add_conf '
 global(	defaultNetstreamDriverCAFile="'$srcdir/tls-certs/ca.pem'"
--- a/tests/sndrcv_tls_ossl_anon_ciphers.sh
+++ b/tests/sndrcv_tls_ossl_anon_ciphers.sh
@ -0,0 +1,72 @@
+#!/bin/bash
+# This file is part of the rsyslog project, released under ASL 2.0
+. ${srcdir:=.}/diag.sh init
+# start up the instances
+# uncomment for debugging support:
+#export RSYSLOG_DEBUG="debug nostdout noprintmutexaction"
+#export RSYSLOG_DEBUGLOG="$RSYSLOG_DYNNAME.receiver.debuglog"
+generate_conf
+add_conf '
+global(	
+	defaultNetstreamDriver="ossl"
+	debug.whitelist="on"
+	debug.files=["nsd_ossl.c", "tcpsrv.c", "nsdsel_ossl.c", "nsdpoll_ptcp.c", "dnscache.c"]
+)
+
+module(	load="../plugins/imtcp/.libs/imtcp"
+	StreamDriver.Name="ossl"
+	StreamDriver.Mode="1"
+	StreamDriver.AuthMode="anon"
+	StreamDriver.PermitExpiredCerts="off"
+	gnutlsPriorityString="CipherString=ECDHE-RSA-AES256-SHA384\nCiphersuites=TLS_AES_256_GCM_SHA384"
+	)
+input(	type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port")
+
+action(type="omfile" file="'$RSYSLOG_OUT_LOG'")
+'
+
+startup
+export PORT_RCVR=$TCPFLOOD_PORT # save this, will be rewritten with next config
+export RSYSLOG_DEBUGLOG="$RSYSLOG_DYNNAME.sender.debuglog"
+generate_conf 2
+add_conf '
+global(	
+	defaultNetstreamDriver="ossl"
+)
+
+action(	type="omfwd"
+	protocol="tcp"
+	target="127.0.0.1"
+	port="'$PORT_RCVR'"
+	StreamDriverMode="1"
+	StreamDriverAuthMode="anon"
+	gnutlsPriorityString="CipherString=ECDHE-RSA-AES256-SHA384\nCiphersuites=TLS_AES_128_GCM_SHA256"
+)
+' 2
+startup 2
+
+# now inject the messages into instance 2. It will connect to instance 1,
+# and that instance will record the data.
+injectmsg 0 1
+shutdown_when_empty 2
+wait_shutdown 2
+# now it is time to stop the receiver as well
+shutdown_when_empty
+wait_shutdown
+
+# IMPORTANT: this test will generate many error messages. This is exactly it's
+# intent. So do not think something is wrong. The content_check below checks
+# these error codes.
+
+content_check --check-only "TLS library does not support SSL_CONF_cmd"
+ret=$?
+if [ $ret == 0 ]; then
+	echo "SKIP: TLS library does not support SSL_CONF_cmd"
+	skip_test
+else
+	# Kindly check for a failed session
+	content_check "OpenSSL Error Stack"
+	content_check "no shared cipher"
+fi
+
+exit_test
--- a/tests/sndrcv_tls_ossl_certvalid_ciphers.sh
+++ b/tests/sndrcv_tls_ossl_certvalid_ciphers.sh
@ -0,0 +1,75 @@
+#!/bin/bash
+# This file is part of the rsyslog project, released under ASL 2.0
+. ${srcdir:=.}/diag.sh init
+# start up the instances
+# uncomment for debugging support:
+#export RSYSLOG_DEBUG="debug nostdout noprintmutexaction"
+#export RSYSLOG_DEBUGLOG="$RSYSLOG_DYNNAME.receiver.debuglog"
+generate_conf
+add_conf '
+global(	defaultNetstreamDriverCAFile="'$srcdir/tls-certs/ca.pem'"
+	defaultNetstreamDriverCertFile="'$srcdir/tls-certs/cert.pem'"
+	defaultNetstreamDriverKeyFile="'$srcdir/tls-certs/key.pem'"
+	defaultNetstreamDriver="ossl"
+	debug.whitelist="on"
+	debug.files=["nsd_ossl.c", "tcpsrv.c", "nsdsel_ossl.c", "nsdpoll_ptcp.c", "dnscache.c"]
+)
+
+module(	load="../plugins/imtcp/.libs/imtcp"
+	StreamDriver.Name="ossl"
+	StreamDriver.Mode="1"
+	StreamDriver.AuthMode="x509/certvalid"
+	StreamDriver.PermitExpiredCerts="off"
+	gnutlsPriorityString="CipherString=ECDHE-RSA-AES256-GCM-SHA384\nCiphersuites=TLS_AES_256_GCM_SHA384"
+	)
+input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port")
+
+action(type="omfile" file="'$RSYSLOG_OUT_LOG'")
+'
+startup
+export PORT_RCVR=$TCPFLOOD_PORT # save this, will be rewritten with next config
+export RSYSLOG_DEBUGLOG="$RSYSLOG_DYNNAME.sender.debuglog"
+generate_conf 2
+add_conf '
+global(	defaultNetstreamDriverCAFile="'$srcdir/tls-certs/ca.pem'"
+	defaultNetstreamDriverCertFile="'$srcdir/tls-certs/cert.pem'"
+	defaultNetstreamDriverKeyFile="'$srcdir/tls-certs/key.pem'"
+	defaultNetstreamDriver="ossl"
+)
+
+action(	type="omfwd"
+	protocol="tcp"
+	target="127.0.0.1"
+	port="'$PORT_RCVR'"
+	StreamDriverMode="1"
+	StreamDriverAuthMode="x509/certvalid"
+	gnutlsPriorityString="CipherString=ECDHE-RSA-AES128-GCM-SHA256\nCiphersuites=TLS_AES_128_GCM_SHA256"
+)
+' 2
+startup 2
+
+# now inject the messages into instance 2. It will connect to instance 1,
+# and that instance will record the data.
+injectmsg 0 1
+shutdown_when_empty 2
+wait_shutdown 2
+# now it is time to stop the receiver as well
+shutdown_when_empty
+wait_shutdown
+
+# IMPORTANT: this test will generate many error messages. This is exactly it's
+# intent. So do not think something is wrong. The content_check below checks
+# these error codes.
+
+content_check --check-only "TLS library does not support SSL_CONF_cmd"
+ret=$?
+if [ $ret == 0 ]; then
+	echo "SKIP: TLS library does not support SSL_CONF_cmd"
+	skip_test
+else
+	# Kindly check for a failed session
+	content_check "OpenSSL Error Stack"
+	content_check "no shared cipher"
+fi
+
+exit_test