From 30a626371573c553bd3c9886b1f9f8e92a0410fd Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Mon, 3 Jan 2022 20:54:09 +0000 Subject: [PATCH] Avoid modifying global errno when raising event This seems like a safer interface, as new callsites may be added at any time. --- src/src/deliver.c | 24 ++++++++++++++++++------ src/src/functions.h | 2 +- src/src/queue.c | 6 +++--- src/src/smtp_out.c | 2 +- src/src/tls-gnu.c | 6 +++--- src/src/tls-openssl.c | 10 +++++----- src/src/transports/smtp.c | 24 ++++++++++++++---------- src/src/verify.c | 4 ++-- 8 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/src/deliver.c b/src/src/deliver.c index 1bc0a62e7..2ced28e9a 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -854,8 +854,18 @@ return g; #ifndef DISABLE_EVENT +/* Distribute a named event to any listeners. + +Args: action config option specifying listener + event name of the event + ev_data associated data for the event + errnop pointer to errno for modification, or null + +Return: string expansion from listener, or NULL +*/ + uschar * -event_raise(uschar * action, const uschar * event, uschar * ev_data) +event_raise(uschar * action, const uschar * event, uschar * ev_data, int * errnop) { uschar * s; if (action) @@ -882,7 +892,8 @@ if (action) { DEBUG(D_deliver) debug_printf("Event(%s): event_action returned \"%s\"\n", event, s); - errno = ERRNO_EVENT; + if (errnop) + *errnop = ERRNO_EVENT; return s; } } @@ -911,7 +922,7 @@ if (!addr->transport) a filter was used which triggered a fail command (in such a case a transport isn't needed). Convert it to an internal fail event. */ - (void) event_raise(event_action, US"msg:fail:internal", addr->message); + (void) event_raise(event_action, US"msg:fail:internal", addr->message, NULL); } } else @@ -923,7 +934,8 @@ else || Ustrcmp(addr->transport->driver_name, "smtp") == 0 || Ustrcmp(addr->transport->driver_name, "lmtp") == 0 || Ustrcmp(addr->transport->driver_name, "autoreply") == 0 - ? addr->message : NULL); + ? addr->message : NULL, + NULL); } deliver_host_port = save_port; @@ -6341,7 +6353,7 @@ if (process_recipients != RECIP_IGNORE) string_copyn(addr+start, dom ? (dom-1) - start : end - start); deliver_domain = dom ? CUS string_copyn(addr+dom, end - dom) : CUS""; - event_raise(event_action, US"msg:fail:internal", new->message); + (void) event_raise(event_action, US"msg:fail:internal", new->message, NULL); deliver_localpart = save_local; deliver_domain = save_domain; @@ -8092,7 +8104,7 @@ if (!addr_defer) f.deliver_freeze = FALSE; #ifndef DISABLE_EVENT - (void) event_raise(event_action, US"msg:complete", NULL); + (void) event_raise(event_action, US"msg:complete", NULL, NULL); #endif } diff --git a/src/src/functions.h b/src/src/functions.h index 851973ee5..34a6fb786 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -236,7 +236,7 @@ extern BOOL dscp_lookup(const uschar *, int, int *, int *, int *); extern void enq_end(uschar *); extern BOOL enq_start(uschar *, unsigned); #ifndef DISABLE_EVENT -extern uschar *event_raise(uschar *, const uschar *, uschar *); +extern uschar *event_raise(uschar *, const uschar *, uschar *, int *); extern void msg_event_raise(const uschar *, const address_item *); #endif diff --git a/src/src/queue.c b/src/src/queue.c index 93d69f89f..dff6168c0 100644 --- a/src/src/queue.c +++ b/src/src/queue.c @@ -1348,15 +1348,15 @@ switch(action) deliver_domain = dom ? CUS string_copyn(addr+dom, end - dom) : CUS""; - event_raise(event_action, US"msg:fail:internal", - string_sprintf("message removed by %s", username)); + (void) event_raise(event_action, US"msg:fail:internal", + string_sprintf("message removed by %s", username), NULL); deliver_localpart = save_local; deliver_domain = save_domain; } } } - (void) event_raise(event_action, US"msg:complete", NULL); + (void) event_raise(event_action, US"msg:complete", NULL, NULL); #endif log_write(0, LOG_MAIN, "removed by %s", username); log_write(0, LOG_MAIN, "Completed"); diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c index 9547c4b81..608a781eb 100644 --- a/src/src/smtp_out.c +++ b/src/src/smtp_out.c @@ -283,7 +283,7 @@ const blob * fastopen_blob = NULL; #ifndef DISABLE_EVENT deliver_host_address = host->address; deliver_host_port = port; -if (event_raise(tb->event_action, US"tcp:connect", NULL)) return -1; +if (event_raise(tb->event_action, US"tcp:connect", NULL, &errno)) return -1; #endif if ((sock = ip_socket(SOCK_STREAM, host_af)) < 0) return -1; diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index 4f1039903..53635acae 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -2721,7 +2721,7 @@ if ((cert_list = gnutls_certificate_get_peers(session, &cert_list_size))) state->tlsp->peercert = crt; if ((yield = event_raise(state->event_action, - US"tls:cert", string_sprintf("%d", cert_list_size)))) + US"tls:cert", string_sprintf("%d", cert_list_size), &errno))) { log_write(0, LOG_MAIN, "SSL verify denied by event-action: depth=%d: %s", @@ -3053,13 +3053,13 @@ if (rc != GNUTLS_E_SUCCESS) if (sigalrm_seen) { tls_error(US"gnutls_handshake", US"timed out", NULL, errstr); - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); gnutls_db_remove_session(state->session); } else { tls_error_gnu(state, US"gnutls_handshake", rc, errstr); - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); (void) gnutls_alert_send_appropriate(state->session, rc); gnutls_deinit(state->session); gnutls_certificate_free_credentials(state->lib_state.x509_cred); diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index 0c7772921..5130455fe 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -1009,7 +1009,7 @@ if (ev) old_cert = tlsp->peercert; tlsp->peercert = X509_dup(cert); /* NB we do not bother setting peerdn */ - if ((yield = event_raise(ev, US"tls:cert", string_sprintf("%d", depth)))) + if ((yield = event_raise(ev, US"tls:cert", string_sprintf("%d", depth), &errno))) { log_write(0, LOG_MAIN, "[%s] %s verify denied by event-action: " "depth=%d cert=%s: %s", @@ -3311,7 +3311,7 @@ if (rc <= 0) case SSL_ERROR_ZERO_RETURN: DEBUG(D_tls) debug_printf("Got SSL_ERROR_ZERO_RETURN\n"); (void) tls_error(US"SSL_accept", NULL, sigalrm_seen ? US"timed out" : NULL, errstr); - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); if (SSL_get_shutdown(ssl) == SSL_RECEIVED_SHUTDOWN) SSL_shutdown(ssl); @@ -3331,7 +3331,7 @@ if (rc <= 0) || r == SSL_R_UNKNOWN_PROTOCOL || r == SSL_R_UNSUPPORTED_PROTOCOL) s = string_sprintf("(%s)", SSL_get_version(ssl)); (void) tls_error(US"SSL_accept", NULL, sigalrm_seen ? US"timed out" : s, errstr); - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); return FAIL; } @@ -3342,7 +3342,7 @@ if (rc <= 0) if (!errno) { *errstr = US"SSL_accept: TCP connection closed by peer"; - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); return FAIL; } DEBUG(D_tls) debug_printf(" - syscall %s\n", strerror(errno)); @@ -3351,7 +3351,7 @@ if (rc <= 0) sigalrm_seen ? US"timed out" : ERR_peek_error() ? NULL : string_sprintf("ret %d", error), errstr); - (void) event_raise(event_action, US"tls:fail:connect", *errstr); + (void) event_raise(event_action, US"tls:fail:connect", *errstr, NULL); return FAIL; } } diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 721056f27..2f109a97f 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -678,7 +678,8 @@ deliver_localpart = addr->local_part; : string_copy(addr->message) : addr->basic_errno > 0 ? string_copy(US strerror(addr->basic_errno)) - : NULL); + : NULL, + NULL); deliver_localpart = save_local; deliver_domain = save_domain; @@ -751,7 +752,7 @@ sx->helo_response = string_copy(sx->buffer); #endif #ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, - US"smtp:ehlo", sx->buffer); + US"smtp:ehlo", sx->buffer, NULL); #endif return TRUE; } @@ -2131,7 +2132,8 @@ if (continue_hostname && continue_proxy_cipher) # ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, US"dane:fail", sx->dane_required - ? US"dane-required" : US"dnssec-invalid"); + ? US"dane-required" : US"dnssec-invalid", + NULL); # endif return rc; } @@ -2218,7 +2220,8 @@ if (!continue_hostname) # ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, US"dane:fail", sx->dane_required - ? US"dane-required" : US"dnssec-invalid"); + ? US"dane-required" : US"dnssec-invalid", + NULL); # endif return rc; } @@ -2230,7 +2233,7 @@ if (!continue_hostname) FAIL, FALSE, &sx->delivery_start); # ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, - US"dane:fail", US"dane-required"); + US"dane:fail", US"dane-required", NULL); # endif return FAIL; } @@ -2349,7 +2352,7 @@ will be? Somehow I doubt it. */ uschar * s; lookup_dnssec_authenticated = sx->conn_args.host->dnssec==DS_YES ? US"yes" : sx->conn_args.host->dnssec==DS_NO ? US"no" : NULL; - s = event_raise(sx->conn_args.tblock->event_action, US"smtp:connect", sx->buffer); + s = event_raise(sx->conn_args.tblock->event_action, US"smtp:connect", sx->buffer, NULL); if (s) { set_errno_nohost(sx->addrlist, ERRNO_EXPANDFAIL, @@ -2686,7 +2689,7 @@ if ( smtp_peer_options & OPTION_TLS sx->conn_args.host->name, sx->conn_args.host->address, tls_errstr); # ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, - US"dane:fail", US"validation-failure"); /* could do with better detail */ + US"dane:fail", US"validation-failure", NULL); /* could do with better detail */ # endif } # endif @@ -2854,7 +2857,8 @@ else if ( sx->smtps (void) event_raise(sx->conn_args.tblock->event_action, US"dane:fail", smtp_peer_options & OPTION_TLS ? US"validation-failure" /* could do with better detail */ - : US"starttls-not-supported"); + : US"starttls-not-supported", + NULL); # endif goto TLS_FAILED; } @@ -3172,7 +3176,7 @@ if (sx->send_quit) sx->cctx.sock = -1; #ifndef DISABLE_EVENT -(void) event_raise(sx->conn_args.tblock->event_action, US"tcp:close", NULL); +(void) event_raise(sx->conn_args.tblock->event_action, US"tcp:close", NULL, NULL); #endif continue_transport = NULL; @@ -4832,7 +4836,7 @@ continue_transport = NULL; continue_hostname = NULL; #ifndef DISABLE_EVENT -(void) event_raise(tblock->event_action, US"tcp:close", NULL); +(void) event_raise(tblock->event_action, US"tcp:close", NULL, NULL); #endif #ifdef SUPPORT_DANE diff --git a/src/src/verify.c b/src/src/verify.c index c182e12e1..6478a9e7e 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -815,7 +815,7 @@ tls_retry_connection: sx->cctx.sock = -1; #ifndef DISABLE_EVENT (void) event_raise(addr->transport->event_action, - US"tcp:close", NULL); + US"tcp:close", NULL, NULL); #endif addr->address = main_address; addr->transport_return = PENDING_DEFER; @@ -1127,7 +1127,7 @@ no_conn: (void)close(sx->cctx.sock); sx->cctx.sock = -1; #ifndef DISABLE_EVENT - (void) event_raise(addr->transport->event_action, US"tcp:close", NULL); + (void) event_raise(addr->transport->event_action, US"tcp:close", NULL, NULL); #endif } } -- 2.30.2