fixed memory leaks in restconf evhtp module

This commit is contained in:
Olof hagsand 2020-08-22 18:02:08 +02:00
parent 6be4b18391
commit 2e23856676
4 changed files with 83 additions and 45 deletions

View file

@ -1,13 +1,37 @@
# Clixon Restconf # Clixon Restconf
* [Installation](#installation) * [Evhtp](#evhtp)
* [Nginx](#nginx)
* [Streams](#streams) * [Streams](#streams)
* [Nchan Streams](#nchan) * [Nchan Streams](#nchan)
* [Debugging](#debugging) * [Debugging](#debugging)
## Installation There are two installation instructions: for libevhtp and nginx.
The examples are based on Nginx. Other reverse proxies should work but are not verified. ## Evhtp
Download, build and install libevhtp from source. Prereqs: libevent and cmake.
```
git clone https://github.com/criticalstack/libevhtp.git
cd libevhtp/build
cmake -DEVHTP_DISABLE_REGEX=ON -DEVHTP_DISABLE_EVTHR=ON ..
make
sudo make install
```
Configure clixon with evhtp:
```
./configure --with-restconf=evhtp
```
Ensure www-data is member of the CLICON_SOCK_GROUP (default clicon). If not, add it:
```
sudo usermod -a -G clicon www-data
```
## Nginx
Installation instruction for Nginx. Other reverse proxies should work but are not verified.
Ensure www-data is member of the CLICON_SOCK_GROUP (default clicon). If not, add it: Ensure www-data is member of the CLICON_SOCK_GROUP (default clicon). If not, add it:
``` ```
@ -49,6 +73,8 @@ Or on FreeBSD:
sudo service nginx start sudo service nginx start
``` ```
## Run
Start clixon backend daemon (if not already started) Start clixon backend daemon (if not already started)
``` ```
sudo clixon_backend -s init -f /usr/local/etc/example.xml sudo clixon_backend -s init -f /usr/local/etc/example.xml

View file

@ -118,7 +118,6 @@ restconf_reply_header(void *req0,
clicon_err(OE_CFG, errno, "evhttp_header_new"); clicon_err(OE_CFG, errno, "evhttp_header_new");
goto done; goto done;
} }
value = NULL; /* freed by evhtp */
evhtp_headers_add_header(req->headers_out, evhdr); evhtp_headers_add_header(req->headers_out, evhdr);
retval = 0; retval = 0;
done: done:

View file

@ -38,11 +38,12 @@
#ifdef HAVE_CONFIG_H #ifdef HAVE_CONFIG_H
#include "clixon_config.h" /* generated by config & autoconf */ #include "clixon_config.h" /* generated by config & autoconf */
#endif #endif
/* compilation withotu threading support
* XXX: could be disabled already in configure? /* The clixon evhtp code can be compiled with or without threading support
* The choice is set at libevhtp compile time by cmake. Eg:
* cmake -DEVHTP_DISABLE_EVTHR=ON # Disable threads.
* Default in testing is disabled threads.
*/ */
//#define EVHTP_DISABLE_EVTHR
#define EVHTP_DISABLE_REGEX
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
@ -87,7 +88,24 @@
/* Need global variable to for signal handler XXX */ /* Need global variable to for signal handler XXX */
static clicon_handle _CLICON_HANDLE = NULL; static clicon_handle _CLICON_HANDLE = NULL;
static struct evhtp_handle{
evhtp_t *eh_htp;
struct event_base *eh_evbase;
evhtp_ssl_cfg_t *eh_ssl_config;
} _EVHTP_HANDLE = {0,};
static void
evhtp_terminate(struct evhtp_handle *eh)
{
if (eh->eh_htp){
evhtp_unbind_socket(eh->eh_htp);
evhtp_free(eh->eh_htp);
}
if (eh->eh_evbase)
event_base_free(eh->eh_evbase);
if (eh->eh_ssl_config)
free(eh->eh_ssl_config);
}
/*! Signall terminates process /*! Signall terminates process
*/ */
@ -101,6 +119,7 @@ restconf_sig_term(int arg)
__PROGRAM__, __FUNCTION__, getpid(), arg); __PROGRAM__, __FUNCTION__, getpid(), arg);
else else
exit(-1); exit(-1);
evhtp_terminate(&_EVHTP_HANDLE);
if (_CLICON_HANDLE){ if (_CLICON_HANDLE){
// stream_child_freeall(_CLICON_HANDLE); // stream_child_freeall(_CLICON_HANDLE);
restconf_terminate(_CLICON_HANDLE); restconf_terminate(_CLICON_HANDLE);
@ -457,6 +476,8 @@ cx_path_restconf(evhtp_request_t *req,
if (restconf_param_del_all(h) < 0) if (restconf_param_del_all(h) < 0)
goto done; goto done;
done: done:
if (qvec)
cvec_free(qvec);
return; /* void */ return; /* void */
} }
@ -601,15 +622,13 @@ main(int argc,
size_t cligen_bufthreshold; size_t cligen_bufthreshold;
uint16_t defaultport; uint16_t defaultport;
uint16_t port = 0; uint16_t port = 0;
evhtp_t *htp = NULL;
struct event_base *evbase = NULL;
evhtp_ssl_cfg_t *ssl_config = NULL;
int dbg = 0; int dbg = 0;
int use_ssl = 0; int use_ssl = 0;
int ssl_verify_clients = 0; int ssl_verify_clients = 0;
char *restconf_ipv4_addr = NULL; char *restconf_ipv4_addr = NULL;
char *restconf_ipv6_addr = NULL; char *restconf_ipv6_addr = NULL;
int i; int i;
struct evhtp_handle *eh = &_EVHTP_HANDLE;
/* In the startup, logs to stderr & debug flag set later */ /* In the startup, logs to stderr & debug flag set later */
clicon_log_init(__PROGRAM__, LOG_INFO, logdst); clicon_log_init(__PROGRAM__, LOG_INFO, logdst);
@ -747,20 +766,20 @@ main(int argc,
/* Check server ssl certs */ /* Check server ssl certs */
if (use_ssl){ if (use_ssl){
/* Init evhtp ssl config struct */ /* Init evhtp ssl config struct */
if ((ssl_config = malloc(sizeof(evhtp_ssl_cfg_t))) == NULL){ if ((eh->eh_ssl_config = malloc(sizeof(evhtp_ssl_cfg_t))) == NULL){
clicon_err(OE_UNIX, errno, "malloc"); clicon_err(OE_UNIX, errno, "malloc");
goto done; goto done;
} }
memset(ssl_config, 0, sizeof(evhtp_ssl_cfg_t)); memset(eh->eh_ssl_config, 0, sizeof(evhtp_ssl_cfg_t));
ssl_config->ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1; eh->eh_ssl_config->ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1;
if (cx_get_certs(h, ssl_verify_clients, ssl_config) < 0) if (cx_get_certs(h, ssl_verify_clients, eh->eh_ssl_config) < 0)
goto done; goto done;
ssl_config->x509_verify_cb = cx_verify_certs; /* Is extra verification necessary? */ eh->eh_ssl_config->x509_verify_cb = cx_verify_certs; /* Is extra verification necessary? */
if (ssl_verify_clients){ if (ssl_verify_clients){
ssl_config->verify_peer = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT; eh->eh_ssl_config->verify_peer = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
ssl_config->x509_verify_cb = cx_verify_certs; eh->eh_ssl_config->x509_verify_cb = cx_verify_certs;
ssl_config->verify_depth = 2; eh->eh_ssl_config->verify_depth = 2;
} }
} }
@ -770,44 +789,44 @@ main(int argc,
clicon_argv_set(h, argv0, argc, argv); clicon_argv_set(h, argv0, argc, argv);
/* Init evhtp */ /* Init evhtp */
if ((evbase = event_base_new()) == NULL){ if ((eh->eh_evbase = event_base_new()) == NULL){
clicon_err(OE_UNIX, errno, "event_base_new"); clicon_err(OE_UNIX, errno, "event_base_new");
goto done; goto done;
} }
/* create a new evhtp_t instance */ /* create a new evhtp_t instance */
if ((htp = evhtp_new(evbase, NULL)) == NULL){ if ((eh->eh_htp = evhtp_new(eh->eh_evbase, NULL)) == NULL){
clicon_err(OE_UNIX, errno, "evhtp_new"); clicon_err(OE_UNIX, errno, "evhtp_new");
goto done; goto done;
} }
/* Here the daemon either uses SSL or not, ie you cant seem to mix http and https :-( */ /* Here the daemon either uses SSL or not, ie you cant seem to mix http and https :-( */
if (use_ssl){ if (use_ssl){
if (evhtp_ssl_init(htp, ssl_config) < 0){ if (evhtp_ssl_init(eh->eh_htp, eh->eh_ssl_config) < 0){
clicon_err(OE_UNIX, errno, "evhtp_new"); clicon_err(OE_UNIX, errno, "evhtp_new");
goto done; goto done;
} }
} }
#ifndef EVHTP_DISABLE_EVTHR #ifndef EVHTP_DISABLE_EVTHR
evhtp_use_threads_wexit(htp, NULL, NULL, 4, NULL); evhtp_use_threads_wexit(eh->eh_htp, NULL, NULL, 4, NULL);
#endif #endif
/* Callback before the connection is accepted. */ /* Callback before the connection is accepted. */
evhtp_set_pre_accept_cb(htp, cx_pre_accept, h); evhtp_set_pre_accept_cb(eh->eh_htp, cx_pre_accept, h);
/* Callback right after a connection is accepted. */ /* Callback right after a connection is accepted. */
evhtp_set_post_accept_cb(htp, cx_post_accept, h); evhtp_set_post_accept_cb(eh->eh_htp, cx_post_accept, h);
/* Callback to be executed for all /restconf api calls */ /* Callback to be executed for all /restconf api calls */
if (evhtp_set_cb(htp, "/" RESTCONF_API, cx_path_restconf, h) == NULL){ if (evhtp_set_cb(eh->eh_htp, "/" RESTCONF_API, cx_path_restconf, h) == NULL){
clicon_err(OE_EVENTS, errno, "evhtp_set_cb"); clicon_err(OE_EVENTS, errno, "evhtp_set_cb");
goto done; goto done;
} }
/* Callback to be executed for all /restconf api calls */ /* Callback to be executed for all /restconf api calls */
if (evhtp_set_cb(htp, RESTCONF_WELL_KNOWN, cx_path_wellknown, h) == NULL){ if (evhtp_set_cb(eh->eh_htp, RESTCONF_WELL_KNOWN, cx_path_wellknown, h) == NULL){
clicon_err(OE_EVENTS, errno, "evhtp_set_cb"); clicon_err(OE_EVENTS, errno, "evhtp_set_cb");
goto done; goto done;
} }
/* Generic callback called if no other callbacks are matched */ /* Generic callback called if no other callbacks are matched */
evhtp_set_gencb(htp, cx_gencb, h); evhtp_set_gencb(eh->eh_htp, cx_gencb, h);
/* bind to a socket, optionally with specific protocol support formatting /* bind to a socket, optionally with specific protocol support formatting
*/ */
@ -825,7 +844,7 @@ main(int argc,
goto done; goto done;
} }
cprintf(cb, "ipv4:%s", restconf_ipv4_addr); cprintf(cb, "ipv4:%s", restconf_ipv4_addr);
if (evhtp_bind_socket(htp, /* evhtp handle */ if (evhtp_bind_socket(eh->eh_htp, /* evhtp handle */
cbuf_get(cb), /* string address, eg ipv4:<ipv4addr> */ cbuf_get(cb), /* string address, eg ipv4:<ipv4addr> */
port, /* port */ port, /* port */
SOCKET_LISTEN_BACKLOG /* backlog flag, see listen(5) */ SOCKET_LISTEN_BACKLOG /* backlog flag, see listen(5) */
@ -836,14 +855,15 @@ main(int argc,
if (cb) if (cb)
cbuf_free(cb); cbuf_free(cb);
} }
if (restconf_ipv6_addr != NULL && strlen(restconf_ipv6_addr)){ /* Eeh can only bind one */
if (0 && restconf_ipv6_addr != NULL && strlen(restconf_ipv6_addr)){
cbuf *cb; cbuf *cb;
if ((cb = cbuf_new()) == NULL){ if ((cb = cbuf_new()) == NULL){
clicon_err(OE_UNIX, errno, "cbuf_new"); clicon_err(OE_UNIX, errno, "cbuf_new");
goto done; goto done;
} }
cprintf(cb, "ipv6:%s", restconf_ipv6_addr); cprintf(cb, "ipv6:%s", restconf_ipv6_addr);
if (evhtp_bind_socket(htp, /* evhtp handle */ if (evhtp_bind_socket(eh->eh_htp, /* evhtp handle */
cbuf_get(cb), /* string address, eg ipv6:<ipv6addr> */ cbuf_get(cb), /* string address, eg ipv6:<ipv6addr> */
port, /* port */ port, /* port */
SOCKET_LISTEN_BACKLOG /* backlog flag, see listen(5) */ SOCKET_LISTEN_BACKLOG /* backlog flag, see listen(5) */
@ -855,7 +875,7 @@ main(int argc,
cbuf_free(cb); cbuf_free(cb);
} }
/* Drop privileges to WWWUSER if started as root */ /* Drop privileges to WWWUSER if started as root */
if (restconf_drop_privileges(h, WWWUSER) < 0) if (0 && restconf_drop_privileges(h, WWWUSER) < 0)
goto done; goto done;
/* Init cligen buffers */ /* Init cligen buffers */
@ -944,20 +964,13 @@ main(int argc,
if (clixon_plugin_start_all(h) < 0) if (clixon_plugin_start_all(h) < 0)
goto done; goto done;
/* Find and read configfile */ event_base_loop(eh->eh_evbase, 0);
if (clicon_options_main(h) < 0)
goto done;
event_base_loop(evbase, 0);
evhtp_unbind_socket(htp);
// evhtp_safe_free(htp, evhtp_free);
// evhtp_safe_free(evbase, event_base_free);
retval = 0; retval = 0;
done: done:
clicon_debug(1, "restconf_main_evhtp done");
// stream_child_freeall(h); // stream_child_freeall(h);
evhtp_terminate(&_EVHTP_HANDLE);
restconf_terminate(h); restconf_terminate(h);
return retval; return retval;
} }

View file

@ -554,8 +554,8 @@ yang_parse_file(int fd,
len = BUFLEN; /* any number is fine */ len = BUFLEN; /* any number is fine */
if ((buf = malloc(len)) == NULL){ if ((buf = malloc(len)) == NULL){
perror("pt_file malloc"); clicon_err(OE_XML, errno, "malloc");
return NULL; goto done;
} }
memset(buf, 0, len); memset(buf, 0, len);
i = 0; /* position in buf */ i = 0; /* position in buf */
@ -579,7 +579,7 @@ yang_parse_file(int fd,
if ((ymod = yang_parse_str(buf, name, yspec)) < 0) if ((ymod = yang_parse_str(buf, name, yspec)) < 0)
goto done; goto done;
done: done:
if (buf) if (buf != NULL)
free(buf); free(buf);
return ymod; /* top-level (sub)module */ return ymod; /* top-level (sub)module */
} }