diff --git a/CHANGELOG.md b/CHANGELOG.md index f3d96261..4431b64d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,7 +91,14 @@ Developers may need to change their code * `cli_xml2txt(x, fn, l)` -> `clixon_txt2file(stdout, x, l, NULL, 0)` * `xml2txt(f, x, l)` -> `clixon_txt2file(f, x, l, NULL, 0)` * `xml2txt_cb(f, x, fn)` -> `clixon_txt2file(f, x, 0, NULL, 0)` - + +### Minor features + +* [Feature Request: Log SSL events](https://github.com/clicon/clixon/issues/331) + * Added syslog NOTICE on failed user certs + +### Corrected Bugs + ## 5.7.0 17 May 2022 diff --git a/apps/restconf/restconf_main_native.c b/apps/restconf/restconf_main_native.c index 4e08a08a..db55112b 100644 --- a/apps/restconf/restconf_main_native.c +++ b/apps/restconf/restconf_main_native.c @@ -849,13 +849,22 @@ restconf_accept_client(int fd, /* Get the actual peer, XXX this maybe could be done in ca-auth client-cert code ? * 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 */ + if ((ret = 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); } } +#if 0 + else{ + clicon_log(LOG_NOTICE, "Cert error: %s", X509_verify_cert_error_string(ret)); + /* Maybe should return already here, but to get proper return message need to + * continue to http/1 or http/2 handling + * @see restconf_connection_sanity + */ + } +#endif #if 0 /* debug */ if (clicon_debug_get()) restconf_listcerts(rc->rc_ssl); diff --git a/apps/restconf/restconf_native.c b/apps/restconf/restconf_native.c index 443e72b6..591109bb 100644 --- a/apps/restconf/restconf_native.c +++ b/apps/restconf/restconf_native.c @@ -261,6 +261,7 @@ ssl_x509_name_oneline(SSL *ssl, * @param[out] term Terminate session * @retval -1 Error * @retval 0 OK + * @see restconf_accept_client where connection can be exited at an earlier stage */ int restconf_connection_sanity(clicon_handle h, @@ -273,6 +274,7 @@ restconf_connection_sanity(clicon_handle h, cbuf *cberr = NULL; restconf_media media_out = YANG_DATA_JSON; char *media_str = NULL; + char *oneline = NULL; /* 1) Check if http/2 non-tls is disabled */ if (rc->rc_ssl == NULL && @@ -294,11 +296,20 @@ restconf_connection_sanity(clicon_handle h, /* 2) Check if ssl client cert is valid */ else if (rc->rc_ssl != NULL && (code = SSL_get_verify_result(rc->rc_ssl)) != 0){ + /* Syslog cert failure */ + if (ssl_x509_name_oneline(rc->rc_ssl, &oneline) < 0) + goto done; + if (oneline) + clicon_log(LOG_NOTICE, "Cert error: %s: %s", oneline, X509_verify_cert_error_string(code)); + else + clicon_log(LOG_NOTICE, "Cert error: %s", X509_verify_cert_error_string(code)); + /* Send return error message */ 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); + cprintf(cberr, "HTTP cert verification failed: %s", + X509_verify_cert_error_string(code)); if (netconf_invalid_value_xml(&xerr, "protocol", cbuf_get(cberr)) < 0) goto done; if ((media_str = restconf_param_get(h, "HTTP_ACCEPT")) == NULL){ @@ -314,6 +325,8 @@ restconf_connection_sanity(clicon_handle h, } retval = 0; done: + if (oneline) + free(oneline); if (cberr) cbuf_free(cberr); if (xerr) @@ -519,14 +532,13 @@ read_ssl(restconf_conn *rc, usleep(1000); *again = 1; break; + case SSL_ERROR_ZERO_RETURN: + *np = 0; /* should already be zero */ + break; default: -#if 1 /* Make socket read error handling more resilient: log and continue instead of exit */ - clicon_log(LOG_WARNING, "%s SSL_read(): %s", __FUNCTION__, strerror(errno)); + clicon_log(LOG_WARNING, "%s SSL_read(): %s sslerr:%d", __FUNCTION__, strerror(errno), sslerr); *np = 0; -#else - clicon_err(OE_XML, errno, "SSL_read %d", sslerr); - goto done; -#endif + break; } /* switch */ } retval = 0; diff --git a/test/test_restconf_ssl_certs.sh b/test/test_restconf_ssl_certs.sh index 0131c033..edb1afd2 100755 --- a/test/test_restconf_ssl_certs.sh +++ b/test/test_restconf_ssl_certs.sh @@ -288,10 +288,10 @@ 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 400" "\"error-message\": \"HTTP cert verification failed, unknown ca" + 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: 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, unknown ca" + 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 "too weak cert (sign w md5)" # Either curl error or error return ret=$(curl $CURLOPTS --key $certdir/mymd5.key --cert $certdir/mymd5.crt -X GET $RCPROTO://localhost/restconf/data/example:x 2> /dev/null)