experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Restconf http-data server updates

Browse source 
Check data paths for .., ~ and soft links
Changed semantics of `CLICON_HTTP_DATA_PATH` and `_ROOT`
Change URI catch-all to 404 instead of 400
Fixed some memory leaks
This commit is contained in:
Olof hagsand 2022-04-24 09:44:27 +02:00
parent 0ed8b836b1
commit 84d88c8ad8
12 changed files with 303 additions and 106 deletions
Download patch file Download diff file Expand all files Collapse all files

178
apps/restconf/clixon_http_data.c
View file

@ -86,32 +86,34 @@ static const map_str2str mime_map[] = {
/*! Check if uri path denotes a data path
*
* @param[out] data Pointer to string where data starts if retval = 1
* @param[in] h Clixon handle
* @retval 0 No, not a data path, or not enabled
* @retval 1 Yes, a data path and "data" points to www-data if given
*/
int
api_path_is_data(clicon_handle h,
char **data)
api_path_is_data(clicon_handle h)
{
char *path;
int retval = 0;
char *path = NULL;
char *http_data_path;
if (restconf_http_data_get(h) == 0)
return 0;
goto done;
if ((path = restconf_uripath(h)) == NULL)
return 0;
goto done;
if ((http_data_path = clicon_option_str(h, "CLICON_HTTP_DATA_PATH")) == NULL)
return 0;
goto done;
if (strlen(path) < strlen(http_data_path))
return 0;
goto done;
if (path[0] != '/')
return 0;
goto done;
if (strncmp(path, http_data_path, strlen(http_data_path)) != 0)
return 0;
if (data)
*data = path + strlen(http_data_path);
return 1;
goto done;
retval = 1;
done:
if (path)
free(path);
return retval;
}
/*! Generic restconf error function on get/head request
@ -156,6 +158,75 @@ api_http_data_err(clicon_handle h,
return retval;
}
/*! Check validity of path, may only be regular dir or file
* No .., soft link, ~, etc
* @param[in] prefix Prefix of path0, where to start file check
* @param[in] path0 Filepath
* @param[out] code Error code, if retval = 0
* @retval -1 Error
* @retval 0 Invalid, code set
* @retval 1 OK
*/
static int
check_file_path(char *prefix,
char *path0,
int *code)
{
int retval = -1;
char *path = NULL;
int i;
struct stat fstat;
if (prefix == NULL || path0 == NULL || code == NULL){
clicon_err(OE_UNIX, EINVAL, "prefix, path0 or code is NULL");
goto done;
}
if ((path = strdup(path0)) == NULL){
clicon_err(OE_UNIX, errno, "strdup");
goto done;
}
for (i=strlen(prefix); i<strlen(path0); i++){
if (path[i] == '/'){ /* Check valid dir */
path[i] = '\0';
/* Ensure not soft link */
if (lstat(path, &fstat) < 0){
*code = 404;
goto invalid;
}
if (!S_ISDIR(fstat.st_mode)){
*code = 403;
goto invalid;
}
path[i] = '/';
}
else if (path[i] == '~'){
*code = 403;
goto invalid;
}
else if (path[i] == '.' && i>strlen(prefix) && path[i-1] == '.'){
*code = 403;
goto invalid;
}
}
/* Resulting file (ensure not soft link) */
if (lstat(path, &fstat) < 0){
*code = 404;
goto invalid;
}
if (!S_ISREG(fstat.st_mode)){
*code = 403;
goto invalid;
}
retval = 1; /* OK */
done:
if (path)
free(path);
return retval;
invalid:
retval = 0;
goto done;
}
/*! Read file data request
* @param[in] h Clicon handle
* @param[in] req Generic Www handle (can be part of clixon handle)
@ -167,21 +238,23 @@ api_http_data_err(clicon_handle h,
*/
static int
api_http_data_file(clicon_handle h,
void *req,
char *pathname,
int head)
void *req,
char *pathname,
int head)
{
int retval = -1;
cbuf *cbfile = NULL;
char *filename;
struct stat fstat;
cbuf *cbdata = NULL;
FILE *f = NULL;
long fsize;
size_t sz;
char *www_data_root = NULL;
char *suffix;
char *media;
int ret;
int code = 0;
char *str = NULL;
size_t sz;
if ((cbfile = cbuf_new()) == NULL){
clicon_err(OE_UNIX, errno, "cbuf_new");
@ -204,8 +277,12 @@ api_http_data_file(clicon_handle h,
if (pathname)
cprintf(cbfile, "/%s", pathname);
filename = cbuf_get(cbfile);
if (stat(filename, &fstat) < 0){
if (api_http_data_err(h, req, 404) < 0) /* not found */
clicon_debug(1, "%s %s", __FUNCTION__, filename);
if ((ret = check_file_path(www_data_root, filename, &code)) < 0)
goto done;
if (ret == 0){
clicon_debug(1, "%s code:%d", __FUNCTION__, code);
if (api_http_data_err(h, req, code) < 0)
goto done;
goto ok;
}
@ -214,6 +291,9 @@ api_http_data_file(clicon_handle h,
goto done;
goto ok;
}
/* Size could have been taken from stat() but this reduces the race condition interval
* There is still one without flock
*/
fseek(f, 0, SEEK_END);
fsize = ftell(f);
fseek(f, 0, SEEK_SET); /* same as rewind(f); */
@ -221,31 +301,28 @@ api_http_data_file(clicon_handle h,
clicon_err(OE_UNIX, errno, "cbuf_new_alloc");
goto done;
}
#if 0 /* Direct read but cannot set cb_len via API */
fread(cbuf_get(cbdata), fsize, 1, f);
#else
{
char *str;
if ((str = malloc(fsize + 1)) == NULL){
clicon_err(OE_UNIX, errno, "malloc");
goto done;
}
if ((sz = fread(str, fsize, 1, f)) < 0){
clicon_err(OE_UNIX, errno, "fread");
goto done;
}
if (sz != 1){
clicon_log(LOG_NOTICE, "%s: file read %s", __FUNCTION__, filename);
// XXX error handling: file read
goto done;
}
str[fsize] = 0;
if (cbuf_append_str(cbdata, str) < 0){
clicon_err(OE_UNIX, errno, "cbuf_append_str");
goto done;
}
/* Unoptimized, no direct read but requires an extra copy,
* the cligen buf API should have some mechanism for this case without the extra copy.
*/
if ((str = malloc(fsize + 1)) == NULL){
clicon_err(OE_UNIX, errno, "malloc");
goto done;
}
if ((sz = fread(str, fsize, 1, f)) < 0){
clicon_err(OE_UNIX, errno, "fread");
goto done;
}
if (sz != 1){
if (api_http_data_err(h, req, 500) < 0) /* Internal error? */
goto done;
goto ok;
}
clicon_debug(1, "%s code:%d", __FUNCTION__, code);
str[fsize] = 0;
if (cbuf_append_str(cbdata, str) < 0){
clicon_err(OE_UNIX, errno, "cbuf_append_str");
goto done;
}
#endif
if (restconf_reply_header(req, "Content-Type", "%s", media) < 0)
goto done;
if (restconf_reply_send(req, 200, cbdata, head) < 0)
@ -254,6 +331,8 @@ api_http_data_file(clicon_handle h,
ok:
retval = 0;
done:
if (str)
free(str);
if (f)
fclose(f);
if (cbfile)
@ -291,8 +370,8 @@ api_http_data(clicon_handle h,
int options = 0;
int ret;
cbuf *indata = NULL;
char *pathname = NULL;
char *path = NULL;
clicon_debug(1, "%s", __FUNCTION__);
if (req == NULL){
errno = EINVAL;
@ -300,11 +379,12 @@ api_http_data(clicon_handle h,
}
/* 1. path: with stripped prefix, ultimately: dir/filename
*/
if (!api_path_is_data(h, &pathname)){
if (!api_path_is_data(h)){
if (api_http_data_err(h, req, 404) < 0) /* not found */
goto done;
goto ok;
}
path = restconf_uripath(h);
/* 2. operation GET or HEAD */
request_method = restconf_param_get(h, "REQUEST_METHOD");
if (strcmp(request_method, "GET") == 0){
@ -360,11 +440,13 @@ api_http_data(clicon_handle h,
if (restconf_reply_send(req, 200, NULL, 0) < 0)
goto done;
}
else if (api_http_data_file(h, req, pathname, head) < 0)
else if (api_http_data_file(h, req, path, head) < 0)
goto done;
ok:
retval = 0;
done:
if (path)
free(path);
clicon_debug(1, "%s %d", __FUNCTION__, retval);
return retval;
}

2
apps/restconf/clixon_http_data.h
View file

@ -41,7 +41,7 @@
/*
* Prototypes
*/
int api_path_is_data(clicon_handle h, char **data);
int api_path_is_data(clicon_handle h);
int api_http_data(clicon_handle h, void *req, cvec *qvec);
#endif /* _CLIXON_HTTP_DATA_H_ */

11
apps/restconf/restconf_http1.c
View file

@ -345,7 +345,7 @@ restconf_http1_path_root(clicon_handle h,
restconf_conn *rc)
{
int retval = -1;
restconf_stream_data *sd;
restconf_stream_data *sd = NULL;
cvec *cvv = NULL;
char *cn;
char *subject = NULL;
@ -380,6 +380,13 @@ restconf_http1_path_root(clicon_handle h,
goto done;
goto fail;
}
#if 1
/* XXX gives mem leak in multiple requests,
* but maybe the error is that sd is not freed.
*/
if (sd->sd_path != NULL)
free(sd->sd_path);
#endif
if ((sd->sd_path = restconf_uripath(rc->rc_h)) == NULL)
goto done; // XXX SHOULDNT EXIT if no REQUEST_URI
if (rc->rc_proto_d2 == 0 && rc->rc_proto == HTTP_11)
@ -427,7 +434,7 @@ restconf_http1_path_root(clicon_handle h,
if (api_root_restconf(h, sd, sd->sd_qvec) < 0)
goto done;
}
else if (api_path_is_data(h, NULL)){
else if (api_path_is_data(h)){
if (api_http_data(h, sd, sd->sd_qvec) < 0)
goto done;
}

11
apps/restconf/restconf_nghttp2.c
View file

@ -325,7 +325,7 @@ restconf_nghttp2_path(restconf_stream_data *sd)
if (api_root_restconf(h, sd, sd->sd_qvec) < 0)
goto done;
}
else if (api_path_is_data(h, NULL)){
else if (api_path_is_data(h)){
if (api_http_data(h, sd, sd->sd_qvec) < 0)
goto done;
}
@ -475,14 +475,17 @@ http2_exec(restconf_conn *rc,
sd->sd_proto = HTTP_2; /* XXX is this necessary? */
if (strncmp(sd->sd_path, "/" RESTCONF_API, strlen("/" RESTCONF_API)) == 0
|| strcmp(sd->sd_path, RESTCONF_WELL_KNOWN) == 0
|| api_path_is_data(rc->rc_h, NULL)){
|| api_path_is_data(rc->rc_h)){
if (restconf_nghttp2_path(sd) < 0)
goto done;
}
else{
sd->sd_code = 400;
; /* ignore */
sd->sd_code = 404; /* not found */
}
if (restconf_param_del_all(rc->rc_h) < 0) // XXX
goto done;
/* If body, add a content-length header
* A server MUST NOT send a Content-Length header field in any response
* with a status code of 1xx (Informational) or 204 (No Content). A

17
apps/restconf/restconf_root.c
View file

@ -78,18 +78,23 @@
int
api_path_is_restconf(clicon_handle h)
{
char *path;
int retval = 0;
char *path = NULL;
char *restconf_path = RESTCONF_API;
if ((path = restconf_uripath(h)) == NULL)
return 0;
goto done;
if (strlen(path) < 1 + strlen(restconf_path)) /* "/" + restconf */
return 0;
goto done;
if (path[0] != '/')
return 0;
goto done;
if (strncmp(path+1, restconf_path, strlen(restconf_path)) != 0)
return 0;
return 1;
goto done;
retval = 1;
done:
if (path)
free(path);
return retval;
}
/*! Determine the root of the RESTCONF API by accessing /.well-known

20
apps/restconf/restconf_stream_fcgi.c
View file

@ -126,20 +126,26 @@ static struct stream_child *STREAM_CHILD = NULL;
int
api_path_is_stream(clicon_handle h)
{
char *path;
int retval = 0;
char *path = NULL;
char *stream_path;
if ((path = restconf_uripath(h)) == NULL)
return 0;
goto done;
if ((stream_path = clicon_option_str(h, "CLICON_STREAM_PATH")) == NULL)
return 0;
goto done;
if (strlen(path) < 1 + strlen(stream_path)) /* "/" + stream */
return 0;
goto done;
if (path[0] != '/')
return 0;
goto done;
if (strncmp(path+1, stream_path, strlen(stream_path)) != 0)
return 0;
return 1;
goto done;
retval = 1;
done:
if (path)
free(path);
return retval;
}
/*! Find restconf child using PID and cleanup FCGI Request data

1
configure.ac
View file

@ -86,6 +86,7 @@ AC_DEFINE_UNQUOTED(CLIXON_VERSION_PATCH, $CLIXON_VERSION_PATCH, [Clixon path ver
AC_CHECK_LIB(m, main)
# defines: target_cpu, target_vendor, and target_os.
AC_CANONICAL_TARGET
# AC_SUBST(var) makes @var@ appear in makefiles.

10
doc/DEVELOP.md
View file

@ -113,6 +113,16 @@ Use:
- PRIu64 for uint64
- %p for pointers
### Include files
Avoid include statements in .h files, place them in .c files whenever possible.
The reason is to avoid deep include chains where file dependencies are
difficult to analyze and understand. If include statements are only placed in .c
files, there is only a single level of include file dependencies.
The drawback is that the same include file may need to be repeated in many .c files.
## How to work in git
Clixon uses semantic versioning (https://semver.org).

11
test/lib.sh
View file

@ -94,6 +94,17 @@ DEFAULTNS="$DEFAULTONLY message-id=\"42\""
# Minimal hello message as a prelude to netconf rpcs
DEFAULTHELLO="<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>"
# XXX cannot get this to work for all combinations of nc/netcat fcgi/native
# But leave it here for debugging where netcat works properly
if [ -n "$(type netcat 2> /dev/null)" ]; then
netcat="netcat -w 1" # -N does not work on fcgi
# nc on freebsd does not work either
#elif [ -n "$(type nc 2> /dev/null)" ]; then
# netcat=nc
else
netcat=
fi
# Options passed to curl calls
# -s : silent
# -S : show error

114
test/test_http_data.sh
View file

@ -14,6 +14,7 @@ APPNAME=example
cfg=$dir/conf.xml
rm -rf $dir/www
mkdir $dir/www
mkdir $dir/www/data
# Does not work with fcgi
if [ "${WITH_RESTCONF}" = "fcgi" ]; then
@ -22,7 +23,7 @@ if [ "${WITH_RESTCONF}" = "fcgi" ]; then
fi
# Data file
cat <<EOF > $dir/www/index.html
cat <<EOF > $dir/www/data/index.html
<!DOCTYPE html>
<html>
<head>
@ -43,7 +44,7 @@ working. Further configuration is required.</p>
</html>
EOF
cat <<EOF > $dir/www/example.css
cat <<EOF > $dir/www/data/example.css
img {
display: inline;
border:
@ -67,6 +68,54 @@ h1,h2,h3,h4,h5,h6 {
}
EOF
# Outside wwwdir, should not be able to access this
cat <<EOF > $dir/outside.html
<!DOCTYPE html>
<html>
<head>
<title>Dont access this</title>
<style>
body {
width: 35em;
margin: 0 auto;
font-family: Tahoma, Verdana, Arial, sans-serif;
}
</style>
</head>
<body>
<h1>Dont access this!</h1>
<p>If you see this page, you accessed a file outside the root domain</p>
</body>
</html>
EOF
# Create a soft link from inside to outside
ln -s $dir/outside.html $dir/www/data/inside.html
# Disable read access
cat <<EOF > $dir/www/data/noread.html
<!DOCTYPE html>
<html>
<head>
<title>No read</title>
<style>
body {
width: 35em;
margin: 0 auto;
font-family: Tahoma, Verdana, Arial, sans-serif;
}
</style>
</head>
<body>
<h1>No read!</h1>
<p>If you see this page, you have read access to root</p>
</body>
</html>
EOF
# remove read access
chmod 660 $dir/www/data/noread.html
# Http test routine with arguments:
# 1. proto:http/https
function testrun()
@ -134,32 +183,55 @@ EOF
new "wait restconf"
wait_restconf $proto
# echo "curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html"
if $enable; then
echo "curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html"
if ! $enable; then
# XXX or bad request?
new "WWW get html, not enabled, expect not found"
# echo "curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 404"
else
new "WWW get html"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "Content-Type: text/html" "<title>Welcome to Clixon!</title>"
else
new "WWW get html, not enabled, expect bad request"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 400"
return
fi
new "WWW get css"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/example.css)" 0 "HTTP/$HVER 200" "Content-Type: text/css" "display: inline;" --not-- "Content-Type: text/html"
new "WWW get css"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/example.css)" 0 "HTTP/$HVER 200" "Content-Type: text/css" "display: inline;" --not-- "Content-Type: text/html"
new "WWW head"
expectpart "$(curl $CURLOPTS --head -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "Content-Type: text/html" --not-- "<title>Welcome to Clixon!</title>"
new "WWW head"
expectpart "$(curl $CURLOPTS --head -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "Content-Type: text/html" --not-- "<title>Welcome to Clixon!</title>"
new "WWW options"
expectpart "$(curl $CURLOPTS -X OPTIONS $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "allow: OPTIONS,HEAD,GET"
new "WWW options"
expectpart "$(curl $CURLOPTS -X OPTIONS $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "allow: OPTIONS,HEAD,GET"
# negative errors
new "WWW get http not found"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/notfound.html)" 0 "HTTP/$HVER 404" "Content-Type: text/html" "<title>404 Not Found</title>"
# negative errors
new "WWW get http not found"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/notfound.html)" 0 "HTTP/$HVER 404" "Content-Type: text/html" "<title>404 Not Found</title>"
new "WWW post not allowed"
expectpart "$(curl $CURLOPTS -X POST -H 'Accept: text/html' -H "Content-Type: application/yang-data+json" -d '{"ietf-interfaces:interfaces":{"interface":{"name":"eth/0/0","type":"clixon-example:eth","enabled":true}}}' $proto://localhost/data/notfound.html)" 0 "HTTP/$HVER 405" "Content-Type: text/html" "<title>405 Method Not Allowed</title>"
new "WWW get http soft link"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/inside.html)" 0 "HTTP/$HVER 403" "Content-Type: text/html" "<title>403 Forbidden</title>" --not-- "<title>Dont access this</title>"
if [ ! -f /.dockerenv ] ; then # XXX Privs dont not work on docker/alpine?
new "WWW get http not read access"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/noread.html)" 0 "HTTP/$HVER 403" "Content-Type: text/html" "<title>403 Forbidden</title>"
fi
# Try .. Cannot get .. in path to work in curl (it seems to remove it)
if [ "$proto" = http -a -n "$netcat" ]; then
new "WWW get outside using .. netcat"
expectpart "$(${netcat} 127.0.0.1 80 <<EOF
GET /data/../../outside.html HTTP/1.1
Host: localhost
Accept: text_html
EOF
)" 0 "HTTP/1.1 403" "Forbidden"
fi
new "WWW post not allowed"
expectpart "$(curl $CURLOPTS -X POST -H 'Accept: text/html' -H "Content-Type: application/yang-data+json" -d '{"ietf-interfaces:interfaces":{"interface":{"name":"eth/0/0","type":"clixon-example:eth","enabled":true}}}' $proto://localhost/data/notfound.html)" 0 "HTTP/$HVER 405" "Content-Type: text/html" "<title>405 Method Not Allowed</title>"
fi
if [ $RC -ne 0 ]; then
new "Kill restconf daemon"
stop_restconf
@ -190,7 +262,7 @@ if [ "${WITH_RESTCONF}" = "native" ]; then
fi
for proto in $protos; do
for enable in true false; do
for enable in true false; do # false
new "http-data proto:$proto enabled:$enable"
testrun $proto $enable
done

13
test/test_restconf_err.sh
View file

@ -209,19 +209,6 @@ expectpart "$(curl $CURLOPTS -X POST -H 'Content-Type: application/yang-data+xml
new "restconf GET initial datastore"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPROTO://localhost/restconf/data/example:a=0)" 0 "HTTP/$HVER 200" "$XML"
# XXX cannot get this to work for all combinations of nc/netcat fcgi/native
# But leave it here for debugging where netcat works properly
# Alt try something like:
# printf "Hello World!" | (exec 3<>/dev/tcp/127.0.0.1/80; cat >&3; cat <&3; exec 3<&-)
# Look for netcat or nc for direct socket http calls
if [ -n "$(type netcat 2> /dev/null)" ]; then
netcat="netcat -w 1" # -N does not work on fcgi
# nc on freebsd does not work either
#elif [ -n "$(type nc 2> /dev/null)" ]; then
# netcat=nc
else
netcat=
fi
if [ -n "$netcat" -a "${WITH_RESTCONF}" != "fcgi" ]; then
# new "restconf try fuzz crash"

21
yang/clixon/clixon-config@2022-03-21.yang
View file

@ -605,16 +605,28 @@ module clixon-config {
}
leaf CLICON_HTTP_DATA_PATH {
if-feature "clrc:http-data";
default "/";
type string;
description
"If set, enable www data on this sub-path, must start with / (example: /data)";
"URI match for http-data serving files specified by CLICON_HTTP_DATA_ROOT.
Must start with / (example: /)
Restconf paths at /restconf is always done before data (or streams)
The PATH is appended to CLICON_HTTP_DATA_ROOT to find a file.
Example, if PATH is /data and ROOT is /www, and a GET /index.html, the
corresponding file is '/www/data/index.html'
Both feature clixon-restconf:http-data and restconf/enable-http-data
must be enabled for this match to occur.";
}
leaf CLICON_HTTP_DATA_ROOT {
if-feature "clrc:http-data";
type string;
default "/var/www";
description
"public web root";
"Location in file system where http-data files are looked for.
Soft links, '..', '~' etc are not followed.
See also CLICON_HTTP_DATA_PATH
Both feature clixon-restconf:http-data and restconf/enable-http-data
must be enabled for this match to occur.";
}
leaf CLICON_CLI_DIR {
type string;
@ -1051,8 +1063,9 @@ module clixon-config {
leaf CLICON_STREAM_PATH {
type string;
default "streams";
description "Stream path appended to CLICON_STREAM_URL to form
stream subscription URL.";
description
"Stream path appended to CLICON_STREAM_URL to form
stream subscription URL.";
}
leaf CLICON_STREAM_URL {
type string;