Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add s2n_strerror_source API #4209

Merged
merged 4 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/s2n.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ extern "C" {
* explaining the error in English by calling s2n_strerror(s2n_errno, "EN").
* A string containing human readable error name; can be generated with `s2n_strerror_name`.
* A string containing internal debug information, including filename and line number, can be generated with `s2n_strerror_debug`.
* A string containing only the filename and line number can be generated with `s2n_strerror_source`.
* This string is useful to include when reporting issues to the s2n-tls development team.
*
* @warning To avoid possible confusion, s2n_errno should be cleared after processing an error: `s2n_errno = S2N_ERR_T_OK`
Expand Down Expand Up @@ -407,6 +408,14 @@ S2N_API extern const char *s2n_strerror_debug(int error, const char *lang);
*/
S2N_API extern const char *s2n_strerror_name(int error);

/**
* Translates an s2n_error code to a filename and line number
*
* @param error The error code to explain. Usually this is s2n_errno
* @returns The error string.
*/
S2N_API extern const char *s2n_strerror_source(int error);

/**
* Opaque stack trace structure.
*/
Expand Down
20 changes: 18 additions & 2 deletions error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#endif

__thread int s2n_errno;
__thread const char *s2n_debug_str;
__thread struct s2n_debug_info _s2n_debug_info = { .debug_str = "", .source = "" };

/**
* Returns the address of the thread-local `s2n_errno` variable
Expand Down Expand Up @@ -378,7 +378,17 @@ const char *s2n_strerror_debug(int error, const char *lang)
return s2n_strerror(error, lang);
}

return s2n_debug_str;
return _s2n_debug_info.debug_str;
}

const char *s2n_strerror_source(int error)
{
/* No error, just return an empty string */
if (error == S2N_ERR_OK) {
return "";
}

return _s2n_debug_info.source;
}

int s2n_error_get_type(int error)
Expand All @@ -400,6 +410,12 @@ int s2n_stack_traces_enabled_set(bool newval)
return S2N_SUCCESS;
}

void s2n_debug_info_reset(void)
{
_s2n_debug_info.debug_str = "";
_s2n_debug_info.source = "";
}

#ifdef S2N_STACKTRACE

#define MAX_BACKTRACE_DEPTH 20
Expand Down
34 changes: 27 additions & 7 deletions error/s2n_errno.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <stdbool.h>
#include <stdio.h>
#include <string.h>

#include "api/s2n.h"
#include "utils/s2n_ensure.h"
Expand Down Expand Up @@ -320,18 +321,36 @@ typedef enum {
} s2n_error;

#define S2N_DEBUG_STR_LEN 128
extern __thread const char *s2n_debug_str;

struct s2n_debug_info {
const char *debug_str;
const char *source;
};

extern __thread struct s2n_debug_info _s2n_debug_info;

#define TO_STRING(s) #s
#define STRING_(s) TO_STRING(s)
#define STRING__LINE__ STRING_(__LINE__)

#define _S2N_DEBUG_LINE "Error encountered in " __FILE__ ":" STRING__LINE__
#define _S2N_ERROR(x) \
do { \
s2n_debug_str = _S2N_DEBUG_LINE; \
s2n_errno = (x); \
s2n_calculate_stacktrace(); \
/* gets the basename of a file path */
/* _S2N_EXTRACT_BASENAME("Error encountered in /path/to/my/file.c") -> "file.c" */
#if !(defined(CBMC) || defined(__TIMING_CONTRACTS__))
#define _S2N_RSPLIT(subject, c) (strrchr((subject), c) ? strrchr((subject), c) + 1 : (subject))
#define _S2N_EXTRACT_BASENAME(path) _S2N_RSPLIT((path) + strlen(_S2N_DEBUG_LINE_PREFIX), '/')
#else
#define _S2N_EXTRACT_BASENAME(path) ""
#endif

#define _S2N_DEBUG_LINE_PREFIX "Error encountered in "
#define _S2N_DEBUG_LINE _S2N_DEBUG_LINE_PREFIX __FILE__ ":" STRING__LINE__

#define _S2N_ERROR(x) \
do { \
_s2n_debug_info.debug_str = _S2N_DEBUG_LINE; \
_s2n_debug_info.source = _S2N_EXTRACT_BASENAME(_s2n_debug_info.debug_str); \
s2n_errno = (x); \
s2n_calculate_stacktrace(); \
} while (0)
#define S2N_ERROR_PRESERVE_ERRNO() \
do { \
Expand Down Expand Up @@ -374,3 +393,4 @@ int s2n_calculate_stacktrace(void);
int s2n_print_stacktrace(FILE *fptr);
int s2n_free_stacktrace(void);
int s2n_get_stacktrace(struct s2n_stacktrace *trace);
void s2n_debug_info_reset(void);
2 changes: 1 addition & 1 deletion scripts/s2n_safety_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def cmp_check(op):
* The size of the data pointed to by both the `destination` and `source` parameters,
shall be at least `len` bytes.
''',
impl = '__S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), {prefix}GUARD_PTR)',
impl = '__S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), {prefix}ENSURE_REF)',
harness = '''
static {ret} {prefix}CHECKED_MEMCPY_harness(uint32_t* dest, uint32_t* source, size_t len)
{{
Expand Down
3 changes: 3 additions & 0 deletions tests/cbmc/proofs/s2n_hmac_init/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,8 @@ UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.5:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.6:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.8:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.9:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.10:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.11:129
UNWINDSET += __CPROVER_file_local_s2n_hmac_c_s2n_tls_hmac_init.12:129

include ../Makefile.common
2 changes: 1 addition & 1 deletion tests/cbmc/proofs/s2n_stuffer_writev_bytes/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ REMOVE_FUNCTION_BODY += s2n_stuffer_wipe
REMOVE_FUNCTION_BODY += s2n_stuffer_wipe_n
REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl

UNWINDSET += s2n_stuffer_writev_bytes.18:$(call addone,$(MAX_IOVEC_SIZE))
UNWINDSET += s2n_stuffer_writev_bytes.20:$(call addone,$(MAX_IOVEC_SIZE))
UNWINDSET += s2n_stuffer_writev_bytes_harness.0:$(call addone,$(MAX_IOVEC_SIZE))

include ../Makefile.common
4 changes: 1 addition & 3 deletions tests/cbmc/stubs/s2n_ensure.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@

#include "utils/s2n_safety.h"

void* s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size, const char *debug_str)
void* s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size)
{
if (to == NULL || from == NULL) {
s2n_errno = S2N_ERR_NULL;
s2n_debug_str = debug_str;
return NULL;
}

Expand Down
16 changes: 8 additions & 8 deletions tests/s2n_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,18 @@ int test_count;
s2n_print_stacktrace(stderr); \
if (isatty(fileno(stderr))) { \
errno = real_errno; \
fprintf(stderr, "\033[31;1mFAILED test %d\033[0m\n%s (%s:%d)\nError Message: '%s'\n Debug String: '%s'\n System Error: %s (%d)\n", test_count, (msg), __FILE__, __LINE__, s2n_strerror(s2n_errno, "EN"), s2n_debug_str, strerror(errno), errno); \
fprintf(stderr, "\033[31;1mFAILED test %d\033[0m\n%s (%s:%d)\nError Message: '%s'\n Debug String: '%s'\n System Error: %s (%d)\n", test_count, (msg), __FILE__, __LINE__, s2n_strerror(s2n_errno, "EN"), s2n_strerror_debug(s2n_errno, "EN"), strerror(errno), errno); \
} \
else { \
errno = real_errno; \
fprintf(stderr, "FAILED test %d\n%s (%s:%d)\nError Message: '%s'\n Debug String: '%s'\n System Error: %s (%d)\n", test_count, (msg), __FILE__, __LINE__, s2n_strerror(s2n_errno, "EN"), s2n_debug_str, strerror(errno), errno); \
fprintf(stderr, "FAILED test %d\n%s (%s:%d)\nError Message: '%s'\n Debug String: '%s'\n System Error: %s (%d)\n", test_count, (msg), __FILE__, __LINE__, s2n_strerror(s2n_errno, "EN"), s2n_strerror_debug(s2n_errno, "EN"), strerror(errno), errno); \
} \
} while(0)

#define RESET_ERRNO() \
do { \
s2n_errno = 0; \
s2n_debug_str = NULL; \
s2n_debug_info_reset(); \
errno = 0; \
} while(0);

Expand All @@ -140,22 +140,22 @@ int test_count;
do { \
EXPECT_EQUAL( (function_call) , -1 ); \
EXPECT_NOT_EQUAL(s2n_errno, 0); \
EXPECT_NOT_NULL(s2n_debug_str); \
EXPECT_NOT_NULL(s2n_strerror_debug(s2n_errno, "EN")); \
RESET_ERRNO(); \
} while(0)
#define EXPECT_ERROR( function_call ) \
do { \
EXPECT_TRUE( s2n_result_is_error(function_call) ); \
EXPECT_NOT_EQUAL(s2n_errno, 0); \
EXPECT_NOT_NULL(s2n_debug_str); \
EXPECT_NOT_NULL(s2n_strerror_debug(s2n_errno, "EN")); \
RESET_ERRNO(); \
} while(0)

#define EXPECT_FAILURE_WITH_ERRNO_NO_RESET( function_call, err ) \
do { \
EXPECT_EQUAL( (function_call), -1 ); \
EXPECT_EQUAL(s2n_errno, err); \
EXPECT_NOT_NULL(s2n_debug_str); \
EXPECT_NOT_NULL(s2n_strerror_debug(s2n_errno, "EN")); \
} while(0)

#define EXPECT_FAILURE_WITH_ERRNO( function_call, err ) \
Expand All @@ -178,7 +178,7 @@ int test_count;
do { \
EXPECT_TRUE( s2n_result_is_error(function_call) ); \
EXPECT_EQUAL(s2n_errno, err); \
EXPECT_NOT_NULL(s2n_debug_str); \
EXPECT_NOT_NULL(s2n_strerror_debug(s2n_errno, "EN")); \
} while(0)

/* for use with S2N_RESULT */
Expand All @@ -192,7 +192,7 @@ int test_count;
do { \
EXPECT_NULL( (function_call) ); \
EXPECT_EQUAL(s2n_errno, err); \
EXPECT_NOT_NULL(s2n_debug_str); \
EXPECT_NOT_NULL(s2n_strerror_debug(s2n_errno, "EN")); \
} while(0)

#define EXPECT_NULL_WITH_ERRNO( function_call, err ) \
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to your PR, but should this even be committed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not...? No idea 😄

Original file line number Diff line number Diff line change
Expand Up @@ -3762,7 +3762,7 @@ [email protected] -> (- 14934)
[email protected] -> (- 19248)
s_s2n_in_unit_test -> (- 11758)
s2n_calculate_stacktrace -> (- 46273)
s2n_debug_str -> (- 32776)
_s2n_debug_info -> (- 32776)
s2n_errno -> (- 32784)
s2n_hash_copy -> (- 56593)
s2n_hash_digest_size -> (- 59689)
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/s2n_error_lookup_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
#include "error/s2n_errno.h"
#include "s2n_test.h"
#include "testlib/s2n_testlib.h"
#include "utils/s2n_safety.h"

S2N_RESULT test_function(bool is_valid)
{
/* the line number of this check is important; tests below will fail if it changed */
RESULT_ENSURE(is_valid, S2N_ERR_SAFETY);
return S2N_RESULT_OK;
}

int main(void)
{
Expand Down Expand Up @@ -86,6 +94,18 @@ int main(void)
EXPECT_EQUAL(strcmp(s2n_strerror_name((S2N_ERR_T_USAGE + 1) << S2N_ERR_NUM_VALUE_BITS), "Internal s2n error"), 0);
EXPECT_EQUAL(strcmp(s2n_strerror((S2N_ERR_T_USAGE + 1) << S2N_ERR_NUM_VALUE_BITS, "EN"), "Internal s2n error"), 0);

/* Ensure the file/line information is returned as expected */
s2n_result_ignore(test_function(false));
EXPECT_EQUAL(strcmp(s2n_strerror_source(S2N_ERR_SAFETY), "s2n_error_lookup_test.c:25"), 0);

/* constructs a debug string from a path */
#define EXAMPLE_DEBUG_STR(path) (_S2N_EXTRACT_BASENAME(_S2N_DEBUG_LINE_PREFIX path))

EXPECT_EQUAL(strcmp(EXAMPLE_DEBUG_STR("/absolute/path/to/file.c"), "file.c"), 0);
EXPECT_EQUAL(strcmp(EXAMPLE_DEBUG_STR("relative/path/to/file.c"), "file.c"), 0);
EXPECT_EQUAL(strcmp(EXAMPLE_DEBUG_STR("path / with / spaces /file.c"), "file.c"), 0);
EXPECT_EQUAL(strcmp(EXAMPLE_DEBUG_STR("file.c"), "file.c"), 0);

/* Test that lookup works even after s2n_cleanup */
EXPECT_SUCCESS(s2n_cleanup());

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_kex_with_kem_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static int assert_pq_disabled_checks(struct s2n_cipher_suite *cipher_suite, cons

POSIX_GUARD(s2n_connection_free(server_conn));
s2n_errno = 0;
s2n_debug_str = NULL;
s2n_debug_info_reset();

return S2N_SUCCESS;
}
Expand Down
6 changes: 3 additions & 3 deletions tls/s2n_handshake_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ int s2n_conn_set_handshake_type(struct s2n_connection *conn)

if (conn->config->use_tickets) {
if (conn->session_ticket_status == S2N_DECRYPT_TICKET) {
/* We reuse the session if a valid TLS12 ticket is provided.
/* We reuse the session if a valid TLS12 ticket is provided.
* Otherwise, we will perform a full handshake and then generate
* a new session ticket. */
if (s2n_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt) == S2N_SUCCESS) {
Expand Down Expand Up @@ -1639,7 +1639,7 @@ int s2n_negotiate_impl(struct s2n_connection *conn, s2n_blocked_status *blocked)
/* Non-retryable write error. The peer might have sent an alert. Try and read it. */
const int write_errno = errno;
const int write_s2n_errno = s2n_errno;
const char *write_s2n_debug_str = s2n_debug_str;
struct s2n_debug_info write_s2n_debug_info = _s2n_debug_info;

if (s2n_handshake_read_io(conn) < 0 && s2n_errno == S2N_ERR_ALERT) {
/* s2n_handshake_read_io has set s2n_errno */
Expand All @@ -1648,7 +1648,7 @@ int s2n_negotiate_impl(struct s2n_connection *conn, s2n_blocked_status *blocked)
/* Let the write error take precedence if we didn't read an alert. */
errno = write_errno;
s2n_errno = write_s2n_errno;
s2n_debug_str = write_s2n_debug_str;
_s2n_debug_info = write_s2n_debug_info;
S2N_ERROR_PRESERVE_ERRNO();
}
}
Expand Down
4 changes: 1 addition & 3 deletions utils/s2n_ensure.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@

#include "utils/s2n_safety.h"

void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size, const char *debug_str)
void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size)
{
if (to == NULL || from == NULL) {
s2n_errno = S2N_ERR_NULL;
s2n_debug_str = debug_str;
return NULL;
}

Expand Down
20 changes: 10 additions & 10 deletions utils/s2n_ensure.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@
#define __S2N_ENSURE_POSTCONDITION(result) (s2n_likely(s2n_result_is_ok(result)) ? S2N_RESULT_OK : S2N_RESULT_ERROR)
#endif

#define __S2N_ENSURE_SAFE_MEMCPY(d, s, n, guard) \
do { \
__typeof(n) __tmp_n = (n); \
if (s2n_likely(__tmp_n)) { \
void *r = s2n_ensure_memcpy_trace((d), (s), (__tmp_n), _S2N_DEBUG_LINE); \
guard(r); \
} \
#define __S2N_ENSURE_SAFE_MEMCPY(d, s, n, guard) \
do { \
__typeof(n) __tmp_n = (n); \
if (s2n_likely(__tmp_n)) { \
void *r = s2n_ensure_memcpy_trace((d), (s), (__tmp_n)); \
guard(r); \
} \
} while (0)

#define __S2N_ENSURE_SAFE_MEMSET(d, c, n, guard) \
Expand Down Expand Up @@ -103,9 +103,9 @@
*
*/
#if defined(S2N___RESTRICT__SUPPORTED)
void *s2n_ensure_memcpy_trace(void *__restrict__ to, const void *__restrict__ from, size_t size, const char *debug_str);
void *s2n_ensure_memcpy_trace(void *__restrict__ to, const void *__restrict__ from, size_t size);
#else
void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size, const char *debug_str);
void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size_t size);
#endif

/**
Expand Down Expand Up @@ -152,7 +152,7 @@ void *s2n_ensure_memcpy_trace(void *restrict to, const void *restrict from, size
*/
#ifdef CBMC
#define CONTRACT_ASSIGNS(...) __CPROVER_assigns(__VA_ARGS__)
#define CONTRACT_ASSIGNS_ERR(...) CONTRACT_ASSIGNS(__VA_ARGS__, s2n_debug_str, s2n_errno)
#define CONTRACT_ASSIGNS_ERR(...) CONTRACT_ASSIGNS(__VA_ARGS__, _s2n_debug_info, s2n_errno)
#define CONTRACT_REQUIRES(...) __CPROVER_requires(__VA_ARGS__)
#define CONTRACT_ENSURES(...) __CPROVER_ensures(__VA_ARGS__)
#define CONTRACT_INVARIANT(...) __CPROVER_loop_invariant(__VA_ARGS__)
Expand Down
6 changes: 3 additions & 3 deletions utils/s2n_safety_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
* * The size of the data pointed to by both the `destination` and `source` parameters,
* shall be at least `len` bytes.
*/
#define RESULT_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), RESULT_GUARD_PTR)
#define RESULT_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), RESULT_ENSURE_REF)

/**
* Performs a safer memset
Expand Down Expand Up @@ -357,7 +357,7 @@
* * The size of the data pointed to by both the `destination` and `source` parameters,
* shall be at least `len` bytes.
*/
#define POSIX_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), POSIX_GUARD_PTR)
#define POSIX_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), POSIX_ENSURE_REF)

/**
* DEPRECATED: all methods (except those in s2n.h) should return s2n_result.
Expand Down Expand Up @@ -563,7 +563,7 @@
* * The size of the data pointed to by both the `destination` and `source` parameters,
* shall be at least `len` bytes.
*/
#define PTR_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), PTR_GUARD)
#define PTR_CHECKED_MEMCPY(destination, source, len) __S2N_ENSURE_SAFE_MEMCPY((destination), (source), (len), PTR_ENSURE_REF)

/**
* DEPRECATED: all methods (except those in s2n.h) should return s2n_result.
Expand Down