From 72817a578c8bace6054c54b0cb7b22272ac9436a Mon Sep 17 00:00:00 2001 From: PascalWithopf Date: Fri, 20 Oct 2017 12:31:12 +0200 Subject: [PATCH 1/3] omrelp/imrelp: fix certificate check When the certificate file specified in the omrelp/imrelp configuration can't be accessed, e.g. because it doesn't exist or you don't have permission to do so, a Segmentation Fault will appear when you start Rsyslog. This commit fixes that problem. closes https://github.com/rsyslog/rsyslog/issues/1869 --- plugins/imrelp/imrelp.c | 9 ++++++--- plugins/omrelp/omrelp.c | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/plugins/imrelp/imrelp.c b/plugins/imrelp/imrelp.c index f9fbf67f4..cc54451d4 100644 --- a/plugins/imrelp/imrelp.c +++ b/plugins/imrelp/imrelp.c @@ -514,8 +514,9 @@ CODESTARTnewInpInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", inst->caCertFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(inppblk.descr[i].name, "tls.mycert")) { inst->myCertFile = (uchar*)es_str2cstr(pvals[i].val.d.estr, NULL); fp = fopen((const char*)inst->myCertFile, "r"); @@ -525,8 +526,9 @@ CODESTARTnewInpInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", inst->myCertFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(inppblk.descr[i].name, "tls.myprivkey")) { inst->myPrivKeyFile = (uchar*)es_str2cstr(pvals[i].val.d.estr, NULL); fp = fopen((const char*)inst->myPrivKeyFile, "r"); @@ -536,8 +538,9 @@ CODESTARTnewInpInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", inst->myPrivKeyFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(inppblk.descr[i].name, "tls.permittedpeer")) { inst->permittedPeers.nmemb = pvals[i].val.d.ar->nmemb; CHKmalloc(inst->permittedPeers.name = diff --git a/plugins/omrelp/omrelp.c b/plugins/omrelp/omrelp.c index d32a66e07..85b962105 100644 --- a/plugins/omrelp/omrelp.c +++ b/plugins/omrelp/omrelp.c @@ -378,8 +378,9 @@ CODESTARTnewActInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", pData->caCertFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(actpblk.descr[i].name, "tls.mycert")) { pData->myCertFile = (uchar*)es_str2cstr(pvals[i].val.d.estr, NULL); fp = fopen((const char*)pData->myCertFile, "r"); @@ -389,8 +390,9 @@ CODESTARTnewActInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", pData->myCertFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(actpblk.descr[i].name, "tls.myprivkey")) { pData->myPrivKeyFile = (uchar*)es_str2cstr(pvals[i].val.d.estr, NULL); fp = fopen((const char*)pData->myPrivKeyFile, "r"); @@ -400,8 +402,9 @@ CODESTARTnewActInst errmsg.LogError(0, RS_RET_NO_FILE_ACCESS, "error: certificate file %s couldn't be accessed: %s\n", pData->myPrivKeyFile, errStr); + } else { + fclose(fp); } - fclose(fp); } else if(!strcmp(actpblk.descr[i].name, "tls.authmode")) { pData->authmode = (uchar*)es_str2cstr(pvals[i].val.d.estr, NULL); } else if(!strcmp(actpblk.descr[i].name, "tls.permittedpeer")) { From 03a5c4332b45bbdf95be50875d88a6b8727f3815 Mon Sep 17 00:00:00 2001 From: PascalWithopf Date: Fri, 20 Oct 2017 12:35:18 +0200 Subject: [PATCH 2/3] omrelp: add test for certificate check Test checks that there is no Segmentation Fault when a certificate cannot be accessed. --- tests/Makefile.am | 4 +++- tests/relp_tls_certificate_not_found.sh | 30 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100755 tests/relp_tls_certificate_not_found.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index ea1fa637a..866f3cde8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -591,7 +591,8 @@ TESTS += sndrcv_relp.sh \ imrelp-manyconn.sh if ENABLE_GNUTLS TESTS += \ - sndrcv_relp_tls.sh + sndrcv_relp_tls.sh \ + relp_tls_certificate_not_found.sh endif endif @@ -1176,6 +1177,7 @@ EXTRA_DIST= \ sndrcv_relp_tls.sh \ testsuites/sndrcv_relp_tls_sender.conf \ testsuites/sndrcv_relp_tls_rcvr.conf \ + relp_tls_certificate_not_found.sh \ sndrcv_relp_dflt_pt.sh \ testsuites/sndrcv_relp_dflt_pt_rcvr.conf \ testsuites/sndrcv_relp_dflt_pt_sender.conf \ diff --git a/tests/relp_tls_certificate_not_found.sh b/tests/relp_tls_certificate_not_found.sh new file mode 100755 index 000000000..e354ccf58 --- /dev/null +++ b/tests/relp_tls_certificate_not_found.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# add 2017-09-21 by Pascal Withopf, released under ASL 2.0 +. $srcdir/diag.sh init +. $srcdir/diag.sh generate-conf +. $srcdir/diag.sh add-conf ' + +module(load="../plugins/omrelp/.libs/omrelp") +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="13514" ruleset="ruleset") +input(type="imtcp" port="13514") + +ruleset(name="ruleset") { + action(type="omrelp" target="127.0.0.1" port="10514" tls="on" tls.authMode="name" tls.caCert="tls-certs/ca.pem" tls.myCert="tls-certs/fake-cert.pem" tls.myPrivKey="tls-certs/fake-key.pem" tls.permittedPeer=["rsyslog-test-root-ca"]) +} + +action(type="omfile" file="rsyslog.out.log") +' +. $srcdir/diag.sh startup +. $srcdir/diag.sh shutdown-when-empty +. $srcdir/diag.sh wait-shutdown + +grep "certificate file tls-certs/fake-cert.pem.*No such file" rsyslog.out.log > /dev/null +if [ $? -ne 0 ]; then + echo + echo "FAIL: expected error message from missing input file not found. rsyslog.out.log is:" + cat rsyslog.out.log + . $srcdir/diag.sh error-exit 1 +fi + +. $srcdir/diag.sh exit From d67f72979e92687dbbb079849a39b1e029450d9b Mon Sep 17 00:00:00 2001 From: PascalWithopf Date: Wed, 25 Oct 2017 11:48:48 +0200 Subject: [PATCH 3/3] imtcp: change error msg to check for NULL On Solaris trying to print an empty parameter leads to a Segmentation Fault when the error message, which contains the parameter, is printed. --- runtime/tcpsrv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/runtime/tcpsrv.c b/runtime/tcpsrv.c index 5a901d024..c6cea5b87 100644 --- a/runtime/tcpsrv.c +++ b/runtime/tcpsrv.c @@ -408,7 +408,8 @@ create_tcp_socket(tcpsrv_t *pThis) localRet = initTCPListener(pThis, pEntry); if(localRet != RS_RET_OK) { errmsg.LogError(0, localRet, "Could not create tcp listener, ignoring port " - "%s bind-address %s.", pEntry->pszPort, pEntry->pszAddr); + "%s bind-address %s.", pEntry->pszPort, + (pEntry->pszAddr == NULL) ? "(null)" : (const char*)pEntry->pszAddr); } pEntry = pEntry->pNext; } @@ -1180,7 +1181,8 @@ static rsRetVal SetGnutlsPriorityString(tcpsrv_t *pThis, uchar *iVal) { DEFiRet; - DBGPRINTF("tcpsrv: gnutlsPriorityString set to %s\n", iVal); + DBGPRINTF("tcpsrv: gnutlsPriorityString set to %s\n", + (iVal == NULL) ? "(null)" : (const char*) iVal); pThis->gnutlsPriorityString = iVal; RETiRet; }