Skip to content

Commit 6aad349

Browse files
brainybrainy
committed
Added error callback infrastructure.
This adds three levels and four kinds of callbacks for reporting errors from Serf: global, context-specific and (incoming or outgoing) connection -specific. Request and response code will use their their connection's callback, but add extra flags to indicate the source of the error message. The SSL code in ssl_buckets.c uses an error context that callers can (or rather "will be able to") define so that error messages get sent to the appropriate, caller-specific callback. This part is not yet implemented because it requires revising some of our SSL APIs. * CMakeLists.txt: Check if <unistd.h> is available, used by tests. (SOURCES): Add the error_callbacks.c file. * SConstruct: Check for <unistd.h>, as above. * serf.h: Add public error callback prototypes and constants. Too many of them to list here individually. * serf_bucket_types.h (serf_ssl_error_cb_set, serf_ssl_error_cb_t): Removed, obsolete. * serf_private.h: Add private helpers for sending error messages to callbacks and the ssl_context infrastructure for handling errors. (serf_context_t): Add error_callback and error_callback_baton. (serf_incoming_t): Likewise. (serf_connection_t): Here, too. * buckets/ssl_buckets.c: Update all calls to the removed ssl-specific error callback to use the new dispatch_ssl_error. (serf_ssl_context_t) Remove error_callback and error_baton. (global_error_ctx): New fallback error context, sends to global callback. (dispatch_ssl_error): New, calls an error context's dispatcher. (log_ssl_error): Use dispatch_ssl_error(). * src/error_callbacks.c: New file. Implements all the private helpers declared in serf_private.h, and also: (serf_global_error_callback_set): Implement here. * src/context.c (serf_context_error_callback_set): Implement here. * src/incoming.c (serf_incoming_error_callback_set): Implement here. * src/outgoing.c (serf_connection_error_callback_set): Implement here. * test/test_util.c (isatty): Import from headers if available, otherwise fake it. (test_error_callback): New, an error callback for the test suite. (setup_test_context): Register test_error_callback in verbose mode. git-svn-id: https://svn.apache.org/repos/asf/serf/trunk@1931047 13f79535-47bb-0310-9956-ffa450edef68
1 parent 257f68b commit 6aad349

File tree

11 files changed

+578
-115
lines changed

11 files changed

+578
-115
lines changed

