From 512a61b2fc1a758c0401562d9753df261b0ef083 Mon Sep 17 00:00:00 2001 From: Graham Leggett Date: Tue, 1 Jul 2025 13:35:29 +0100 Subject: [PATCH 1/2] Report SSL error messages from serf Add the most detailed underlying crypto library error string to the error stack when the context fails due to an SSL failure. SSL errors are no longer reduced to "an error has occurred". This relies on the serf_ssl_error_cb_t callback as provided by serf in https://github.com/apache/serf/pull/9. Example: [minfrin@rocky9 subversion]$ svn info https://svn.example.com/svn/example/core/ svn: E170013: Unable to connect to a repository at URL 'https://svn.example.com/svn/example/core' svn: E120171: TLS: error:0308010C:digital envelope routines::unsupported svn: E120171: Error running context: An error occurred during SSL communication --- build/ac-macros/serf.m4 | 7 ++++++ subversion/libsvn_ra_serf/ra_serf.h | 3 +++ subversion/libsvn_ra_serf/util.c | 35 +++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/build/ac-macros/serf.m4 b/build/ac-macros/serf.m4 index b3650f61861e3..7332d0e88ca0e 100644 --- a/build/ac-macros/serf.m4 +++ b/build/ac-macros/serf.m4 @@ -89,6 +89,13 @@ AC_DEFUN(SVN_LIB_SERF, svn_lib_serf=$serf_found + if test "$svn_lib_serf" = "yes"; then + save_ldflags="$LDFLAGS" + LDFLAGS="$LDFLAGS $SVN_SERF_LIBS" + AC_CHECK_FUNCS(serf_ssl_error_cb_set) + LDFLAGS="$save_ldflags" + fi + SVN_DOT_CLANGD([$SVN_SERF_INCLUDES]) AC_SUBST(SVN_SERF_INCLUDES) AC_SUBST(SVN_SERF_LIBS) diff --git a/subversion/libsvn_ra_serf/ra_serf.h b/subversion/libsvn_ra_serf/ra_serf.h index eace1af5319ac..cc87dd293c8c3 100644 --- a/subversion/libsvn_ra_serf/ra_serf.h +++ b/subversion/libsvn_ra_serf/ra_serf.h @@ -113,6 +113,9 @@ struct svn_ra_serf__session_t { /* Are we using ssl */ svn_boolean_t using_ssl; + /* What was the underlying detail of the last SSL failure, if any */ + const char *ssl_error; + /* Tristate flag that indicates if we should use compression for network transmissions. If svn_tristate_true or svn_tristate_false, the compression should be enabled and disabled, respectively. diff --git a/subversion/libsvn_ra_serf/util.c b/subversion/libsvn_ra_serf/util.c index e010f82d62110..e4a505a5d8717 100644 --- a/subversion/libsvn_ra_serf/util.c +++ b/subversion/libsvn_ra_serf/util.c @@ -451,6 +451,20 @@ ssl_server_cert_cb(void *baton, int failures, return save_error(session, err); } +#if defined(HAVE_SERF_SSL_ERROR_CB_SET) +static apr_status_t +ssl_error_cb(void *baton, + const char *message) +{ + svn_ra_serf__connection_t *conn = baton; + svn_ra_serf__session_t *session = conn->session; + + session->ssl_error = apr_pstrdup(session->pool, message); + + return APR_SUCCESS; +} +#endif + static svn_error_t * load_authorities(svn_ra_serf__connection_t *conn, const char *authorities, apr_pool_t *pool) @@ -567,7 +581,14 @@ conn_setup(apr_socket_t *sock, SERF_CONNECTION_FRAMING_TYPE_NONE); } #endif - } + +#if defined(HAVE_SERF_SSL_ERROR_CB_SET) + serf_ssl_error_cb_set(conn->ssl_context, + ssl_error_cb, + conn); +#endif + + } if (write_bkt) { @@ -958,7 +979,17 @@ svn_ra_serf__context_run(svn_ra_serf__session_t *sess, _("Error running context")); } - return svn_ra_serf__wrap_err(status, _("Error running context")); + if (sess->ssl_error) + { + return svn_error_createf(status, + svn_ra_serf__wrap_err(status, _("Error running context")), + _("TLS: %s"), + sess->ssl_error); + } + else + { + return svn_ra_serf__wrap_err(status, _("Error running context")); + } } return SVN_NO_ERROR; From 675e2948292203d7dc15b3da68be3cb7ec917404 Mon Sep 17 00:00:00 2001 From: Graham Leggett Date: Tue, 1 Jul 2025 17:32:25 +0100 Subject: [PATCH 2/2] Build a svn_error_t from SSL error messages. --- subversion/libsvn_ra_serf/ra_serf.h | 2 +- subversion/libsvn_ra_serf/util.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/subversion/libsvn_ra_serf/ra_serf.h b/subversion/libsvn_ra_serf/ra_serf.h index cc87dd293c8c3..28c37f306f88f 100644 --- a/subversion/libsvn_ra_serf/ra_serf.h +++ b/subversion/libsvn_ra_serf/ra_serf.h @@ -114,7 +114,7 @@ struct svn_ra_serf__session_t { svn_boolean_t using_ssl; /* What was the underlying detail of the last SSL failure, if any */ - const char *ssl_error; + svn_error_t *ssl_error; /* Tristate flag that indicates if we should use compression for network transmissions. If svn_tristate_true or svn_tristate_false, diff --git a/subversion/libsvn_ra_serf/util.c b/subversion/libsvn_ra_serf/util.c index e4a505a5d8717..9fd85defe19f5 100644 --- a/subversion/libsvn_ra_serf/util.c +++ b/subversion/libsvn_ra_serf/util.c @@ -453,13 +453,16 @@ ssl_server_cert_cb(void *baton, int failures, #if defined(HAVE_SERF_SSL_ERROR_CB_SET) static apr_status_t -ssl_error_cb(void *baton, +ssl_error_cb(void *baton, apr_status_t status, const char *message) { svn_ra_serf__connection_t *conn = baton; svn_ra_serf__session_t *session = conn->session; - session->ssl_error = apr_pstrdup(session->pool, message); + session->ssl_error = svn_error_createf(status, + session->ssl_error, + _("TLS: %s"), + message); return APR_SUCCESS; } @@ -981,10 +984,7 @@ svn_ra_serf__context_run(svn_ra_serf__session_t *sess, if (sess->ssl_error) { - return svn_error_createf(status, - svn_ra_serf__wrap_err(status, _("Error running context")), - _("TLS: %s"), - sess->ssl_error); + return sess->ssl_error; } else {