From b5e6a77010a859e13bd177f96d786de91c6c2212 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 11:23:31 +0300 Subject: [PATCH 01/15] Add Contribution guidelines to github Add Contribution Guidelines that will be shown in github, when PRs are made. --- CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..55ebf15b1 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,40 @@ +We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: + +- All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. +- To accept the Contributor’s Licence Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions should be fully tested before submission. +As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. + +### Making a Contribution + +1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. +2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. +3. Write a test which shows that the bug was fixed or that the feature works as expected. +4. Send a pull request and bug us until it gets merged and published. Contributions may need some modifications, so work with us to get your change accepted. We will include your name in the ChangeLog :) + +### Backports + +mbed TLS maintains some legacy branches, which are release as LTS versions. As such, backporting to these branches should be handled according to the following rules: + +1. If the contribution is a new feature\enhancement, no backporting is needed +2. Bug fixes should be backported, as long as the legacy branches have these bugs reproduced +3. Changes in the API, do not require backporting. If a bug fix introduced new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. + +It would be highly appreciated if a contribution would be backported to a legacy branch as well. +At the moment, the legacy branches are: + +1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) +2. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) +3. [development](https://github.com/ARMmbed/mbedtls/tree/development) + +### Tests + +As mentioned, tests that show the correctness of the feature\bug fix should be added to the Pull Request, if not such test exist. +mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. + +### Continuous Integration Tests + +Once a PR has been made, the Continuous Integration tests ( CI ) are triggered and run. You should follow the result of the CI tests, and fix failures. + + + \ No newline at end of file From 7f888982fd3a2d924b890ca7c8c0d23faf7d79a1 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 16:05:52 +0300 Subject: [PATCH 02/15] Modify Contribution Guidelines after comments Modify the Contribution guidelines after comments from Gilles, Andres and Jaeden --- CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 55ebf15b1..bfd6cb3d7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,24 +1,32 @@ We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: + - As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. + - The contribution should not break API or ABI, unless there is a real justification for that. If there is an API change, the contribution, if accepted, will be merged only when there will be a major release. + +### Contributor License Agreement ( CLA ) - All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. -- To accept the Contributor’s Licence Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. -- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions should be fully tested before submission. -As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. +- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. + +### Coding Standards +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. +- The code should be written in a clean and readable style. +- The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. +- The code should be secure, and will be reviewed in a security point of view as well. ### Making a Contribution 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. 2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. 3. Write a test which shows that the bug was fixed or that the feature works as expected. -4. Send a pull request and bug us until it gets merged and published. Contributions may need some modifications, so work with us to get your change accepted. We will include your name in the ChangeLog :) +4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) ### Backports -mbed TLS maintains some legacy branches, which are release as LTS versions. As such, backporting to these branches should be handled according to the following rules: +mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: -1. If the contribution is a new feature\enhancement, no backporting is needed -2. Bug fixes should be backported, as long as the legacy branches have these bugs reproduced -3. Changes in the API, do not require backporting. If a bug fix introduced new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. +1. If the contribution is a new feature or enhancement, no backporting is needed. +2. Bug fixes should be backported to the legacy branches containing these bugs. +3. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. It would be highly appreciated if a contribution would be backported to a legacy branch as well. At the moment, the legacy branches are: @@ -29,12 +37,24 @@ At the moment, the legacy branches are: ### Tests -As mentioned, tests that show the correctness of the feature\bug fix should be added to the Pull Request, if not such test exist. +As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. +Sample applications, if needed, should be modified as well. + ### Continuous Integration Tests -Once a PR has been made, the Continuous Integration tests ( CI ) are triggered and run. You should follow the result of the CI tests, and fix failures. +Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. + +### Documentation + +mbed TLS should be well documented. If documentation is needed, speak out! + +1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. +2. Complex parts in the code should include comments. +3. If needed, a Readme file is advised +4. If a KB article should be added, write this as a comment in the PR description. +5. A Changelog entry should be added for this contribution. \ No newline at end of file From 1680d3dc1929f325f80530b8eb97a11fc96296bf Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 17:28:21 +0300 Subject: [PATCH 03/15] Add a couple of statements to the contribution section Add a notice for short contributions, and for Apache license header that should be added. Added an adivce to enable the git hooks scripts as well. --- CONTRIBUTING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bfd6cb3d7..95219e544 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,8 @@ We gratefully accept bug reports and contributions from the community. There are 2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. 3. Write a test which shows that the bug was fixed or that the feature works as expected. 4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +5. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. +6. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. ### Backports @@ -45,6 +47,7 @@ Sample applications, if needed, should be modified as well. ### Continuous Integration Tests Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. +It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtls/tree/development/tests/git-scripts) prior to pushing your changes, for catching some of the issues as early as possible. ### Documentation From ea24d75c67d79b31d50499affa66ec88d3756e59 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 31 Aug 2017 17:02:01 +0300 Subject: [PATCH 04/15] Addres Andres' comment Update the document after Andres review comments --- CONTRIBUTING.md | 61 +++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95219e544..f7bf5f8db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,63 +1,64 @@ +Contributing +============ We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: - As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. - The contribution should not break API or ABI, unless there is a real justification for that. If there is an API change, the contribution, if accepted, will be merged only when there will be a major release. -### Contributor License Agreement ( CLA ) +Contributor License Agreement (CLA) +----------------------------------- - All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. - To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. -### Coding Standards +Coding Standards +---------------- - We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. - The code should be written in a clean and readable style. - The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. -- The code should be secure, and will be reviewed in a security point of view as well. - -### Making a Contribution +- The code should be secure, and will be reviewed from a security point of view as well. +Making a Contribution +--------------------- 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. -2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. -3. Write a test which shows that the bug was fixed or that the feature works as expected. -4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) -5. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. -6. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. - -### Backports +1. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. +1. Write a test which shows that the bug was fixed or that the feature works as expected. +1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +1. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. +1. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. +Backports +--------- mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: 1. If the contribution is a new feature or enhancement, no backporting is needed. -2. Bug fixes should be backported to the legacy branches containing these bugs. -3. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. +1. Bug fixes should be backported to the legacy branches containing these bugs. +1. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. It would be highly appreciated if a contribution would be backported to a legacy branch as well. At the moment, the legacy branches are: -1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) -2. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) -3. [development](https://github.com/ARMmbed/mbedtls/tree/development) - -### Tests +1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) +1. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) +1. [development](https://github.com/ARMmbed/mbedtls/tree/development) +Tests +----- As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. Sample applications, if needed, should be modified as well. -### Continuous Integration Tests - +Continuous Integration Tests +---------------------------- Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtls/tree/development/tests/git-scripts) prior to pushing your changes, for catching some of the issues as early as possible. -### Documentation - +Documentation +------------- mbed TLS should be well documented. If documentation is needed, speak out! 1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. -2. Complex parts in the code should include comments. -3. If needed, a Readme file is advised -4. If a KB article should be added, write this as a comment in the PR description. -5. A Changelog entry should be added for this contribution. - - - \ No newline at end of file +1. Complex parts in the code should include comments. +1. If needed, a Readme file is advised. +1. If a [Knowledge Base (KB)](https://tls.mbed.org/kb) article should be added, write this as a comment in the PR description. +1. A [ChangeLog](https://github.com/ARMmbed/mbedtls/blob/development/ChangeLog) entry should be added for this contribution. From 0a47d127170a94c76932a9b1dcc4525fd8521435 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 3 Sep 2017 10:20:25 +0300 Subject: [PATCH 05/15] Rephrase the backport sectio Rephrase the backport sectoin, since development branch is not a legacy branch --- CONTRIBUTING.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7bf5f8db..c1870547b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,12 +34,11 @@ mbed TLS maintains some legacy branches, which are released as LTS versions. mbe 1. Bug fixes should be backported to the legacy branches containing these bugs. 1. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. -It would be highly appreciated if a contribution would be backported to a legacy branch as well. +It would be highly appreciated if a contribution would be backported to a legacy branch in addition to the [development branch](https://github.com/ARMmbed/mbedtls/tree/development). At the moment, the legacy branches are: 1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) 1. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) -1. [development](https://github.com/ARMmbed/mbedtls/tree/development) Tests ----- From b2231fc31a8e7840734b5fd6d9b64d30635ac3d4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 10 Sep 2017 17:32:05 +0300 Subject: [PATCH 06/15] Address review comments Addres review comments done by Hanno --- CONTRIBUTING.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c1870547b..3c6dc74c8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,12 +7,12 @@ We gratefully accept bug reports and contributions from the community. There are Contributor License Agreement (CLA) ----------------------------------- -- All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. -- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. +- All contributions, whether large or small, require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. +- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an Mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an Mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to Arm as described in the instructions given. Coding Standards ---------------- -- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission, as mentioned in the [Tests](#tests) and [Continuous Integration](#continuous-integration-tests) sections. - The code should be written in a clean and readable style. - The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. - The code should be secure, and will be reviewed from a security point of view as well. @@ -20,15 +20,15 @@ Coding Standards Making a Contribution --------------------- 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. -1. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. +1. Fork the [Mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. 1. Write a test which shows that the bug was fixed or that the feature works as expected. -1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) 1. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. -1. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. +1. Mbed TLS is released under the Apache license, and as such, all the added files should include the Apache license header. Backports --------- -mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: +Mbed TLS maintains some legacy branches, which are released as LTS versions. Mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: 1. If the contribution is a new feature or enhancement, no backporting is needed. 1. Bug fixes should be backported to the legacy branches containing these bugs. @@ -42,8 +42,8 @@ At the moment, the legacy branches are: Tests ----- -As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. -mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. +As mentioned, tests that show the correctness of the feature or bug fix should be added to the pull request, if no such tests exist. +Mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. Sample applications, if needed, should be modified as well. @@ -54,7 +54,7 @@ It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtl Documentation ------------- -mbed TLS should be well documented. If documentation is needed, speak out! +Mbed TLS should be well documented. If documentation is needed, speak out! 1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. 1. Complex parts in the code should include comments. From 8db3efbc76243971adcae0d5abe439bc3af931f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 21 Feb 2018 19:16:20 +0100 Subject: [PATCH 07/15] Add missing MBEDTLS_DEPRECATED_REMOVED guards Add missing MBEDTLS_DEPRECATED_REMOVED guards around the definitions of mbedtls_aes_decrypt and mbedtls_aes_encrypt. This fixes the build under -Wmissing-prototypes -Werror. Fixes #1388 --- ChangeLog | 2 ++ library/aes.c | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 5f49c0beb..9a61ec31d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,8 @@ Bugfix * Fix the name of a DHE parameter that was accidentally changed in 2.7.0. Fixes #1358. * Fix test_suite_pk to work on 64-bit ILP32 systems. #849 + * Don't define mbedtls_aes_decrypt and mbedtls_aes_encrypt under + MBEDTLS_DEPRECATED_REMOVED. #1388 Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. diff --git a/library/aes.c b/library/aes.c index dba4a5f57..3d2eac82d 100644 --- a/library/aes.c +++ b/library/aes.c @@ -765,12 +765,14 @@ int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, } #endif /* !MBEDTLS_AES_ENCRYPT_ALT */ +#if !defined(MBEDTLS_DEPRECATED_REMOVED) void mbedtls_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { mbedtls_internal_aes_encrypt( ctx, input, output ); } +#endif /* !MBEDTLS_DEPRECATED_REMOVED */ /* * AES-ECB block decryption @@ -831,12 +833,14 @@ int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, } #endif /* !MBEDTLS_AES_DECRYPT_ALT */ +#if !defined(MBEDTLS_DEPRECATED_REMOVED) void mbedtls_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { mbedtls_internal_aes_decrypt( ctx, input, output ); } +#endif /* !MBEDTLS_DEPRECATED_REMOVED */ /* * AES-ECB block encryption/decryption From cf092b2ccf6fe88ec7b6e075aa89d93cadaa059a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 6 Mar 2018 14:23:38 +0000 Subject: [PATCH 08/15] Deprecate support for record compression --- ChangeLog | 4 ++++ include/mbedtls/check_config.h | 8 ++++++++ include/mbedtls/config.h | 3 +++ 3 files changed, 15 insertions(+) diff --git a/ChangeLog b/ChangeLog index 68fb6f5e9..75a8f1186 100644 --- a/ChangeLog +++ b/ChangeLog @@ -26,6 +26,10 @@ Features OpenVPN Inc. Fixes #1339 * Add support for public keys encoded in PKCS#1 format. #1122 +New deprecations + * Deprecate support for record compression (configuration option + MBEDTLS_ZLIB_SUPPORT). + Bugfix * Fix the name of a DHE parameter that was accidentally changed in 2.7.0. Fixes #1358. diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index be8033296..655612e20 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -66,6 +66,14 @@ #error "MBEDTLS_HAVE_TIME_DATE without MBEDTLS_HAVE_TIME does not make sense" #endif +#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_WARNING) +#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will likely be removed in a future version of the library" +#endif + +#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_REMOVED) +#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" +#endif + #if defined(MBEDTLS_AESNI_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_AESNI_C defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 1c98558eb..05f67fa3c 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1541,6 +1541,9 @@ * * \note Currently compression can't be used with DTLS. * + * \deprecated This feature is deprecated and will likely be removed + * in a future version of the library. + * * Used in: library/ssl_tls.c * library/ssl_cli.c * library/ssl_srv.c From e494e20f0c39499badb1a52eaafea23d2f7b02db Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Mar 2018 13:26:12 +0000 Subject: [PATCH 09/15] Move and reword deprecation warning/error on compression support --- include/mbedtls/check_config.h | 8 -------- include/mbedtls/config.h | 4 ++-- include/mbedtls/ssl.h | 9 +++++++++ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 655612e20..be8033296 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -66,14 +66,6 @@ #error "MBEDTLS_HAVE_TIME_DATE without MBEDTLS_HAVE_TIME does not make sense" #endif -#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_WARNING) -#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will likely be removed in a future version of the library" -#endif - -#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_REMOVED) -#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" -#endif - #if defined(MBEDTLS_AESNI_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_AESNI_C defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 05f67fa3c..d47e9c7af 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1541,8 +1541,8 @@ * * \note Currently compression can't be used with DTLS. * - * \deprecated This feature is deprecated and will likely be removed - * in a future version of the library. + * \deprecated This feature is deprecated and will be removed + * in the next major revision of the library. * * Used in: library/ssl_tls.c * library/ssl_cli.c diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 51e843ae2..a67971722 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -49,6 +49,15 @@ #endif #if defined(MBEDTLS_ZLIB_SUPPORT) + +#if defined(MBEDTLS_DEPRECATED_WARNING) +#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will be removed in the next major revision of the library" +#endif + +#if defined(MBEDTLS_DEPRECATED_REMOVED) +#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" +#endif + #include "zlib.h" #endif From a1098f81c252b317ad34ea978aea2bc47760b215 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:28:49 +0100 Subject: [PATCH 10/15] Add bounds check before signature length read --- library/ssl_cli.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 2534346a4..279a127ba 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2478,6 +2478,14 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) /* * Read signature */ + + if( p > end - 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } sig_len = ( p[0] << 8 ) | p[1]; p += 2; From 027f84c69f4ef30c0693832a6c396ef19e563ca1 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:29:24 +0100 Subject: [PATCH 11/15] Prevent arithmetic overflow on bounds check --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 279a127ba..df6abc389 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2489,7 +2489,7 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) sig_len = ( p[0] << 8 ) | p[1]; p += 2; - if( end != p + sig_len ) + if( p != end - sig_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, From 740b218386083dc708ce98ccc94a63a95cd5629e Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:31:14 +0100 Subject: [PATCH 12/15] Add bounds check before length read --- library/ssl_cli.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 2534346a4..585750ef2 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2057,6 +2057,12 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, * * opaque psk_identity_hint<0..2^16-1>; */ + if( (*p) > end - 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " + "(psk_identity_hint length)" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } len = (*p)[0] << 8 | (*p)[1]; *p += 2; From 5224a7544c95552553e2e6be0b4a789956a6464e Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:31:38 +0100 Subject: [PATCH 13/15] Prevent arithmetic overflow on bounds check --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 585750ef2..759a4562a 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2066,7 +2066,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, len = (*p)[0] << 8 | (*p)[1]; *p += 2; - if( (*p) + len > end ) + if( (*p) > end - len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " "(psk_identity_hint length)" ) ); From 00bbf572afc5558026a65ccb1000023bd1ce872d Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Wed, 14 Mar 2018 11:14:13 +0100 Subject: [PATCH 14/15] Update change log --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index dfd34bf69..6e497bc1d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Security implementation allowed an offline 2^80 brute force attack on the HMAC key of a single, uninterrupted connection (with no resumption of the session). + * Fix a buffer overread in ssl_parse_server_key_exchange() that could cause + a crash on invalid input. Features * Extend PKCS#8 interface by introducing support for the entire SHA @@ -44,6 +46,8 @@ Bugfix Nick Wilson on issue #355 * In test_suite_pk, pass valid parameters when testing for hash length overflow. #1179 + * Fix a possible arithmetic overflow in ssl_parse_server_key_exchange() + that could cause a key exchange to fail on valid data. Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. From 7fa1ae70c85e847fcd5e434b1417c8dc4cc62c72 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 17:17:38 +0100 Subject: [PATCH 15/15] Add Changelog entry --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index dfd34bf69..585c81a1e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Security implementation allowed an offline 2^80 brute force attack on the HMAC key of a single, uninterrupted connection (with no resumption of the session). + * Fix a buffer overread in ssl_parse_server_psk_hint() that could cause a + crash on invalid input. Features * Extend PKCS#8 interface by introducing support for the entire SHA @@ -44,6 +46,8 @@ Bugfix Nick Wilson on issue #355 * In test_suite_pk, pass valid parameters when testing for hash length overflow. #1179 + * Fix a possible arithmetic overflow in ssl_parse_server_psk_hint() that + could cause a key exchange to fail on valid data. Changes * Fix tag lengths and value ranges in the documentation of CCM encryption.