Skip to content

Commit

Permalink
QUIC err handling: Save and restore error state
Browse files Browse the repository at this point in the history
We save the error state from the thread that encountered
a permanent error condition caused by system or internal
error to the QUIC_CHANNEL.

Then we restore it whenever we are returning to a user
call when protocol is shutdown.
  • Loading branch information
t8m committed May 30, 2023
1 parent fc03fba commit a13d62f
Show file tree
Hide file tree
Showing 11 changed files with 155 additions and 17 deletions.
2 changes: 1 addition & 1 deletion crypto/err/build.info
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
LIBS=../../libcrypto
SOURCE[../../libcrypto]=\
err_blocks.c err_mark.c err.c err_all.c err_all_legacy.c err_prn.c
err_blocks.c err_mark.c err.c err_all.c err_all_legacy.c err_prn.c err_save.c
10 changes: 4 additions & 6 deletions crypto/err/err.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ ERR_STATE *ERR_get_state(void);
static int err_load_strings(const ERR_STRING_DATA *str);
#endif

static void ERR_STATE_free(ERR_STATE *s);
#ifndef OPENSSL_NO_ERR
static ERR_STRING_DATA ERR_str_libraries[] = {
{ERR_PACK(ERR_LIB_NONE, 0, 0), "unknown library"},
Expand Down Expand Up @@ -199,7 +198,7 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)
}
#endif

static void ERR_STATE_free(ERR_STATE *state)
void OSSL_ERR_STATE_free(ERR_STATE *state)
{
int i;

Expand Down Expand Up @@ -649,7 +648,7 @@ static void err_delete_thread_state(void *unused)
return;

CRYPTO_THREAD_set_local(&err_thread_local, NULL);
ERR_STATE_free(state);
OSSL_ERR_STATE_free(state);
}

#ifndef OPENSSL_NO_DEPRECATED_1_1_0
Expand Down Expand Up @@ -689,16 +688,15 @@ ERR_STATE *ossl_err_get_state_int(void)
if (!CRYPTO_THREAD_set_local(&err_thread_local, (ERR_STATE*)-1))
return NULL;

/* calling CRYPTO_zalloc(.., NULL, 0) prevents mem alloc error loop */
state = CRYPTO_zalloc(sizeof(*state), NULL, 0);
state = OSSL_ERR_STATE_new();
if (state == NULL) {
CRYPTO_THREAD_set_local(&err_thread_local, NULL);
return NULL;
}

if (!ossl_init_thread_start(NULL, NULL, err_delete_thread_state)
|| !CRYPTO_THREAD_set_local(&err_thread_local, state)) {
ERR_STATE_free(state);
OSSL_ERR_STATE_free(state);
CRYPTO_THREAD_set_local(&err_thread_local, NULL);
return NULL;
}
Expand Down
5 changes: 3 additions & 2 deletions crypto/err/err_local.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ static ossl_inline void err_set_debug(ERR_STATE *es, size_t i,
OPENSSL_free(es->err_func[i]);
if (fn == NULL || fn[0] == '\0')
es->err_func[i] = NULL;
else
es->err_func[i] = OPENSSL_strdup(fn);
else if ((es->err_func[i] = CRYPTO_malloc(strlen(fn) + 1,
NULL, 0)) != NULL)
strcpy(es->err_func[i], fn);
}

