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

Native restconf: SSL client certs failures are returned as http 405 errors, not fail during SSL negotiation

Browse source
This commit is contained in:
Olof hagsand 2021-08-27 09:33:59 +02:00
parent 7794c619cc
commit cdacca125c
6 changed files with 195 additions and 58 deletions
Download patch file Download diff file Expand all files Collapse all files

6
CHANGELOG.md
View file

@ -57,8 +57,10 @@ Users may have to change how they access the system
* See changes under new feature "YANG leafref feature update"
* Validation of referred node type (not referring)
* Leafref required-instance must be set to make strict data-node check
* Native Restconf is now default, not fcgi/nginx
* That is, to configure with fcgi, you need to explicitly configure: `--with-restconf=fcgi`
* Native Restconf
* Native restocnf is now default, not fcgi/nginx
* That is, to configure with fcgi, you need to explicitly configure: `--with-restconf=fcgi`
* SSL client certs failures are returned as http 405 errors, not fail during SSL negotiation
* New clixon-config@2021-07-11.yang revision
* Added: `CLICON_RESTCONF_HTTP2_PLAIN`
* Removed default of `CLICON_RESTCONF_INSTALLDIR`

107
apps/restconf/restconf_evhtp.c
View file

@ -465,6 +465,53 @@ 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
@ -518,7 +565,7 @@ restconf_path_root(evhtp_request_t *req,
/* Query vector, ie the ?a=x&b=y stuff */
if (sd->sd_qvec)
cvec_free(sd->sd_qvec);
if ((sd->sd_qvec = cvec_new(0)) ==NULL){
if ((sd->sd_qvec = cvec_new(0)) == NULL){
clicon_err(OE_UNIX, errno, "cvec_new");
evhtp_internal_error(req);
goto done;
@ -541,21 +588,26 @@ restconf_path_root(evhtp_request_t *req,
evhtp_internal_error(req);
goto done;
}
/* Check sanity of session, eg ssl client cert validation, may set rc_exit */
if (restconf_evhtp_sanity(h, rc, sd) < 0)
goto done;
if (rc->rc_exit == 0){
#ifdef HAVE_LIBNGHTTP2
if (ret == 1){
if ((ret = evhtp_upgrade_http2(h, sd)) < 0){
evhtp_internal_error(req);
goto done;
if (ret == 1){
if ((ret = evhtp_upgrade_http2(h, sd)) < 0){
evhtp_internal_error(req);
goto done;
}
if (ret == 0)
keep_params = 1;
}
if (ret == 0)
keep_params = 1;
}
#endif
if (ret == 1){
/* call generic function */
if (api_root_restconf(h, sd, sd->sd_qvec) < 0){
evhtp_internal_error(req);
goto done;
if (ret == 1){
/* call generic function */
if (api_root_restconf(h, sd, sd->sd_qvec) < 0){
evhtp_internal_error(req);
goto done;
}
}
}
/* Clear input request parameters from this request */
@ -632,21 +684,26 @@ restconf_path_wellknown(evhtp_request_t *req,
evhtp_internal_error(req);
goto done;
}
/* Check sanity of session, eg ssl client cert validation, may set rc_exit */
if (restconf_evhtp_sanity(h, rc, sd) < 0)
goto done;
if (rc->rc_exit == 0){
#ifdef HAVE_LIBNGHTTP2
if (ret == 1){
if ((ret = evhtp_upgrade_http2(h, sd)) < 0){
evhtp_internal_error(req);
goto done;
if (ret == 1){
if ((ret = evhtp_upgrade_http2(h, sd)) < 0){
evhtp_internal_error(req);
goto done;
}
if (ret == 0)
keep_params = 1;
}
if (ret == 0)
keep_params = 1;
}
#endif
if (ret == 1){
/* call generic function */
if (api_well_known(h, sd) < 0){
evhtp_internal_error(req);
goto done;
if (ret == 1){
/* call generic function */
if (api_well_known(h, sd) < 0){
evhtp_internal_error(req);
goto done;
}
}
}
/* Clear input request parameters from this request */

48
apps/restconf/restconf_main_native.c
View file

@ -421,7 +421,13 @@ restconf_verify_certs(int preverify_ok,
/* Verify the CA name */
}
// h = SSL_get_app_data(ssl);
return preverify_ok;
/* Different schemes for return values if failure detected:
* - 0 (preferity_ok) the session terminates here in SSL negotiation, an http client
* will get a low level error (not http reply)
* - 1 Check if the cert is valid using SSL_get_verify_result(rc->rc_ssl)
* @see restconf_evhtp_sanity and restconf_nghttp2_sanity where this is done for http/1 and http/2
*/
return 1;
}
/*! Debug print of all incoming alpn alternatives, eg h2 and http/1.1
@ -851,6 +857,19 @@ restconf_connection(int s,
"<errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"><error><error-type>protocol</error-type><error-tag>malformed-message</error-tag><error-message>No evhtp output</error-message></error></errors>") < 0)
goto done;
}
if (rc->rc_exit){ /* Server-initiated exit for http/2 */
SSL_free(rc->rc_ssl);
rc->rc_ssl = NULL;
evconn->ssl = NULL;
if (close(rc->rc_s) < 0){
clicon_err(OE_UNIX, errno, "close");
goto done;
}
clixon_event_unreg_fd(rc->rc_s, restconf_connection);
clicon_debug(1, "%s evconn-free (%p) 2", __FUNCTION__, evconn);
restconf_conn_free(rc);
goto ok;
}
#ifdef HAVE_LIBNGHTTP2
if (sd->sd_upgrade2){
nghttp2_error ngerr;
@ -900,16 +919,23 @@ restconf_connection(int s,
#endif /* HAVE_LIBEVHTP */
#ifdef HAVE_LIBNGHTTP2
case HTTP_2:
if ((ret = http2_recv(rc, (unsigned char *)buf, n)) < 0)
goto done;
if (ret == 0){
restconf_close_ssl_socket(rc, 1);
if (restconf_conn_free(rc) < 0)
goto done;
goto ok;
if (rc->rc_exit){ /* Server-initiated exit for http/2 */
nghttp2_error ngerr;
if ((ngerr = nghttp2_session_terminate_session(rc->rc_ngsession, 0)) < 0)
clicon_err(OE_NGHTTP2, ngerr, "nghttp2_session_terminate_session %d", ngerr);
}
else {
if ((ret = http2_recv(rc, (unsigned char *)buf, n)) < 0)
goto done;
if (ret == 0){
restconf_close_ssl_socket(rc, 1);
if (restconf_conn_free(rc) < 0)
goto done;
goto ok;
}
/* There may be more data frames */
readmore++;
}
/* There may be more data frames */
readmore++;
break;
#endif /* HAVE_LIBNGHTTP2 */
default:
@ -1274,9 +1300,7 @@ restconf_accept_client(int fd,
* Note this _only_ works if SSL_set1_host() was set previously,...
*/
if (SSL_get_verify_result(rc->rc_ssl) == X509_V_OK) { /* for peer cert */
const char *peername = SSL_get0_peername(rc->rc_ssl);
if (peername != NULL) {
/* Name checks were in scope and matched the peername */
clicon_debug(1, "%s peername:%s", __FUNCTION__, peername);

3
apps/restconf/restconf_native.h
View file

@ -62,7 +62,7 @@ extern "C" {
/* Forward */
struct restconf_conn;
/* session stream struct, mainly for http/2 but htp/1 has a single pseudo-stream with id=0
/* session stream struct, mainly for http/2 but http/1 has a single pseudo-stream with id=0
*/
typedef struct {
qelem_t sd_qelem; /* List header */
@ -95,6 +95,7 @@ typedef struct restconf_conn {
clicon_handle rc_h; /* Clixon handle */
SSL *rc_ssl; /* Structure for SSL connection */
restconf_stream_data *rc_streams; /* List of http/2 session streams */
int rc_exit; /* Set to close socket server-side (NYI) */
/* Decision to keep lib-specific data here, otherwise new struct necessary
* drawback is specific includes need to go everywhere */
#ifdef HAVE_LIBEVHTP

80
apps/restconf/restconf_nghttp2.c
View file

@ -277,6 +277,62 @@ 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
@ -298,26 +354,18 @@ restconf_nghttp2_path(restconf_stream_data *sd)
char *oneline = NULL;
cvec *cvv = NULL;
char *cn;
clicon_debug(1, "------------");
rc = sd->sd_conn;
if ((h = rc->rc_h) == NULL){
clicon_err(OE_RESTCONF, EINVAL, "arg is NULL");
goto done;
}
if (rc->rc_ssl == NULL){
if (clicon_option_bool(h, "CLICON_RESTCONF_HTTP2_PLAIN") == 0){
cxobj *xerr = NULL;
if (netconf_operation_not_supported_xml(&xerr, "protocol", "HTTP/2 plain / non-tls is not allowed") < 0)
goto done;
if (api_return_err0(h, sd, xerr, 1, YANG_DATA_JSON, 0) < 0)
goto done;
goto ok;
}
}
else {
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
* SSL subject fields, eg CN (Common Name) , can add more here? */
@ -339,10 +387,10 @@ restconf_nghttp2_path(restconf_stream_data *sd)
}
else if (api_root_restconf(h, sd, sd->sd_qvec) < 0)
goto done;
ok:
/* 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);
@ -894,7 +942,7 @@ http2_recv(restconf_conn *rc,
{
int retval = -1;
nghttp2_error ngerr;
clicon_debug(1, "%s", __FUNCTION__);
if (rc->rc_ngsession == NULL){
/* http2_session_init not called */

9
test/test_restconf_ssl_certs.sh
View file

@ -181,6 +181,9 @@ EOF
done # invalid ca
fi # XXX
# Generate random certificate
openssl req -newkey rsa:2048 -nodes -keyout $certdir/random.key -x509 -days 365 -out $certdir/random.crt -subj "/C=XX/ST=TEST/L=TEST/O=TEST/OU=TEST/CN=TEST"
fi # genkeys
# Write local config
@ -286,13 +289,15 @@ EOF
# code
# 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)" "16 35 55 56" # 55 "certificate expired"
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"
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"
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"
# Havent been able to generate "wrong CA"
# new "invalid cert from wrong CA"
# expectpart "$(curl $CURLOPTS --key $certdir/invalid.key --cert $certdir/invalid.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2>&1)" 0 foo # 58 "unable to set private key file" # 58 unable to set private key file