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 configuration options to enable CMAC in mbedtls by default #7144

Merged
merged 2 commits into from
Jun 7, 2018

Conversation

k-stachowiak
Copy link
Contributor

@k-stachowiak k-stachowiak commented Jun 6, 2018

Description

This commit forces the CMAC feature to be enabled by default in Mbed TLS both in the regular and in the no-entropy configurations.

Warning! Currently the MBEDTLS_CMAC_C option is not present in the no-entropy config header, therefore the Mbed TLS scripting will append the option at the end of the file. This means the #define... ends up after the #ifndef... guard. This doesn't produce incorrect code, however is not perfect. A solution is to add (preferably commented out) CMAC define to the no-entropy configuration file upstream, which will make the scripting just uncomment the option making the code fully correct.

Edit:
The MBEDTLS_CMAC_C Has been added upstream, therefore the define for the no-entropy config will no longer be placed outside the header guards.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@0xc0170 0xc0170 requested a review from a team June 6, 2018 12:01
# Comments and uncomments #define lines in the given configuration header file
# to configure the file for use in mbed OS.
#
# Usage: adjust-config.sh [path to config script] [path to no-entropy config file]
Copy link
Contributor

Choose a reason for hiding this comment

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

Script name needs updating.

#
# This file is part of mbed TLS (https://tls.mbed.org)
#
# Copyright (c) 2015-2018, ARM Limited, All Rights Reserved
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright should be 2018.

Copy link

@mazimkhan mazimkhan left a comment

Choose a reason for hiding this comment

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

Looks Ok. Although, two different scripts are introduced for two different header files, while both scripts take file name as argument. Either single script should handle different files or filename argument should be removed for clear understanding of the scope of the script(s).

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 7, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

Build : ABORTED

Build number : 2271
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7144/

@cmonr
Copy link
Contributor

cmonr commented Jun 7, 2018

Odd.

/morph build

@cmonr
Copy link
Contributor

cmonr commented Jun 7, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

Build : ABORTED

Build number : 2274
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7144/

@cmonr
Copy link
Contributor

cmonr commented Jun 7, 2018

/morph build

1 similar comment
@cmonr
Copy link
Contributor

cmonr commented Jun 7, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

Build : SUCCESS

Build number : 2280
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7144/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

1 similar comment
@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

Build : SUCCESS

Build number : 2280
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7144/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

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

Successfully merging this pull request may close these issues.

None yet

7 participants