static ossl_inline void err_set_data(ERR_STATE *es, size_t i,
Expand Down
88 changes: 88 additions & 0 deletions crypto/err/err_save.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright 2023 The OpenSSL Project Authors. All Rights Reserved.
*
* Licensed under the Apache License 2.0 (the "License"). You may not use
* this file except in compliance with the License. You can obtain a copy
* in the file LICENSE in the source distribution or at
* https://www.openssl.org/source/license.html
*/

#define OSSL_FORCE_ERR_STATE

#include <openssl/err.h>
#include "err_local.h"

/*
* Save and restore error state.
* We are using CRYPTO_zalloc(.., NULL, 0) instead of OPENSSL_malloc() in
* these functions to prevent mem alloc error loop.
*/

ERR_STATE *OSSL_ERR_STATE_new(void)
{
return CRYPTO_zalloc(sizeof(ERR_STATE), NULL, 0);
}

void OSSL_ERR_STATE_save(ERR_STATE *es)
{
size_t i;
ERR_STATE *thread_es;

if (es == NULL)
return;

for (i = 0; i < ERR_NUM_ERRORS; i++) {
err_clear(es, i, 1);
}

thread_es = ossl_err_get_state_int();
if (thread_es == NULL)
return;

memcpy(es, thread_es, sizeof(*es));
/* Taking over the pointers, just clear the thread state. */
memset(thread_es, 0, sizeof(*thread_es));
}

void OSSL_ERR_STATE_restore(const ERR_STATE *es)
{
size_t i;
ERR_STATE *thread_es;

if (es == NULL || es->bottom == es->top)
return;

thread_es = ossl_err_get_state_int();
if (thread_es == NULL)
return;

for (i = ((size_t)es->bottom + 1) % ERR_NUM_ERRORS; i != (size_t)es->top;
i = (i + 1) % ERR_NUM_ERRORS) {
size_t top;

if ((es->err_flags[i] & ERR_FLAG_CLEAR) != 0)
continue;

err_get_slot(thread_es);
top = thread_es->top;
err_clear(thread_es, top, 0);

thread_es->err_flags[top] = es->err_flags[i] & ~ERR_FLAG_MARK;
thread_es->err_buffer[top] = es->err_buffer[i];

err_set_debug(thread_es, top, es->err_file[i], es->err_line[i],
es->err_func[i]);

if (es->err_data[i] != NULL && es->err_data_size[i] != 0) {
void *data;
size_t data_sz = es->err_data_size[i];

data = CRYPTO_malloc(data_sz, NULL, 0);
if (data != NULL) {
memcpy(data, es->err_data[i], data_sz);
err_set_data(thread_es, top, data, data_sz,
es->err_data_flags[i] | ERR_TXT_MALLOCED);
}
}
}
}
6 changes: 6 additions & 0 deletions doc/build.info
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,10 @@ DEPEND[html/man3/OSSL_ENCODER_to_bio.html]=man3/OSSL_ENCODER_to_bio.pod
GENERATE[html/man3/OSSL_ENCODER_to_bio.html]=man3/OSSL_ENCODER_to_bio.pod
DEPEND[man/man3/OSSL_ENCODER_to_bio.3]=man3/OSSL_ENCODER_to_bio.pod
GENERATE[man/man3/OSSL_ENCODER_to_bio.3]=man3/OSSL_ENCODER_to_bio.pod
DEPEND[html/man3/OSSL_ERR_STATE_save.html]=man3/OSSL_ERR_STATE_save.pod
GENERATE[html/man3/OSSL_ERR_STATE_save.html]=man3/OSSL_ERR_STATE_save.pod
DEPEND[man/man3/OSSL_ERR_STATE_save.3]=man3/OSSL_ERR_STATE_save.pod
GENERATE[man/man3/OSSL_ERR_STATE_save.3]=man3/OSSL_ERR_STATE_save.pod
DEPEND[html/man3/OSSL_ESS_check_signing_certs.html]=man3/OSSL_ESS_check_signing_certs.pod
GENERATE[html/man3/OSSL_ESS_check_signing_certs.html]=man3/OSSL_ESS_check_signing_certs.pod
DEPEND[man/man3/OSSL_ESS_check_signing_certs.3]=man3/OSSL_ESS_check_signing_certs.pod
Expand Down Expand Up @@ -3312,6 +3316,7 @@ html/man3/OSSL_ENCODER.html \
html/man3/OSSL_ENCODER_CTX.html \
html/man3/OSSL_ENCODER_CTX_new_for_pkey.html \
html/man3/OSSL_ENCODER_to_bio.html \
html/man3/OSSL_ERR_STATE_save.html \
html/man3/OSSL_ESS_check_signing_certs.html \
html/man3/OSSL_HPKE_CTX_new.html \
html/man3/OSSL_HTTP_REQ_CTX.html \
Expand Down Expand Up @@ -3947,6 +3952,7 @@ man/man3/OSSL_ENCODER.3 \
man/man3/OSSL_ENCODER_CTX.3 \
man/man3/OSSL_ENCODER_CTX_new_for_pkey.3 \
man/man3/OSSL_ENCODER_to_bio.3 \
man/man3/OSSL_ERR_STATE_save.3 \
man/man3/OSSL_ESS_check_signing_certs.3 \
man/man3/OSSL_HPKE_CTX_new.3 \
man/man3/OSSL_HTTP_REQ_CTX.3 \
Expand Down
3 changes: 3 additions & 0 deletions include/internal/quic_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
*/
int ossl_quic_channel_net_error(QUIC_CHANNEL *ch);

/* Restore saved error state (best effort) */
void ossl_quic_channel_restore_err_state(QUIC_CHANNEL *ch);

/* For RXDP use. */
void ossl_quic_channel_on_remote_conn_close(QUIC_CHANNEL *ch,
OSSL_QUIC_FRAME_CONN_CLOSE *f);
Expand Down
5 changes: 5 additions & 0 deletions include/openssl/err.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,11 @@ int ERR_set_mark(void);
int ERR_pop_to_mark(void);
int ERR_clear_last_mark(void);

ERR_STATE *OSSL_ERR_STATE_new(void);
void OSSL_ERR_STATE_save(ERR_STATE *es);
void OSSL_ERR_STATE_restore(const ERR_STATE *es);
void OSSL_ERR_STATE_free(ERR_STATE *es);

#ifdef __cplusplus
}
#endif
Expand Down
29 changes: 28 additions & 1 deletion ssl/quic/quic_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
* https://www.openssl.org/source/license.html
*/

#include <openssl/rand.h>
#include <openssl/err.h>
#include "internal/quic_channel.h"
#include "internal/quic_error.h"
#include "internal/quic_rx_depack.h"
#include "../ssl_local.h"
#include "quic_channel_local.h"
#include <openssl/rand.h>

/*
* NOTE: While this channel implementation currently has basic server support,
Expand Down Expand Up @@ -339,6 +340,7 @@ static void ch_cleanup(QUIC_CHANNEL *ch)
ossl_qrx_free(ch->qrx);
ossl_quic_demux_free(ch->demux);
OPENSSL_free(ch->local_transport_params);
OSSL_ERR_STATE_free(ch->err_state);
}

QUIC_CHANNEL *ossl_quic_channel_new(const QUIC_CHANNEL_ARGS *args)
Expand Down Expand Up @@ -2196,11 +2198,23 @@ void ossl_quic_channel_on_new_conn_id(QUIC_CHANNEL *ch,
}
}

static void ch_save_err_state(QUIC_CHANNEL *ch)
{
if (ch->err_state == NULL)
ch->err_state = OSSL_ERR_STATE_new();

if (ch->err_state == NULL)
return;

OSSL_ERR_STATE_save(ch->err_state);
}

static void ch_raise_net_error(QUIC_CHANNEL *ch)
{
QUIC_TERMINATE_CAUSE tcause = {0};

ch->net_error = 1;
ch_save_err_state(ch);

tcause.error_code = QUIC_ERR_INTERNAL_ERROR;

Expand All @@ -2216,13 +2230,26 @@ int ossl_quic_channel_net_error(QUIC_CHANNEL *ch)
return ch->net_error;
}

void ossl_quic_channel_restore_err_state(QUIC_CHANNEL *ch)
{
if (ch == NULL)
return;

OSSL_ERR_STATE_restore(ch->err_state);
}


void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
uint64_t error_code,
uint64_t frame_type,
const char *reason)
{
QUIC_TERMINATE_CAUSE tcause = {0};

if (error_code == QUIC_ERR_INTERNAL_ERROR)
/* Internal errors might leave some errors on the stack. */
ch_save_err_state(ch);

tcause.error_code = error_code;
tcause.frame_type = frame_type;

Expand Down
3 changes: 3 additions & 0 deletions ssl/quic/quic_channel_local.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ struct quic_channel_st {

/* Permanent net error encountered */
unsigned int net_error : 1;

/* Saved error stack in case permanent error was encountered */
ERR_STATE *err_state;
};

# endif
Expand Down
17 changes: 10 additions & 7 deletions ssl/quic/quic_impl.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,23 @@ static int quic_raise_non_normal_error(QCTX *ctx,
{
va_list args;

ERR_new();
ERR_set_debug(file, line, func);

va_start(args, fmt);
ERR_vset_error(ERR_LIB_SSL, reason, fmt, args);
va_end(args);

if (ctx != NULL) {
if (ctx->is_stream && ctx->xso != NULL)
ctx->xso->last_error = SSL_ERROR_SSL;
else if (!ctx->is_stream && ctx->qc != NULL)
ctx->qc->last_error = SSL_ERROR_SSL;

if (reason == SSL_R_PROTOCOL_IS_SHUTDOWN && ctx->qc != NULL)
ossl_quic_channel_restore_err_state(ctx->qc->ch);
}

ERR_new();
ERR_set_debug(file, line, func);

va_start(args, fmt);
ERR_vset_error(ERR_LIB_SSL, reason, fmt, args);
va_end(args);

return 0;
}

Expand Down
4 changes: 4 additions & 0 deletions util/libcrypto.num
Original file line number Diff line number Diff line change
Expand Up @@ -5518,3 +5518,7 @@ X509_STORE_CTX_init_rpk ? 3_2_0 EXIST::FUNCTION:
X509_STORE_CTX_get0_rpk ? 3_2_0 EXIST::FUNCTION:
X509_STORE_CTX_set0_rpk ? 3_2_0 EXIST::FUNCTION:
CRYPTO_atomic_load_int ? 3_2_0 EXIST::FUNCTION:
OSSL_ERR_STATE_new ? 3_2_0 EXIST::FUNCTION:
OSSL_ERR_STATE_save ? 3_2_0 EXIST::FUNCTION:
OSSL_ERR_STATE_restore ? 3_2_0 EXIST::FUNCTION:
OSSL_ERR_STATE_free ? 3_2_0 EXIST::FUNCTION:

0 comments on commit a13d62f

Please sign in to comment.