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

Compile when few options are set #292

Open
wants to merge 14 commits into
base: tls13-prototype
Choose a base branch
from

Conversation

hannestschofenig
Copy link
Owner

@hannestschofenig hannestschofenig commented Jul 15, 2021

Description

When the stack is compiled with minimal extensions (without 0RTT, ALPN, MFL, and MPS) then compilation errors occur due to unused variables.

Status

DONE

Requires Backporting

NO

Migrations

NO

Additional comments

Todos

  • Tests

Steps to test or reproduce

To reproduce the errors, use the config.h file from the EEMBC benchmark here:
https://raw.githubusercontent.com/eembc/mbedtls/eembc-setup/include/mbedtls/config.h

When the stack is compiled with minimal extensions (without 0RTT, ALPN, MFL, and MPS) then compilation errors occur
@petertorelli
Copy link
Collaborator

I'm hitting a new error:

/usr/bin/ld: CMakeFiles/fuzz_client.dir/fuzz_client.c.o: in function `LLVMFuzzerTestOneInput':
fuzz_client.c:(.text+0x5d): undefined reference to `mbedtls_test_cas_pem_len'
/usr/bin/ld: fuzz_client.c:(.text+0x67): undefined reference to `mbedtls_test_cas_pem'
collect2: error: ld returned 1 exit status
make[2]: *** [programs/fuzz/CMakeFiles/fuzz_client.dir/build.make:129: programs/fuzz/fuzz_client] Error 1
make[1]: *** [CMakeFiles/Makefile2:820: programs/fuzz/CMakeFiles/fuzz_client.dir/all] Error 2
make: *** [Makefile:130: all] Error 2

library/ssl_tls13_keys.c Outdated Show resolved Hide resolved
library/ssl_tls13_keys.c Outdated Show resolved Hide resolved
@@ -176,10 +176,13 @@ int ssl_write_early_data_process( mbedtls_ssl_context* ssl )
#endif /* MBEDTLS_SSL_USE_MPS */

#else /* MBEDTLS_ZERO_RTT */
#if defined(MBEDTLS_SSL_USE_MPS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ISTM that we should rather guard the declarations of buf, buf_len, ... by MBEDTLS_SSL_USE_MPS && MBEDTLS_ZERO_RTT.

@@ -2725,6 +2728,9 @@ static int ssl_encrypted_extensions_parse( mbedtls_ssl_context* ssl,
size_t ext_len;
const unsigned char *ext;

/* ssl structure is not used when ALPN, 0RTT, and MFL extensions are not used. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment should be replaced by the appropriate compile-time guard.

@@ -2269,6 +2269,10 @@ static int ssl_client_hello_fetch( mbedtls_ssl_context* ssl,

static void ssl_debug_print_client_hello_exts( mbedtls_ssl_context *ssl )
{
#if !defined(MBEDTLS_DEBUG_C)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ISTM that the entire function could be guarded by MBEDTLS_DEBUG_C and defined as a dummy if !MBEDTLS_DEBUG_C.

Copy link
Collaborator

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

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

Thanks Hannes - left a few minor change requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants