diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bc20083..5a756b74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Expected: October 2024 ### Features +* Restconf: Support for list of media in Accept header * Refactoring of schema mount-points * Add new top-level `Y_MOUNTS` and add top-level yangs and mountpoints in yspecs * New `clixon-autocli@2024-08-01.yang` revision @@ -34,6 +35,7 @@ Expected: October 2024 Users may have to change how they access the system +* Restconf: Better RFC compliance with Accept errors: 406 vs 415 * Removed YANG line-number in error-messages for memory optimization * Re-enable by setting `YANG_SPEC_LINENR` compile-time option * NETCONF error returns of failed leafref references, see https://github.com/clicon/clixon/issues/536 @@ -53,6 +55,7 @@ Developers may need to change their code ### Corrected Busg +* Fixed: [RESTCONF exit on cert error + complex accept](https://github.com/clicon/clixon/issues/551) * Fixed: [Deletion of leaf in YANG choice removes leaf in a different choice/case](https://github.com/clicon/clixon/issues/542) * Fixed: Deviated types were resolved in target context, not lexically in deviation context * Fixed: Signal handling of recv message diff --git a/apps/restconf/clixon_http_data.c b/apps/restconf/clixon_http_data.c index 6db5f676..790745f2 100644 --- a/apps/restconf/clixon_http_data.c +++ b/apps/restconf/clixon_http_data.c @@ -416,7 +416,7 @@ api_http_data(clixon_handle h, { int retval = -1; char *request_method = NULL; - char *media_str = NULL; + char *media_list = NULL; int head = 0; int options = 0; int ret; @@ -469,14 +469,17 @@ api_http_data(clixon_handle h, } /* 5. Accepted media_out: should check text/html, JavaScript, image, and css */ - if ((media_str = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ - } - else if (strcmp(media_str, "*/*") != 0 && - strcmp(media_str, "text/html") != 0){ -#ifdef NOTYET - clixon_log(h, LOG_NOTICE, "%s: media error %s", __FUNCTION__, media_str); - goto done; -#endif + if ((media_list = restconf_param_get(h, "HTTP_ACCEPT")) != NULL){ + if (restconf_media_in_list("text/html", media_list) != 1 && + restconf_media_in_list("*/*", media_list) != 1 + && 0) { /* XXX: not yet */ + /* If the server does not support any of the requested + * output encodings for a request, then it MUST return an error response + * with a "406 Not Acceptable" status-line. */ + if (restconf_not_acceptable(h, req, 1, HTTP_DATA_TEXT_HTML) < 0) + goto done; + goto ok; + } } /* 6. Authenticate * Note, error handling may need change since it is restconf based diff --git a/apps/restconf/clixon_restconf.h b/apps/restconf/clixon_restconf.h index 7aa5e5b9..ef5cf073 100644 --- a/apps/restconf/clixon_restconf.h +++ b/apps/restconf/clixon_restconf.h @@ -55,6 +55,7 @@ enum restconf_media{ YANG_PATCH_JSON, /* "application/yang-patch+json" */ YANG_PATCH_XML, /* "application/yang-patch+xml" */ YANG_PAGINATION_XML, /* draft-wwlh-netconf-list-pagination-rc-02.txt */ + HTTP_DATA_TEXT_HTML /* For http_data */ /* For JSON, the existing "application/yang-data+json" media type is sufficient, as the JSON format has built-in support for encoding arrays. */ diff --git a/apps/restconf/restconf_err.c b/apps/restconf/restconf_err.c index ebf0b0f0..3525e6f9 100644 --- a/apps/restconf/restconf_err.c +++ b/apps/restconf/restconf_err.c @@ -102,6 +102,35 @@ restconf_method_notallowed(clixon_handle h, return retval; } +/*! HTTP error 406 Not acceptable + * + * @param[in] req Generic http handle + * @retval 0 OK + * @retval -1 Error + * @see RFC8040, section 5.2: + * If the server does not support any of the requested output encodings for a request, then it MUST + * return an error response with a "406 Not Acceptable" status-line. + */ +int +restconf_not_acceptable(clixon_handle h, + void *req, + int pretty, + restconf_media media) +{ + int retval = -1; + cxobj *xerr = NULL; + + if (netconf_operation_not_supported_xml(&xerr, "protocol", "Unacceptable output encoding") < 0) + goto done; + if (api_return_err0(h, req, xerr, pretty, media, 406) < 0) + goto done; + if (restconf_reply_send(req, 406, NULL, 0) < 0) + goto done; + retval = 0; + done: + return retval; +} + /*! HTTP error 415 Unsupported media * * @param[in] req Generic http handle @@ -132,36 +161,6 @@ restconf_unsupported_media(clixon_handle h, return retval; } -/*! HTTP error 406 Not acceptable - * - * @param[in] req Generic http handle - * @retval 0 OK - * @retval -1 Error - * @see RFC8040, section 5.2: - * If the server does not support any of the requested output encodings for a request, then it MUST - * return an error response with a "406 Not Acceptable" status-line. - */ -int -restconf_not_acceptable(clixon_handle h, - void *req, - int pretty, - restconf_media media) -{ - int retval = -1; - cxobj *xerr = NULL; - - if (netconf_operation_not_supported_xml(&xerr, "protocol", "Unacceptable output encoding") < 0) - goto done; - /* Override with 415 netconf->restoconf translation which gives a 405 */ - if (api_return_err0(h, req, xerr, pretty, media, 415) < 0) - goto done; - if (restconf_reply_send(req, 415, NULL, 0) < 0) - goto done; - retval = 0; - done: - return retval; -} - /*! HTTP error 501 Not implemented * @param[in] req Generic http handle */ @@ -286,6 +285,7 @@ api_return_err(clixon_handle h, case YANG_DATA_XML: case YANG_PATCH_XML: case YANG_PAGINATION_XML: + case HTTP_DATA_TEXT_HTML: clixon_debug(CLIXON_DBG_RESTCONF, "code:%d", code); if (pretty){ cprintf(cb, " \n"); @@ -302,6 +302,7 @@ api_return_err(clixon_handle h, break; case YANG_DATA_JSON: case YANG_PATCH_JSON: + default: /* Override -1 with JSON return, not technically correct */ clixon_debug(CLIXON_DBG_RESTCONF, "code:%d", code); if (pretty){ cprintf(cb, "{\n\"ietf-restconf:errors\" : "); @@ -317,10 +318,11 @@ api_return_err(clixon_handle h, cprintf(cb, "}\r\n"); } break; - default: /* Just ignore the body so that there is a reply */ - clixon_err(OE_YANG, EINVAL, "Invalid media type %d", media); - goto done; +#if 0 /* Maybe this is correct, but content-type may not yet be known */ + default: /* Override -1 or anything else to ensure there is an error reply */ + cprintf(cb, "\r\n\r\n"); break; +#endif } /* switch media */ assert(cbuf_len(cb)); if (restconf_reply_send(req, code, cb, 0) < 0) @@ -363,6 +365,7 @@ api_return_err0(clixon_handle h, int retval = -1; cxobj *xe; + clixon_debug(CLIXON_DBG_RESTCONF, ""); if ((xe = xml_find_type(xerr, NULL, "rpc-error", CX_ELMNT)) == NULL){ clixon_err(OE_XML, EINVAL, "Expected xml on the form .."); goto done; diff --git a/apps/restconf/restconf_err.h b/apps/restconf/restconf_err.h index 454690d5..0fbdb521 100644 --- a/apps/restconf/restconf_err.h +++ b/apps/restconf/restconf_err.h @@ -44,8 +44,8 @@ */ int restconf_method_notallowed(clixon_handle h, void *req, char *allow, int pretty, restconf_media media); -int restconf_unsupported_media(clixon_handle h, void *req, int pretty, restconf_media media); int restconf_not_acceptable(clixon_handle h, void *req, int pretty, restconf_media media); +int restconf_unsupported_media(clixon_handle h, void *req, int pretty, restconf_media media); int restconf_notimplemented(clixon_handle h, void *req, int pretty, restconf_media media); int api_return_err(clixon_handle h, void *req, cxobj *xerr, int pretty, restconf_media media, int code0); diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index 1b830f21..d00ed8e4 100644 --- a/apps/restconf/restconf_lib.c +++ b/apps/restconf/restconf_lib.c @@ -208,6 +208,7 @@ static const map_str2int http_media_map[] = { {"application/yang-patch+xml", YANG_PATCH_XML}, {"application/yang-patch+json", YANG_PATCH_JSON}, {"application/yang-data+xml-list", YANG_PAGINATION_XML}, /* draft-wwlh-netconf-list-pagination-rc-02 */ + {"text/html", HTTP_DATA_TEXT_HTML}, /* for http_data */ {NULL, -1} }; @@ -231,12 +232,82 @@ restconf_code2reason(int code) return clicon_int2str(http_reason_phrase_map, code); } +/*! One media to integer representation + */ const restconf_media restconf_media_str2int(char *media) { return clicon_str2int(http_media_map, media); } +/*! Select first registered media in media list to integer + * + * Example: list="imag/avif,application/yang-data+xml,**" + * returns: YANG_DATA_XML + * Note that if no registered medias are found, then return -1 + * Filters any "; q=.." + * @param[in] list + * @retval -1 No registered media found + */ +const restconf_media +restconf_media_list_str2int(char *list) +{ + int reti = -1; + cg_var *cv; + cvec *cvv = NULL; + char *str; + + if (uri_str2cvec(list, ',', ';', 0, &cvv) < 0) + return -1; + cv = NULL; + while ((cv = cvec_each(cvv, cv)) != NULL){ + str = cv_name_get(cv); + if ((reti = clicon_str2int(http_media_map, str)) != -1) + break; + } + if (cvv) + cvec_free(cvv); + return reti; +} + +/*! Check if media exists in media string + * + * Example: media="text/html"; + * list="img/avif,text/html,application/yang-data+xml,**" + * Returns: 1 + * @param[in] media Single media string + * @param[in] list Comma-separated list of medias + * @retval 1 Found + * @retval 0 Not found + * @retval -1 Error + */ +int +restconf_media_in_list(char *media, + char *list) +{ + int retval = -1; + cg_var *cv; + cvec *cvv = NULL; + char *str; + + if (uri_str2cvec(list, ',', ';', 0, &cvv) < 0) + goto done; + cv = NULL; + while ((cv = cvec_each(cvv, cv)) != NULL){ + str = cv_name_get(cv); + if (strcmp(str, media) == 0) + break; + } + if (cv != NULL) + retval = 1; + else + retval = 0; + done: + if (cvv) + cvec_free(cvv); + return retval; +} + const char * restconf_media_int2str(restconf_media media) { diff --git a/apps/restconf/restconf_lib.h b/apps/restconf/restconf_lib.h index 93fbdbd5..3eea7352 100644 --- a/apps/restconf/restconf_lib.h +++ b/apps/restconf/restconf_lib.h @@ -55,6 +55,7 @@ enum restconf_media{ YANG_PATCH_JSON, /* "application/yang-patch+json" */ YANG_PATCH_XML, /* "application/yang-patch+xml" */ YANG_PAGINATION_XML, /* draft-wwlh-netconf-list-pagination-rc-02.txt */ + HTTP_DATA_TEXT_HTML /* For http_data */ }; typedef enum restconf_media restconf_media; @@ -84,6 +85,8 @@ typedef enum restconf_http_proto restconf_http_proto; int restconf_err2code(char *tag); const char *restconf_code2reason(int code); const restconf_media restconf_media_str2int(char *media); +int restconf_media_in_list(char *media, char *list); +const restconf_media restconf_media_list_str2int(char *list); const char *restconf_media_int2str(restconf_media media); int restconf_str2proto(char *str); const char *restconf_proto2str(int proto); diff --git a/apps/restconf/restconf_native.c b/apps/restconf/restconf_native.c index f4db752a..91cb9450 100644 --- a/apps/restconf/restconf_native.c +++ b/apps/restconf/restconf_native.c @@ -320,7 +320,7 @@ restconf_connection_sanity(clixon_handle h, long code; cbuf *cberr = NULL; restconf_media media_out = YANG_DATA_JSON; - char *media_str = NULL; + char *media_list = NULL; char *oneline = NULL; clixon_debug(CLIXON_DBG_RESTCONF, ""); @@ -331,12 +331,11 @@ restconf_connection_sanity(clixon_handle h, clixon_debug(CLIXON_DBG_RESTCONF, "Sanity check http/2 non-tls failed, disconnect"); if (netconf_invalid_value_xml(&xerr, "protocol", "Only HTTP/2 with TLS is enabled, plain http/2 is disabled") < 0) goto done; - if ((media_str = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ + if ((media_list = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ media_out = YANG_DATA_JSON; } - else if ((int)(media_out = restconf_media_str2int(media_str)) == -1){ - if (strcmp(media_str, "*/*") == 0) /* catch-all */ - media_out = YANG_DATA_JSON; + else { + media_out = restconf_media_list_str2int(media_list); } if (api_return_err0(h, sd, xerr, 1, media_out, 0) < 0) goto done; @@ -362,12 +361,8 @@ restconf_connection_sanity(clixon_handle h, X509_verify_cert_error_string(code), code); if (netconf_invalid_value_xml(&xerr, "protocol", cbuf_get(cberr)) < 0) goto done; - if ((media_str = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ - media_out = YANG_DATA_JSON; - } - else if ((int)(media_out = restconf_media_str2int(media_str)) == -1){ - if (strcmp(media_str, "*/*") == 0) /* catch-all */ - media_out = YANG_DATA_JSON; + if ((media_list = restconf_param_get(h, "HTTP_ACCEPT")) != NULL){ + media_out = restconf_media_list_str2int(media_list); } if (api_return_err0(sd->sd_conn->rc_h, sd, xerr, 1, media_out, 0) < 0) goto done; diff --git a/apps/restconf/restconf_nghttp2.c b/apps/restconf/restconf_nghttp2.c index 4417ed18..560e9122 100644 --- a/apps/restconf/restconf_nghttp2.c +++ b/apps/restconf/restconf_nghttp2.c @@ -625,6 +625,7 @@ on_frame_recv_callback(nghttp2_session *session, } retval = 0; done: + clixon_debug(CLIXON_DBG_RESTCONF|CLIXON_DBG_DETAIL, "retval:%d", retval); return retval; } @@ -969,7 +970,7 @@ http2_recv(restconf_conn *rc, * when |session| was configured as server and * `nghttp2_option_set_no_recv_client_magic()` is not used with * nonzero value. */ - clixon_log(NULL, LOG_INFO, "%s Received bad client magic byte strin", __FUNCTION__); + clixon_log(NULL, LOG_INFO, "%s Received bad client magic byte string", __FUNCTION__); /* unsure if this does anything, byt does not seem to hurt */ if ((ngerr = nghttp2_session_terminate_session(rc->rc_ngsession, ngerr)) < 0) clixon_err(OE_NGHTTP2, ngerr, "nghttp2_session_terminate_session %d", ngerr); diff --git a/apps/restconf/restconf_root.c b/apps/restconf/restconf_root.c index 03741e1b..83dcc036 100644 --- a/apps/restconf/restconf_root.c +++ b/apps/restconf/restconf_root.c @@ -474,7 +474,7 @@ api_root_restconf(clixon_handle h, int pn; int pretty; cbuf *cb = NULL; - char *media_str = NULL; + char *media_list = NULL; restconf_media media_out = YANG_DATA_JSON; char *indata = NULL; char *username = NULL; @@ -495,23 +495,21 @@ api_root_restconf(clixon_handle h, * operation POST, etc * If accept is * default is yang-json */ - if ((media_str = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ -#if 0 /* Use default +json */ - if (restconf_not_acceptable(h, r, pretty, media_out) < 0) - goto done; - goto ok; -#endif - } - else if ((int)(media_out = restconf_media_str2int(media_str)) == -1){ - if (strcmp(media_str, "*/*") == 0) /* catch-all */ - media_out = YANG_DATA_JSON; - else{ - if (restconf_not_acceptable(h, req, pretty, YANG_DATA_JSON) < 0) - goto done; - goto ok; + if ((media_list = restconf_param_get(h, "HTTP_ACCEPT")) != NULL){ + if ((int)(media_out = restconf_media_list_str2int(media_list)) == -1) { + if (restconf_media_in_list("*/*", media_list) == 1) + media_out = YANG_DATA_JSON; + else{ + /* If the server does not support any of the requested + * output encodings for a request, then it MUST return an error response + * with a "406 Not Acceptable" status-line. */ + if (restconf_not_acceptable(h, req, pretty, YANG_DATA_JSON) < 0) + goto done; + goto ok; + } } } - clixon_debug(CLIXON_DBG_RESTCONF, "ACCEPT: %s %s", media_str, restconf_media_int2str(media_out)); + clixon_debug(CLIXON_DBG_RESTCONF, "ACCEPT: %s %s", media_list, restconf_media_int2str(media_out)); if ((pvec = clicon_strsep(path, "/", &pn)) == NULL) goto done; diff --git a/apps/restconf/restconf_stream.c b/apps/restconf/restconf_stream.c index e7265cdd..e906011d 100644 --- a/apps/restconf/restconf_stream.c +++ b/apps/restconf/restconf_stream.c @@ -234,7 +234,7 @@ api_stream(clixon_handle h, int pretty; int besock = -1; restconf_media media_reply = YANG_DATA_XML; - char *media_str = NULL; + char *media_list = NULL; char *stream_name; int ret; @@ -259,15 +259,14 @@ api_stream(clixon_handle h, } /* Get media for output (proactive negotiation) RFC7231 by using */ - media_str = restconf_param_get(h, "HTTP_ACCEPT"); - if (media_str == NULL){ + if ((media_list = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ if (restconf_not_acceptable(h, req, pretty, media_reply) < 0) goto done; goto ok; } /* Accept only text_event-stream or */ - if (strcmp(media_str, "*/*") != 0 && - strcmp(media_str, "text/event-stream") != 0){ + if (restconf_media_in_list("text/event-stream", media_list) != 1 && + restconf_media_in_list("*/*", media_list) != 1) { if (restconf_not_acceptable(h, req, pretty, media_reply) < 0) goto done; goto ok; diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 9dfb2047..1d0f8114 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -1577,9 +1577,10 @@ upgrade_callback_call(clixon_handle h, goto done; } -/* Authentication type +/*! Authentication type + * * @see http-auth-type in clixon-restconf.yang - * @see restconf_media_str2int + * @see clixon_auth_type_str2int */ static const map_str2int clixon_auth_type[] = { {"none", CLIXON_AUTH_NONE}, diff --git a/test/test_http_data.sh b/test/test_http_data.sh index 60aca6ad..acb4ffa0 100755 --- a/test/test_http_data.sh +++ b/test/test_http_data.sh @@ -205,7 +205,19 @@ EOF new "WWW get index.html" expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "Content-Type: text/html" "Welcome to Clixon!" - + + new "List of medias" + expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html,*/*' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 200" "Content-Type: text/html" "Welcome to Clixon!" + + new "List of medias2" + expectpart "$(curl $CURLOPTS -X GET -H 'Accept: wrong/media,*/*' $proto://localhost/data/index.html)" 0 "HT +TP/$HVER 200" "Content-Type: text/html" "Welcome to Clixon!" + +if false; then # XXX Se step 5 in api_http_data, unclear which media should be accepted + new "Server does not support list of medias Expect 406" + expectpart "$(curl $CURLOPTS -X GET -H 'Accept: wrong/media' $proto://localhost/data/index.html)" 0 "HTTP/$HVER 406" "content-type: text/html" "Unacceptable output encoding" +fi + new "WWW get dir -> expect index.html" expectpart "$(curl $CURLOPTS -X GET -H 'Accept: text/html' $proto://localhost/data)" 0 "HTTP/$HVER 200" "Content-Type: text/html" "Welcome to Clixon!" @@ -259,7 +271,7 @@ EOF expectpart "$(${netcat} 127.0.0.1 80 <&1)" 0 "HTTP/$HVER 400" "\"error-message\": \"HTTP cert verification failed: certificate has expired" + # https://github.com/clicon/clixon/issues/551 + new "limited invalid cert, multiple media" + expectpart "$(curl $CURLOPTS --key $certdir/limited.key --cert $certdir/limited.crt -H "Accept: application/yang-data+xml,*/*" -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 400" "HTTP cert verification failed: certificate has expired" new "limited invalid cert, xml" expectpart "$(curl $CURLOPTS --key $certdir/limited.key --cert $certdir/limited.crt -H "Accept: application/yang-data+xml" -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 400" "HTTP cert verification failed: certificate has expired"