From f0ddbed44c332391ae6d9bbf6b07e2f06c4dd676 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Sat, 29 Nov 2008 07:22:48 +0100 Subject: security bugfix: $AllowedSender was not honored, ...all senders were permitted instead --- runtime/net.c | 69 ++++++++++++++++++++++++++++++++++++++++--------------- runtime/net.h | 7 ++---- runtime/rsyslog.h | 1 + 3 files changed, 54 insertions(+), 23 deletions(-) (limited to 'runtime') diff --git a/runtime/net.c b/runtime/net.c index b9af700c..76dc185c 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -92,6 +92,30 @@ int ACLDontResolve = 0; /* add hostname to acl instead of resolving it /* ------------------------------ begin permitted peers code ------------------------------ */ +/* sets the correct allow root pointer based on provided type + * rgerhards, 2008-12-01 + */ +static inline rsRetVal +setAllowRoot(struct AllowedSenders **ppAllowRoot, uchar *pszType) +{ + DEFiRet; + + if(!strcmp((char*)pszType, "UDP")) + *ppAllowRoot = pAllowedSenders_UDP; + else if(!strcmp((char*)pszType, "TCP")) + *ppAllowRoot = pAllowedSenders_TCP; + else if(!strcmp((char*)pszType, "GSS")) + *ppAllowRoot = pAllowedSenders_GSS; + else { + dbgprintf("program error: invalid allowed sender ID '%s', denying...\n", pszType); + ABORT_FINALIZE(RS_RET_CODE_ERR); /* everything is invalid for an invalid type */ + } + +finalize_it: + RETiRet; +} + + /* add a wildcard entry to this permitted peer. Entries are always * added at the tail of the list. pszStr and lenStr identify the wildcard * entry to be added. Note that the string is NOT \0 terminated, so @@ -507,27 +531,32 @@ static rsRetVal AddAllowedSenderEntry(struct AllowedSenders **ppRoot, struct All } /* function to clear the allowed sender structure in cases where - * it must be freed (occurs most often when HUPed. - * TODO: reconsider recursive implementation - * I think there is also a memory leak, because only the last entry - * is acutally deleted... -- rgerhards, 2007-12-25 + * it must be freed (occurs most often when HUPed). + * rgerhards, 2008-12-02: revamped this code when we fixed the interface + * definition. Now an iterative algorithm is used. */ -void clearAllowedSenders (struct AllowedSenders *pAllow) +static void +clearAllowedSenders(uchar *pszType) { - if (pAllow != NULL) { - if (pAllow->pNext != NULL) - clearAllowedSenders (pAllow->pNext); - else { - if (F_ISSET(pAllow->allowedSender.flags, ADDR_NAME)) - free (pAllow->allowedSender.addr.HostWildcard); - else - free (pAllow->allowedSender.addr.NetAddr); - - free (pAllow); - } + struct AllowedSenders *pPrev; + struct AllowedSenders *pCurr; + + if(setAllowRoot(&pCurr, pszType) != RS_RET_OK) + return; /* if something went wrong, so let's leave */ + + while(pCurr != NULL) { + pPrev = pCurr; + pCurr = pCurr->pNext; + /* now delete the entry we are right now processing */ + if(F_ISSET(pPrev->allowedSender.flags, ADDR_NAME)) + free(pPrev->allowedSender.addr.HostWildcard); + else + free(pPrev->allowedSender.addr.NetAddr); + free(pPrev); } } + /* function to add an allowed sender to the allowed sender list. The * root of the list is caller-provided, so it can be used for all * supported lists. The caller must provide a pointer to the root, @@ -905,11 +934,15 @@ static inline int MaskCmp(struct NetAddr *pAllow, uint8_t bits, struct sockaddr * returns 1, if the sender is allowed, 0 otherwise. * rgerhards, 2005-09-26 */ -static int isAllowedSender(struct AllowedSenders *pAllowRoot, struct sockaddr *pFrom, const char *pszFromHost) +static int isAllowedSender(uchar *pszType, struct sockaddr *pFrom, const char *pszFromHost) { struct AllowedSenders *pAllow; - + struct AllowedSenders *pAllowRoot; + assert(pFrom != NULL); + + if(setAllowRoot(&pAllowRoot, pszType) != RS_RET_OK) + return 0; /* if something went wrong, we denie access - that's the better choice... */ if(pAllowRoot == NULL) return 1; /* checking disabled, everything is valid! */ diff --git a/runtime/net.h b/runtime/net.h index 0d36e824..092c3116 100644 --- a/runtime/net.h +++ b/runtime/net.h @@ -135,11 +135,11 @@ BEGINinterface(net) /* name must also be changed in ENDinterface macro! */ /* things to go away after proper modularization */ rsRetVal (*addAllowedSenderLine)(char* pName, uchar** ppRestOfConfLine); void (*PrintAllowedSenders)(int iListToPrint); - void (*clearAllowedSenders) (); + void (*clearAllowedSenders)(uchar*); void (*debugListenInfo)(int fd, char *type); int *(*create_udp_socket)(uchar *hostname, uchar *LogPort, int bIsServer); void (*closeUDPListenSockets)(int *finet); - int (*isAllowedSender)(struct AllowedSenders *pAllowRoot, struct sockaddr *pFrom, const char *pszFromHost); + int (*isAllowedSender)(uchar *pszType, struct sockaddr *pFrom, const char *pszFromHost); rsRetVal (*getLocalHostname)(uchar**); int (*should_use_so_bsdcompat)(void); /* permitted peer handling should be replaced by something better (see comments above) */ @@ -149,9 +149,6 @@ BEGINinterface(net) /* name must also be changed in ENDinterface macro! */ /* data members - these should go away over time... TODO */ int *pACLAddHostnameOnFail; /* add hostname to acl when DNS resolving has failed */ int *pACLDontResolve; /* add hostname to acl instead of resolving it to IP(s) */ - struct AllowedSenders *pAllowedSenders_UDP; - struct AllowedSenders *pAllowedSenders_TCP; - struct AllowedSenders *pAllowedSenders_GSS; ENDinterface(net) #define netCURR_IF_VERSION 4 /* increment whenever you change the interface structure! */ diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 7927af3a..06ffae86 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -248,6 +248,7 @@ enum rsRetVal_ /** return value. All methods return this if not specified oth RS_RET_CERTLESS = -2102, /**< state: we run without machine cert (this may be OK) */ RS_RET_QUEUE_FULL = -2103, /**< queue is full, operation could not be completed */ RS_RET_ACCEPT_ERR = -2104, /**< error during accept() system call */ + RS_RET_CODE_ERR = -2109, /**< program code (internal) error */ /* RainerScript error messages (range 1000.. 1999) */ RS_RET_SYSVAR_NOT_FOUND = 1001, /**< system variable could not be found (maybe misspelled) */ -- cgit From 97b89435aad77bd6d9e18747b55d701e360d5aac Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Sat, 29 Nov 2008 09:47:04 +0100 Subject: bugfix: $AllowedSender handled invalidly for plain TCP transport --- runtime/netstrm.c | 12 ++++++++++++ runtime/netstrm.h | 10 +++++++++- runtime/nsd.h | 12 +++++++++++- runtime/nsd_ptcp.c | 25 +++++++++++++++++++++++++ runtime/nsd_ptcp.h | 3 +++ 5 files changed, 60 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/netstrm.c b/runtime/netstrm.c index 2f4a1964..ffa1c578 100644 --- a/runtime/netstrm.c +++ b/runtime/netstrm.c @@ -265,6 +265,17 @@ GetRemoteIP(netstrm_t *pThis, uchar **ppsz) } +/* get remote addr - slim wrapper for NSD driver function */ +static rsRetVal +GetRemAddr(netstrm_t *pThis, struct sockaddr_storage **ppAddr) +{ + DEFiRet; + ISOBJ_TYPE_assert(pThis, netstrm); + iRet = pThis->Drvr.GetRemAddr(pThis->pDrvrData, ppAddr); + RETiRet; +} + + /* open a connection to a remote host (server). * rgerhards, 2008-03-19 */ @@ -320,6 +331,7 @@ CODESTARTobjQueryInterface(netstrm) pIf->AcceptConnReq = AcceptConnReq; pIf->GetRemoteHName = GetRemoteHName; pIf->GetRemoteIP = GetRemoteIP; + pIf->GetRemAddr = GetRemAddr; pIf->SetDrvrMode = SetDrvrMode; pIf->SetDrvrAuthMode = SetDrvrAuthMode; pIf->SetDrvrPermPeers = SetDrvrPermPeers; diff --git a/runtime/netstrm.h b/runtime/netstrm.h index 1a97ef23..3ab790e8 100644 --- a/runtime/netstrm.h +++ b/runtime/netstrm.h @@ -61,8 +61,16 @@ BEGINinterface(netstrm) /* name must also be changed in ENDinterface macro! */ * this interface. -- rgerhards, 2008-05-05 */ rsRetVal (*GetSock)(netstrm_t *pThis, int *pSock); + rsRetVal (*GetRemAddr)(netstrm_t *pThis, struct sockaddr_storage **ppAddr); + /* getRemAddr() is an aid needed by the legacy ACL system. It exposes the remote + * peer's socket addr structure, so that the legacy matching functions can work on + * it. Note that this ties netstream drivers to things that can be implemented over + * sockets - not really desirable, but not the end of the world... TODO: should be + * reconsidered when a new ACL system is build. -- rgerhards, 2008-12-01 + */ ENDinterface(netstrm) -#define netstrmCURR_IF_VERSION 2 /* increment whenever you change the interface structure! */ +#define netstrmCURR_IF_VERSION 3 /* increment whenever you change the interface structure! */ +/* interface version 3 added GetRemAddr() */ /* prototypes */ PROTOTYPEObj(netstrm); diff --git a/runtime/nsd.h b/runtime/nsd.h index 1811f078..f0c9b9b6 100644 --- a/runtime/nsd.h +++ b/runtime/nsd.h @@ -27,6 +27,8 @@ #ifndef INCLUDED_NSD_H #define INCLUDED_NSD_H +#include + enum nsdsel_waitOp_e { NSDSEL_RD = 1, NSDSEL_WR = 2, @@ -60,8 +62,16 @@ BEGINinterface(nsd) /* name must also be changed in ENDinterface macro! */ * OS sockets. This interface is primarily meant as an internal aid for * those drivers that utilize the nsd_ptcp to do some of their work. */ + rsRetVal (*GetRemAddr)(nsd_t *pThis, struct sockaddr_storage **ppAddr); + /* getRemAddr() is an aid needed by the legacy ACL system. It exposes the remote + * peer's socket addr structure, so that the legacy matching functions can work on + * it. Note that this ties netstream drivers to things that can be implemented over + * sockets - not really desirable, but not the end of the world... TODO: should be + * reconsidered when a new ACL system is build. -- rgerhards, 2008-12-01 + */ ENDinterface(nsd) -#define nsdCURR_IF_VERSION 3 /* increment whenever you change the interface structure! */ +#define nsdCURR_IF_VERSION 4 /* increment whenever you change the interface structure! */ +/* interface version 4 added GetRemAddr() */ /* interface for the select call */ BEGINinterface(nsdsel) /* name must also be changed in ENDinterface macro! */ diff --git a/runtime/nsd_ptcp.c b/runtime/nsd_ptcp.c index 4cb46380..cc531ca0 100644 --- a/runtime/nsd_ptcp.c +++ b/runtime/nsd_ptcp.c @@ -91,6 +91,24 @@ CODESTARTobjDestruct(nsd_ptcp) ENDobjDestruct(nsd_ptcp) +/* Provide access to the sockaddr_storage of the remote peer. This + * is needed by the legacy ACL system. --- gerhards, 2008-12-01 + */ +static rsRetVal +GetRemAddr(nsd_t *pNsd, struct sockaddr_storage **ppAddr) +{ + nsd_ptcp_t *pThis = (nsd_ptcp_t*) pNsd; + DEFiRet; + + ISOBJ_TYPE_assert((pThis), nsd_ptcp); + assert(ppAddr != NULL); + + *ppAddr = &(pThis->remAddr); + + RETiRet; +} + + /* Provide access to the underlying OS socket. This is primarily * useful for other drivers (like nsd_gtls) who utilize ourselfs * for some of their functionality. -- rgerhards, 2008-04-18 @@ -320,6 +338,12 @@ AcceptConnReq(nsd_t *pNsd, nsd_t **ppNew) /* construct our object so that we can use it... */ CHKiRet(nsd_ptcpConstruct(&pNew)); + /* for the legacy ACL code, we need to preserve addr. While this is far from + * begin perfect (from an abstract design perspective), we need this to prevent + * breaking everything. TODO: we need to implement a new ACL module to get rid + * of this function. -- rgerhards, 2008-12-01 + */ + memcpy(&pNew->remAddr, &addr, sizeof(struct sockaddr_storage)); CHKiRet(FillRemHost(pNew, (struct sockaddr*) &addr)); /* set the new socket to non-blocking IO -TODO:do we really need to do this here? Do we always want it? */ @@ -716,6 +740,7 @@ CODESTARTobjQueryInterface(nsd_ptcp) pIf->Construct = (rsRetVal(*)(nsd_t**)) nsd_ptcpConstruct; pIf->Destruct = (rsRetVal(*)(nsd_t**)) nsd_ptcpDestruct; pIf->Abort = Abort; + pIf->GetRemAddr = GetRemAddr; pIf->GetSock = GetSock; pIf->SetSock = SetSock; pIf->SetMode = SetMode; diff --git a/runtime/nsd_ptcp.h b/runtime/nsd_ptcp.h index efd3ed05..b94cc018 100644 --- a/runtime/nsd_ptcp.h +++ b/runtime/nsd_ptcp.h @@ -24,6 +24,8 @@ #ifndef INCLUDED_NSD_PTCP_H #define INCLUDED_NSD_PTCP_H +#include + #include "nsd.h" typedef nsd_if_t nsd_ptcp_if_t; /* we just *implement* this interface */ @@ -32,6 +34,7 @@ struct nsd_ptcp_s { BEGINobjInstance; /* Data to implement generic object - MUST be the first data element! */ uchar *pRemHostIP; /**< IP address of remote peer (currently used in server mode, only) */ uchar *pRemHostName; /**< host name of remote peer (currently used in server mode, only) */ + struct sockaddr_storage remAddr; /**< remote addr as sockaddr - used for legacy ACL code */ int sock; /**< the socket we use for regular, single-socket, operations */ }; -- cgit From 61b59a78c6b558ec06383fc5969178887d00abfc Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 1 Dec 2008 18:39:57 +0100 Subject: added interface function to nsd_gtls needed for ACL control The legacy ACL system needs access to the remote sockaddr_storage data structure. This has been implemented for the ptcp driver and now follows for gtls. See recent commits for reason. We also moved up the version numbers in preparation of the release. --- runtime/nsd_gtls.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'runtime') diff --git a/runtime/nsd_gtls.c b/runtime/nsd_gtls.c index 08623da8..073d333c 100644 --- a/runtime/nsd_gtls.c +++ b/runtime/nsd_gtls.c @@ -1342,6 +1342,20 @@ GetRemoteHName(nsd_t *pNsd, uchar **ppszHName) } +/* Provide access to the sockaddr_storage of the remote peer. This + * is needed by the legacy ACL system. --- gerhards, 2008-12-01 + */ +static rsRetVal +GetRemAddr(nsd_t *pNsd, struct sockaddr_storage **ppAddr) +{ + DEFiRet; + nsd_gtls_t *pThis = (nsd_gtls_t*) pNsd; + ISOBJ_TYPE_assert(pThis, nsd_gtls); + iRet = nsd_ptcp.GetRemAddr(pThis->pTcp, ppAddr); + RETiRet; +} + + /* get the remote host's IP address. The returned string must be freed by the * caller. -- rgerhards, 2008-04-25 */ @@ -1646,6 +1660,7 @@ CODESTARTobjQueryInterface(nsd_gtls) pIf->CheckConnection = CheckConnection; pIf->GetRemoteHName = GetRemoteHName; pIf->GetRemoteIP = GetRemoteIP; + pIf->GetRemAddr = GetRemAddr; finalize_it: ENDobjQueryInterface(nsd_gtls) -- cgit From a7104880ee05722ad957b32824165561e7085ce8 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 3 Dec 2008 10:09:00 +0100 Subject: minor: net.c did not compile if GSSAPI support was disabled --- runtime/net.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'runtime') diff --git a/runtime/net.c b/runtime/net.c index 76dc185c..ac13597c 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -104,8 +104,10 @@ setAllowRoot(struct AllowedSenders **ppAllowRoot, uchar *pszType) *ppAllowRoot = pAllowedSenders_UDP; else if(!strcmp((char*)pszType, "TCP")) *ppAllowRoot = pAllowedSenders_TCP; +#ifdef USE_GSSAPI else if(!strcmp((char*)pszType, "GSS")) *ppAllowRoot = pAllowedSenders_GSS; +#endif else { dbgprintf("program error: invalid allowed sender ID '%s', denying...\n", pszType); ABORT_FINALIZE(RS_RET_CODE_ERR); /* everything is invalid for an invalid type */ -- cgit From 4252edfebe096c805c7884e6775332705d46472f Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 3 Dec 2008 15:45:54 +0100 Subject: bugfix: memory leaks in gtls netstream driver --- runtime/netstrms.c | 4 ++++ runtime/nsd_gtls.c | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/netstrms.c b/runtime/netstrms.c index 2b754ecc..6b28e7ea 100644 --- a/runtime/netstrms.c +++ b/runtime/netstrms.c @@ -104,6 +104,10 @@ CODESTARTobjDestruct(netstrms) obj.ReleaseObj(__FILE__, pThis->pDrvrName+2, pThis->pDrvrName, (void*) &pThis->Drvr); free(pThis->pDrvrName); } + if(pThis->pszDrvrAuthMode != NULL) { + free(pThis->pszDrvrAuthMode); + pThis->pszDrvrAuthMode = NULL; + } if(pThis->pBaseDrvrName != NULL) { free(pThis->pBaseDrvrName); pThis->pBaseDrvrName = NULL; diff --git a/runtime/nsd_gtls.c b/runtime/nsd_gtls.c index 073d333c..3a79a015 100644 --- a/runtime/nsd_gtls.c +++ b/runtime/nsd_gtls.c @@ -1229,7 +1229,6 @@ SetAuthMode(nsd_t *pNsd, uchar *mode) /* TODO: clear stored IDs! */ finalize_it: -dbgprintf("gtls auth mode %d set\n", pThis->authMode); RETiRet; } @@ -1491,6 +1490,13 @@ Rcv(nsd_t *pNsd, uchar *pBuf, ssize_t *pLenBuf) if(pThis->lenRcvBuf == 0) { /* EOS */ *pLenBuf = 0; + /* in this case, we also need to free the receive buffer, if we + * allocated one. -- rgerhards, 2008-12-03 + */ + if(pThis->pszRcvBuf != NULL) { + free(pThis->pszRcvBuf); + pThis->pszRcvBuf = NULL; + } ABORT_FINALIZE(RS_RET_CLOSED); } -- cgit