Skip to content

Conversation

@rlubos
Copy link
Contributor

@rlubos rlubos commented Jul 11, 2018

Part of #7118.

This PR adds a generic config file for mbedTLS, that can be altered with Kconfig.

The idea behind this is to simplify TLS configuration for end-user. User can use Kconfig to select supported TLS algorithms, and the config file will automatically satisfy dependencies. It has been used with #7118, yet for this PR it has been freed from secure socket dependencies and improved a little bit.

For configs not covered by the generic config file (e. g. buffer sizes), a user can specify user config, that will be included at the end of generic config (similar behaviour as in the template config provided with mbedTLS).

The default configuration for config-tls-generic.h mimics the configuration of config-mini-tls1_2.h, hence it was set as a default config file. Tested with echo_client/echo_server samples with corresponding net-tools utils.

@rlubos rlubos requested a review from nashif as a code owner July 11, 2018 12:58
rlubos added 3 commits July 11, 2018 15:08
This commits provides a config file for mbedtls that can be modifed by
Kconfig. In result features like supported ciphersuites can be easily
adjusted from Kconfig.

Signed-off-by: Robert Lubos <[email protected]>
Default configuration of config-tls-generic.h mimics the current default
config file configuration - config-mini-tls1_2.h, thererfore it can be
safely used instead of it.

Signed-off-by: Robert Lubos <[email protected]>
Use the new, default mbedTLS config file in TLS configuration of
echo_client and echo_server.

Signed-off-by: Robert Lubos <[email protected]>
@rlubos rlubos force-pushed the generic-mbedtls-config branch from 2b508a0 to c722189 Compare July 11, 2018 13:15
Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

Codecov Report

Merging #8852 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8852   +/-   ##
=======================================
  Coverage   52.32%   52.32%           
=======================================
  Files         195      195           
  Lines       24730    24730           
  Branches     5140     5140           
=======================================
  Hits        12941    12941           
  Misses       9715     9715           
  Partials     2074     2074

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff0950a...c722189. Read the comment docs.

Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

I very much appreciate doing this.

I assume this is formally independent from #8814 and can be merged ahead of it (as a hint for project maintainers).

By default only DER (binary) format of certificates is supported. Enable
this option to enable support for PEM format.

config TLS_USER_CONFIG_ENABLE
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you consider having just a single Kconfig options for this? E.g. if TLS_USER_CONFIG_FILE is defined to non-empty value, #include it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was my initial thought, but that would require to check for an empty string in preprocessor, and I didn't find an easy way to do that. Any suggestions on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I never thought what #if "" would evaluate too, but it's actually an error to use strings in #if at all:

a.c:1:11: error: token ""foo"" is not valid in preprocessor expressions
 #define a "foo"
           ^
a.c:3:5: note: in expansion of macro ‘a’
 #if a
     ^

So, nevermind.

@rlubos
Copy link
Contributor Author

rlubos commented Jul 12, 2018

@pfalcon Yes, there are no dependencies between these PRs.

@carlescufi carlescufi merged commit 13b160b into zephyrproject-rtos:master Jul 13, 2018
@rlubos rlubos deleted the generic-mbedtls-config branch August 3, 2018 07:56
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.

5 participants