Skip to content

Commit

Permalink
feat: add s2n_strerror_file_line API
Browse files Browse the repository at this point in the history
  • Loading branch information
camshaft committed Sep 20, 2023
1 parent 7afd286 commit 87b6250
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 46 deletions.
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_file_line`.
* 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_file_line(int error);

/**
* Opaque stack trace structure.
*/
Expand Down
2 changes: 1 addition & 1 deletion codebuild/bin/grep_simple_mistakes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ FAILED=0
# Grep for any instances of raw memcpy() function. s2n code should instead be
# using one of the *_ENSURE_MEMCPY macros.
#############################################
S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/pq-crypto/*")
S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/pq-crypto/*" -not -path "*/s2n_ensure.[ch]")
for file in $S2N_FILES_ASSERT_NOT_USING_MEMCPY; do
RESULT_NUM_LINES=`grep 'memcpy(' $file | wc -l`
if [ "${RESULT_NUM_LINES}" != 0 ]; then
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 = "", .file_line = "" };

/**
* Returns the address of the thread-local `s2n_errno` variable
Expand Down Expand Up @@ -379,7 +379,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_file_line(int error)
{
/* No error, just return an empty string */
if (error == S2N_ERR_OK) {
return "";
}

return _s2n_debug_info.file_line;
}

int s2n_error_get_type(int error)
Expand All @@ -401,6 +411,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.file_line = "";
}

#ifdef S2N_STACKTRACE

#define MAX_BACKTRACE_DEPTH 20
Expand Down
40 changes: 31 additions & 9 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 @@ -321,18 +322,38 @@ 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* file_line;
};

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__)

/* gets the basename of a file path */
/* _S2N_BASENAME("/path/to/my/file.c") -> "file.c" */
#if !(defined(CBMC) || defined(__TIMING_CONTRACTS__))
#define _S2N_BASENAME(path) (strrchr((path), '/') ? strrchr((path), '/') + 1 : (path))
#else
#define _S2N_BASENAME(path) ""
#endif

#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(); \

#define _S2N_DEBUG_INFO ((struct s2n_debug_info){ \
.debug_str = _S2N_DEBUG_LINE, \
.file_line = _S2N_BASENAME(_S2N_DEBUG_LINE) })

#define _S2N_ERROR(x) \
do { \
_s2n_debug_info.debug_str = _S2N_DEBUG_LINE; \
_s2n_debug_info.file_line = _S2N_BASENAME(_s2n_debug_info.debug_str); \
s2n_errno = (x); \
s2n_calculate_stacktrace(); \
} while (0)
#define S2N_ERROR_PRESERVE_ERRNO() \
do { \
Expand Down Expand Up @@ -364,14 +385,15 @@ extern __thread const char *s2n_debug_str;

/** Calculate and print stacktraces */
struct s2n_stacktrace {
char **trace;
char** trace;
int trace_size;
};

bool s2n_stack_traces_enabled();
int s2n_stack_traces_enabled_set(bool newval);

int s2n_calculate_stacktrace(void);
int s2n_print_stacktrace(FILE *fptr);
int s2n_print_stacktrace(FILE* fptr);
int s2n_free_stacktrace(void);
int s2n_get_stacktrace(struct s2n_stacktrace *trace);
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
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
11 changes: 11 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,9 @@ 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);

s2n_result_ignore(test_function(false));
EXPECT_EQUAL(strcmp(s2n_strerror_file_line(S2N_ERR_SAFETY), "s2n_error_lookup_test.c:25"), 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
Loading

0 comments on commit 87b6250

Please sign in to comment.