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

Add guards for MBEDTLS_X509_CRL_PARSE_C in sample #2540

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

RonEld
Copy link
Contributor

@RonEld RonEld commented Apr 4, 2019

Description

Add checks in ssl_server2 that MBEDTLS_X509_CRL_PARSE_C is defined
to fix compilation issue. Fixes #560.

Status

READY

Requires Backporting

Yes

Additional comments

  • The bug reporter supplied a patch, but hasn't accepted the CLA yet, so I haven't looked at the patch.
  • I have checked unsetting other configurations in the sni_entry (MBEDTLS_X509_CRT_PARSE_C and MBEDTLS_PK_C), but both of the are dependencies, wither direct or indirect of MBEDTLS_SSL_SERVER_NAME_INDICATION causing check_config.h to fail, so no problem in this case.

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

Steps to test or reproduce

  • unset MBEDTLS_X509_CRL_PARSE_C in the configuration
  • build the sample applications

Add checks in `ssl_server2` that `MBEDTLS_X509_CRL_PARSE_C` is defined
to fix compilation issue. Fixes Mbed-TLS#560.
@RonEld RonEld added bug mbed TLS team needs-review Every commit must be reviewed by at least two team members, component-tls labels Apr 4, 2019
@RonEld RonEld added the needs-backports Backports are missing or are pending review and approval. label Apr 4, 2019
@RonEld
Copy link
Contributor Author

RonEld commented Apr 4, 2019

Backports available in #2541 and #2542

Copy link
Contributor

@andresag01 andresag01 left a comment

Choose a reason for hiding this comment

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

Only a minor comment, but certainly not a show stopper.

@@ -729,6 +741,7 @@ sni_entry *sni_parse( char *sni_string )
if( mbedtls_x509_crl_parse_file( new->crl, crl_file ) != 0 )
goto error;
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: Please add the matching #endif comment /* MBEDTLS_X509_CRL_PARSE_C */. There are also a few other places where this happens.

@RonEld RonEld removed needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, labels Apr 23, 2019
@RonEld
Copy link
Contributor Author

RonEld commented Apr 23, 2019

Both backports have been fully reviewed and approved, so removing the "needs backports" label.
CC @Patater

@Patater Patater added the approved Design and code approved - may be waiting for CI or backports label Apr 23, 2019
@Patater Patater merged commit 80d0419 into Mbed-TLS:development Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-tls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build errors in programs/ssl/ssl_server2.c involving MBEDTLS_X509_CRL_PARSE_C
4 participants