‎CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ list(APPEND EXPORTS_BLACKLIST
137137
list(APPEND SOURCES
138138
"src/config_store.c"
139139
"src/context.c"
140+
"src/error_callbacks.c"
140141
"src/deprecated.c"
141142
"src/incoming.c"
142143
"src/inet_pton.c"
@@ -393,6 +394,7 @@ CheckFunction("SSL_library_init" "" "SERF_HAVE_OPENSSL_SSL_LIBRARY_INIT"
393394
"openssl/ssl.h" "${OPENSSL_INCLUDE_DIR}"
394395
${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES})
395396
CheckHeader("stdbool.h" "HAVE_STDBOOL_H=1")
397+
CheckHeader("unistd.h" "HAVE_UNISTD_H=1")
396398
CheckType("OSSL_HANDSHAKE_STATE" "openssl/ssl.h" "SERF_HAVE_OSSL_HANDSHAKE_STATE" ${OPENSSL_INCLUDE_DIR})
397399
if(Brotli_FOUND)
398400
CheckType("BrotliDecoderResult" "brotli/decode.h" "SERF_HAVE_BROTLI_DECODER_RESULT" ${BROTLI_INCLUDE_DIR})

‎SConstruct‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,8 @@ if CALLOUT_OKAY:
727727
### some configuration stuffs
728728
if conf.CheckCHeader('stdbool.h'):
729729
env.Append(CPPDEFINES=['HAVE_STDBOOL_H'])
730+
if conf.CheckCHeader('unistd.h'):
731+
env.Append(CPPDEFINES=['HAVE_UNISTD_H'])
730732

731733
env = conf.Finish()
732734

‎buckets/ssl_buckets.c‎

Lines changed: 64 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,6 @@ struct serf_ssl_context_t {
206206
X509 *cached_cert;
207207
EVP_PKEY *cached_cert_pw;
208208

209-
/* Error callback */
210-
serf_ssl_error_cb_t error_callback;
211-
void *error_baton;
212-
213209
apr_status_t pending_err;
214210

215211
/* Status of a fatal error, returned on subsequent encrypt or decrypt
@@ -229,6 +225,31 @@ struct serf_ssl_context_t {
229225
serf_config_t *config;
230226
};
231227

228+
/* The fallback SSL error context which logs to the global error callback. */
229+
static const serf__ssl_error_ctx_t global_error_ctx = {
230+
serf__global_ssl_error, /* dispatcher */
231+
NULL /* baton */
232+
};
233+
234+
static apr_status_t dispatch_ssl_error(const serf__ssl_error_ctx_t *err_ctx,
235+
apr_status_t status,
236+
const char *message)
237+
{
238+
return err_ctx->dispatch(err_ctx->baton, status, message);
239+
}
240+
241+
static void log_ssl_error(const serf__ssl_error_ctx_t *err_ctx,
242+
apr_status_t status)
243+
{
244+
unsigned long err;
245+
246+
while ((err = ERR_get_error())) {
247+
char ebuf[256];
248+
ERR_error_string_n(err, ebuf, sizeof(ebuf));
249+
dispatch_ssl_error(err_ctx, status, ebuf);
250+
}
251+
}
252+
232253
typedef struct ssl_context_t {
233254
/* The bucket-independent ssl context that this bucket is associated with */
234255
serf_ssl_context_t *ssl_ctx;
@@ -355,21 +376,6 @@ detect_renegotiate(const SSL *s, int where, int ret)
355376
}
356377
}
357378

358-
static void log_ssl_error(serf_ssl_context_t *ctx)
359-
{
360-
unsigned long err;
361-
362-
while ((err = ERR_get_error())) {
363-
364-
if (err && ctx->error_callback) {
365-
char ebuf[256];
366-
ERR_error_string_n(err, ebuf, sizeof(ebuf));
367-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
368-
}
369-
370-
}
371-
}
372-
373379
static void bio_set_data(BIO *bio, void *data)
374380
{
375381
#ifndef SERF_NO_SSL_BIO_WRAPPERS
@@ -1096,7 +1102,7 @@ static apr_status_t status_from_ssl_error(serf_ssl_context_t *ctx,
10961102
ctx->fatal_err = SERF_ERROR_SSL_COMM_FAILED;
10971103

10981104
status = ctx->fatal_err;
1099-
log_ssl_error(ctx);
1105+
log_ssl_error(&global_error_ctx, status);
11001106
}
11011107
break;
11021108

@@ -1110,7 +1116,7 @@ static apr_status_t status_from_ssl_error(serf_ssl_context_t *ctx,
11101116

11111117
default:
11121118
status = ctx->fatal_err = SERF_ERROR_SSL_COMM_FAILED;
1113-
log_ssl_error(ctx);
1119+
log_ssl_error(&global_error_ctx, status);
11141120
break;
11151121
}
11161122

@@ -1217,7 +1223,7 @@ static apr_status_t ssl_decrypt(void *baton, apr_size_t bufsize,
12171223
} else {
12181224
/* A fatal error occurred. */
12191225
ctx->fatal_err = status = SERF_ERROR_SSL_COMM_FAILED;
1220-
log_ssl_error(ctx);
1226+
log_ssl_error(&global_error_ctx, status);
12211227
}
12221228
} else {
12231229
*len = ssl_len;
@@ -1675,14 +1681,12 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
16751681

16761682
store = OSSL_STORE_open(cert_uri, ui_method, ctx, NULL, NULL);
16771683
if (!store) {
1684+
char ebuf[1024];
1685+
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1686+
apr_snprintf(ebuf, sizeof(ebuf), "could not open URI: %s", cert_uri);
1687+
dispatch_ssl_error(&global_error_ctx, ctx->fatal_err, ebuf);
16781688

1679-
if (ctx->error_callback) {
1680-
char ebuf[1024];
1681-
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1682-
apr_snprintf(ebuf, sizeof(ebuf), "could not open URI: %s", cert_uri);
1683-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1684-
}
1685-
1689+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
16861690
break;
16871691
}
16881692

@@ -1697,14 +1701,12 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
16971701
}
16981702

16991703
if (!info) {
1704+
char ebuf[1024];
1705+
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1706+
apr_snprintf(ebuf, sizeof(ebuf), "could not read URI: %s", cert_uri);
1707+
dispatch_ssl_error(&global_error_ctx, ctx->fatal_err, ebuf);
17001708

1701-
if (ctx->error_callback) {
1702-
char ebuf[1024];
1703-
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1704-
apr_snprintf(ebuf, sizeof(ebuf), "could not read URI: %s", cert_uri);
1705-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1706-
}
1707-
1709+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
17081710
break;
17091711
}
17101712

@@ -1824,8 +1826,7 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
18241826
UI_destroy_method(ui_method);
18251827

18261828
if (ERR_peek_error()) {
1827-
log_ssl_error(ctx);
1828-
1829+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
18291830
return -1;
18301831
}
18311832

@@ -1888,13 +1889,13 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
18881889
ctx->pool);
18891890

18901891
if (status) {
1891-
if (ctx->error_callback) {
1892-
char ebuf[1024];
1893-
apr_snprintf(ebuf, sizeof(ebuf), "could not open PKCS12: %s", cert_path);
1894-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1895-
apr_strerror(status, ebuf, sizeof(ebuf));
1896-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1897-
}
1892+
char ebuf[1024];
1893+
apr_snprintf(ebuf, sizeof(ebuf), "could not open PKCS12: %s", cert_path);
1894+
dispatch_ssl_error(&global_error_ctx, status, ebuf);
1895+
apr_strerror(status, ebuf, sizeof(ebuf));
1896+
dispatch_ssl_error(&global_error_ctx, status, ebuf);
1897+
1898+
ctx->fatal_err = status;
18981899
return -1;
18991900
}
19001901

@@ -1926,10 +1927,12 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
19261927
return 1;
19271928
}
19281929
else {
1930+
char ebuf[1024];
19291931
unsigned long err = ERR_get_error();
19301932
ERR_clear_error();
19311933
if (ERR_GET_LIB(err) == ERR_LIB_PKCS12 &&
1932-
ERR_GET_REASON(err) == PKCS12_R_MAC_VERIFY_FAILURE) {
1934+
ERR_GET_REASON(err) == PKCS12_R_MAC_VERIFY_FAILURE)
1935+
{
19331936
if (ctx->cert_pw_callback) {
19341937
const char *password;
19351938

@@ -1973,44 +1976,33 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
19731976
return 1;
19741977
}
19751978
else {
1979+
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1980+
apr_snprintf(ebuf, sizeof(ebuf), "could not parse PKCS12: %s", cert_path);
1981+
dispatch_ssl_error(&global_error_ctx, ctx->fatal_err, ebuf);
19761982

1977-
if (ctx->error_callback) {
1978-
char ebuf[1024];
1979-
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1980-
apr_snprintf(ebuf, sizeof(ebuf), "could not parse PKCS12: %s", cert_path);
1981-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1982-
}
1983-
1984-
log_ssl_error(ctx);
1983+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
19851984
return -1;
19861985
}
19871986
}
19881987
}
19891988
PKCS12_free(p12);
19901989
bio_meth_free(biom);
19911990

1992-
if (ctx->error_callback) {
1993-
char ebuf[1024];
1994-
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1995-
apr_snprintf(ebuf, sizeof(ebuf), "PKCS12 needs a password: %s", cert_path);
1996-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
1997-
}
1991+
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
1992+
apr_snprintf(ebuf, sizeof(ebuf), "PKCS12 needs a password: %s", cert_path);
1993+
dispatch_ssl_error(&global_error_ctx, ctx->fatal_err, ebuf);
19981994

1999-
log_ssl_error(ctx);
1995+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
20001996
return -1;
20011997
}
20021998
else {
20031999
PKCS12_free(p12);
20042000
bio_meth_free(biom);
20052001

2006-
if (ctx->error_callback) {
2007-
char ebuf[1024];
2008-
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
2009-
apr_snprintf(ebuf, sizeof(ebuf), "could not parse PKCS12: %s", cert_path);
2010-
ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
2011-
}
2002+
ctx->fatal_err = SERF_ERROR_SSL_CERT_FAILED;
2003+
dispatch_ssl_error(&global_error_ctx, ctx->fatal_err, ebuf);
20122004

2013-
log_ssl_error(ctx);
2005+
log_ssl_error(&global_error_ctx, ctx->fatal_err);
20142006
return -1;
20152007
}
20162008
}
@@ -2088,15 +2080,6 @@ void serf_ssl_server_cert_chain_callback_set(
20882080
context->server_cert_userdata = data;
20892081
}
20902082

2091-
void serf_ssl_error_cb_set(
2092-
serf_ssl_context_t *context,
2093-
serf_ssl_error_cb_t callback,
2094-
void *baton)
2095-
{
2096-
context->error_callback = callback;
2097-
context->error_baton = baton;
2098-
}
2099-
21002083
static int ssl_new_session(SSL *ssl, SSL_SESSION *session)
21012084
{
21022085
serf_ssl_context_t *ctx = SSL_get_app_data(ssl);
@@ -2162,9 +2145,6 @@ static serf_ssl_context_t *ssl_init_context(serf_bucket_alloc_t *allocator)
21622145
ssl_ctx->protocol_callback = NULL;
21632146
ssl_ctx->protocol_userdata = NULL;
21642147

2165-
ssl_ctx->error_callback = NULL;
2166-
ssl_ctx->error_baton = NULL;
2167-
21682148
SSL_CTX_set_verify(ssl_ctx->ctx, SSL_VERIFY_PEER,
21692149
validate_server_certificate);
21702150
SSL_CTX_set_options(ssl_ctx->ctx, SSL_OP_ALL);
@@ -2410,14 +2390,10 @@ apr_status_t serf_ssl_load_cert_file(
24102390

24112391
return APR_SUCCESS;
24122392
}
2413-
#if 0
2414-
else {
2415-
/* If we'd have had a serf context *, we could have used serf logging */
2416-
ERR_print_errors_fp(stderr);
2417-
}
2418-
#endif
24192393

2420-
return SERF_ERROR_SSL_CERT_FAILED;
2394+
status = SERF_ERROR_SSL_CERT_FAILED;
2395+
log_ssl_error(&global_error_ctx, status);
2396+
return status;
24212397
}
24222398

24232399

@@ -2479,7 +2455,7 @@ apr_status_t serf_ssl_add_crl_from_file(serf_ssl_context_t *ssl_ctx,
24792455
result = X509_STORE_add_crl(store, crl);
24802456
if (!result) {
24812457
ssl_ctx->fatal_err = status = SERF_ERROR_SSL_CERT_FAILED;
2482-
log_ssl_error(ssl_ctx);
2458+
log_ssl_error(&global_error_ctx, status);
24832459
return status;
24842460
}
24852461

0 commit comments

Comments
 (0)