diff --git a/apps/restconf/restconf_err.c b/apps/restconf/restconf_err.c index d315a85a..29c87ceb 100644 --- a/apps/restconf/restconf_err.c +++ b/apps/restconf/restconf_err.c @@ -187,14 +187,15 @@ restconf_notimplemented(clicon_handle h, * @param[in] code If 0 use rfc8040 sec 7 netconf2restconf error-tag mapping * otherwise use this code * xerr should be on the form: ... otherwise an internal error is generated + * @note there are special cases see code */ int -api_return_err(clicon_handle h, - void *req, - cxobj *xerr, - int pretty, +api_return_err(clicon_handle h, + void *req, + cxobj *xerr, + int pretty, restconf_media media, - int code0) + int code0) { int retval = -1; cbuf *cb = NULL; @@ -203,6 +204,8 @@ api_return_err(clicon_handle h, char *tagstr; int code; cxobj *xerr2 = NULL; + cxobj *xmsg; + char *mb; clicon_debug(1, "%s", __FUNCTION__); if ((cb = cbuf_new()) == NULL){ @@ -253,12 +256,18 @@ api_return_err(clicon_handle h, * 403 Forbidden If the user is not authorized to access a target resource or invoke * an operation */ - cxobj *xmsg; - char *mb; if ((xmsg = xpath_first(xerr, NULL, "error-message")) != NULL && (mb = xml_body(xmsg)) != NULL && strcmp(mb, "The requested URL was unauthorized") == 0) - code = 401; + code = 401; /* Unauthorized */ + } + /* Special case #2 */ + if (code == 400){ + if (strcmp(tagstr, "invalid-value") == 0 && + (xmsg = xpath_first(xerr, NULL, "error-message")) != NULL && + (mb = xml_body(xmsg)) != NULL && + strcmp(mb, "Invalid HTTP data method") == 0) + code = 404; /* Not found */ } } if (restconf_reply_header(req, "Content-Type", "%s", restconf_media_int2str(media)) < 0) // XXX diff --git a/apps/restconf/restconf_evhtp.c b/apps/restconf/restconf_evhtp.c index 8bfc4c7d..4905b45d 100644 --- a/apps/restconf/restconf_evhtp.c +++ b/apps/restconf/restconf_evhtp.c @@ -465,53 +465,6 @@ evhtp_upgrade_http2(clicon_handle h, } #endif /* HAVE_LIBNGHTTP2 */ -/*! Check common connection sanity checks and terminate if found before request processing - * - * These tests maybe could have done earlier, this is somewhat late since the session is - * closed and that is always good to do as early as possible. - * The following are current checks: - * 1) Check if http/2 non-tls is disabled - * 2) Check if ssl client certs ae valid - * @param[in] h Clixon handle - * @param[in] rc Restconf connection handle - * @param[in] sd Http stream - * @param[out] term Terminate session - * @retval -1 Error - * @retval 0 OK - */ -static int -restconf_evhtp_sanity(clicon_handle h, - restconf_conn *rc, - restconf_stream_data *sd) -{ - int retval = -1; - cxobj *xerr = NULL; - long code; - cbuf *cberr = NULL; - - /* 1) Check if ssl client certs are valid */ - if (rc->rc_ssl != NULL && - (code = SSL_get_verify_result(rc->rc_ssl)) != 0){ - if ((cberr = cbuf_new()) == NULL){ - clicon_err(OE_UNIX, errno, "cbuf_new"); - goto done; - } - cprintf(cberr, "HTTP cert verification failed: (code:%ld)", code); - if (netconf_operation_not_supported_xml(&xerr, "protocol", cbuf_get(cberr)) < 0) - goto done; - if (api_return_err0(sd->sd_conn->rc_h, sd, xerr, 1, YANG_DATA_JSON, 0) < 0) - goto done; - rc->rc_exit = 1; - } - retval = 0; - done: - if (cberr) - cbuf_free(cberr); - if (xerr) - xml_free(xerr); - return retval; -} - /*! Callback for each incoming http request for path / * * This are all messages except /.well-known, Registered with evhtp_set_cb @@ -589,7 +542,7 @@ restconf_path_root(evhtp_request_t *req, goto done; } /* Check sanity of session, eg ssl client cert validation, may set rc_exit */ - if (restconf_evhtp_sanity(h, rc, sd) < 0) + if (restconf_connection_sanity(h, rc, sd) < 0) goto done; if (rc->rc_exit == 0){ #ifdef HAVE_LIBNGHTTP2 @@ -685,7 +638,7 @@ restconf_path_wellknown(evhtp_request_t *req, goto done; } /* Check sanity of session, eg ssl client cert validation, may set rc_exit */ - if (restconf_evhtp_sanity(h, rc, sd) < 0) + if (restconf_connection_sanity(h, rc, sd) < 0) goto done; if (rc->rc_exit == 0){ #ifdef HAVE_LIBNGHTTP2 diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index 1f5c28c0..7cb1b1d3 100644 --- a/apps/restconf/restconf_lib.c +++ b/apps/restconf/restconf_lib.c @@ -124,8 +124,8 @@ Mapping netconf error-tag -> status code */ static const map_str2int netconf_restconf_map[] = { {"in-use", 409}, - {"invalid-value", 404}, - {"invalid-value", 400}, + {"invalid-value", 400}, /* or 404 special case if msg is: "Invalid HTTP data method" handled in api_return_err */ + {"invalid-value", 404}, {"invalid-value", 406}, {"too-big", 413}, /* request */ {"too-big", 400}, /* response */ @@ -136,7 +136,7 @@ static const map_str2int netconf_restconf_map[] = { {"bad-element", 400}, {"unknown-element", 400}, {"unknown-namespace", 400}, - {"access-denied", 403}, /* or 401 special case if tagstr is: "The requested URL was unauthorized" handled in api_return_err */ + {"access-denied", 403}, /* or 401 special case if msg is: "The requested URL was unauthorized" handled in api_return_err */ {"access-denied", 401}, {"lock-denied", 409}, {"resource-denied", 409}, diff --git a/apps/restconf/restconf_native.c b/apps/restconf/restconf_native.c index 92cb3a3c..b946a883 100644 --- a/apps/restconf/restconf_native.c +++ b/apps/restconf/restconf_native.c @@ -64,6 +64,8 @@ /* restconf */ #include "restconf_lib.h" /* generic shared with plugins */ +#include "restconf_handle.h" +#include "restconf_err.h" #ifdef HAVE_LIBEVHTP #include /* evbuffer */ #define EVHTP_DISABLE_REGEX @@ -231,3 +233,78 @@ ssl_x509_name_oneline(SSL *ssl, X509_free(cert); return retval; } + +/*! Check common connection sanity checks and terminate if found before request processing + * + * Tests of sanity of connection not really of an individual request, but is triggered by + * the (first) request in http/1 and http/2 + * These tests maybe could have done earlier, this is somewhat late since the session is + * closed and that is always good to do as early as possible. + * The following are current checks: + * 1) Check if http/2 non-tls is disabled + * 2) Check if ssl client certs ae valid + * @param[in] h Clixon handle + * @param[in] rc Restconf connection handle + * @param[in] sd Http stream + * @param[out] term Terminate session + * @retval -1 Error + * @retval 0 OK + */ +int +restconf_connection_sanity(clicon_handle h, + restconf_conn *rc, + restconf_stream_data *sd) +{ + int retval = -1; + cxobj *xerr = NULL; + long code; + cbuf *cberr = NULL; + restconf_media media_out = YANG_DATA_JSON; + char *media_str = NULL; + + /* 1) Check if http/2 non-tls is disabled */ + if (rc->rc_ssl == NULL && + rc->rc_proto == HTTP_2 && + clicon_option_bool(h, "CLICON_RESTCONF_HTTP2_PLAIN") == 0){ + if (netconf_invalid_value_xml(&xerr, "protocol", "Non-tls HTTP/2 is disabled") < 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 (api_return_err0(h, sd, xerr, 1, media_out, 0) < 0) + goto done; + rc->rc_exit = 1; + } + /* 2) Check if ssl client cert is valid */ + else if (rc->rc_ssl != NULL && + (code = SSL_get_verify_result(rc->rc_ssl)) != 0){ + if ((cberr = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cberr, "HTTP cert verification failed, unknown ca: (code:%ld)", 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 (api_return_err0(sd->sd_conn->rc_h, sd, xerr, 1, media_out, 0) < 0) + goto done; + rc->rc_exit = 1; + } + retval = 0; + done: + if (cberr) + cbuf_free(cberr); + if (xerr) + xml_free(xerr); + return retval; +} diff --git a/apps/restconf/restconf_native.h b/apps/restconf/restconf_native.h index 49aabf43..3d6f0c17 100644 --- a/apps/restconf/restconf_native.h +++ b/apps/restconf/restconf_native.h @@ -138,7 +138,9 @@ restconf_conn *restconf_conn_new(clicon_handle h, int s); int restconf_conn_free(restconf_conn *rc); int ssl_x509_name_oneline(SSL *ssl, char **oneline); -int restconf_close_ssl_socket(restconf_conn *rc, int shutdown); /* XXX in restconf_main_native.c */ +int restconf_close_ssl_socket(restconf_conn *rc, int shutdown); /* XXX in restconf_main_native.c */ +int restconf_connection_sanity(clicon_handle h, restconf_conn *rc, restconf_stream_data *sd); + #endif /* _RESTCONF_NATIVE_H_ */ diff --git a/apps/restconf/restconf_nghttp2.c b/apps/restconf/restconf_nghttp2.c index 6da6e395..92f65dd3 100644 --- a/apps/restconf/restconf_nghttp2.c +++ b/apps/restconf/restconf_nghttp2.c @@ -277,62 +277,6 @@ recv_callback(nghttp2_session *session, return 0; } -/*! Check common connection sanity checks and terminate if found before request processing - * - * These tests maybe could have done earlier, this is somewhat late since the session is - * closed and that is always good to do as early as possible. - * The following are current checks: - * 1) Check if http/2 non-tls is disabled - * 2) Check if ssl client certs ae valid - * @param[in] h Clixon handle - * @param[in] rc Restconf connection handle - * @param[in] sd Http stream - * @param[out] term Terminate session - * @retval -1 Error - * @retval 0 OK - */ -static int -restconf_nghttp2_sanity(clicon_handle h, - restconf_conn *rc, - restconf_stream_data *sd) -{ - int retval = -1; - cxobj *xerr = NULL; - long code; - cbuf *cberr = NULL; - - /* 1) Check if http/2 non-tls is disabled */ - if (rc->rc_ssl == NULL && - clicon_option_bool(h, "CLICON_RESTCONF_HTTP2_PLAIN") == 0){ - if (netconf_operation_not_supported_xml(&xerr, "protocol", "Non-tls HTTP/2 is disabled") < 0) - goto done; - if (api_return_err0(h, sd, xerr, 1, YANG_DATA_JSON, 0) < 0) - goto done; - rc->rc_exit = 1; - } - /* 2) Check if ssl client certs ae valid */ - else if (rc->rc_ssl != NULL && - (code = SSL_get_verify_result(rc->rc_ssl)) != 0){ - if ((cberr = cbuf_new()) == NULL){ - clicon_err(OE_UNIX, errno, "cbuf_new"); - goto done; - } - cprintf(cberr, "HTTP cert verification failed: (code:%ld)", code); - if (netconf_operation_not_supported_xml(&xerr, "protocol", cbuf_get(cberr)) < 0) - goto done; - if (api_return_err0(sd->sd_conn->rc_h, sd, xerr, 1, YANG_DATA_JSON, 0) < 0) - goto done; - rc->rc_exit = 1; - } - retval = 0; - done: - if (cberr) - cbuf_free(cberr); - if (xerr) - xml_free(xerr); - return retval; -} - /*! Callback for each incoming http request for path / * * This are all messages except /.well-known, Registered with evhtp_set_cb @@ -361,10 +305,6 @@ restconf_nghttp2_path(restconf_stream_data *sd) clicon_err(OE_RESTCONF, EINVAL, "arg is NULL"); goto done; } - if (restconf_nghttp2_sanity(h, rc, sd) < 0) - goto done; - if (rc->rc_exit == 1) - goto ok; if (rc->rc_ssl != NULL){ /* Slightly awkward way of taking SSL cert subject and CN and add it to restconf parameters * instead of accessing it directly @@ -380,17 +320,21 @@ restconf_nghttp2_path(restconf_stream_data *sd) } } } - /* call generic function */ - if (strcmp(sd->sd_path, RESTCONF_WELL_KNOWN) == 0){ - if (api_well_known(h, sd) < 0) + /* Check sanity of session, eg ssl client cert validation, may set rc_exit */ + if (restconf_connection_sanity(h, rc, sd) < 0) + goto done; + if (!rc->rc_exit){ + /* call generic function */ + if (strcmp(sd->sd_path, RESTCONF_WELL_KNOWN) == 0){ + if (api_well_known(h, sd) < 0) + goto done; + } + else if (api_root_restconf(h, sd, sd->sd_qvec) < 0) goto done; } - else if (api_root_restconf(h, sd, sd->sd_qvec) < 0) - goto done; /* Clear (fcgi) paramaters from this request */ if (restconf_param_del_all(h) < 0) goto done; - ok: retval = 0; done: clicon_debug(1, "%s %d", __FUNCTION__, retval); diff --git a/test/test_restconf_ssl_certs.sh b/test/test_restconf_ssl_certs.sh index 4d7ee112..af61316e 100755 --- a/test/test_restconf_ssl_certs.sh +++ b/test/test_restconf_ssl_certs.sh @@ -290,13 +290,16 @@ EOF # expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 400" new "limited invalid cert" - expectpart "$(curl $CURLOPTS --key $certdir/limited.key --cert $certdir/limited.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 405" "HTTP cert verification failed" + expectpart "$(curl $CURLOPTS --key $certdir/limited.key --cert $certdir/limited.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 400" "\"error-message\": \"HTTP cert verification failed, unknown ca" + 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, unknown ca" + new "too weak cert (sign w md5)" - expectpart "$(curl $CURLOPTS --key $certdir/mymd5.key --cert $certdir/mymd5.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" "35 58" # "md too weak" + expectpart "$(curl $CURLOPTS --key $certdir/mymd5.key --cert $certdir/mymd5.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" "35 58" # "md too weak" new "Random cert" - expectpart "$(curl $CURLOPTS --key $certdir/random.key --cert $certdir/random.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 405" "HTTP cert verification failed" + expectpart "$(curl $CURLOPTS --key $certdir/random.key --cert $certdir/random.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 "HTTP/$HVER 400" "HTTP cert verification failed" # Havent been able to generate "wrong CA" # new "invalid cert from wrong CA"