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

License change #8462

Merged
merged 10 commits into from
Nov 4, 2023
Merged

Conversation

daverodgman
Copy link
Contributor

@daverodgman daverodgman commented Nov 2, 2023

Description

Update to dual license

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

@daverodgman daverodgman added needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests component-docs Docs / website issues filed here for tracking needs-reviewer This PR needs someone to pick it up for review priority-very-high Highest priority - prioritise this over other review work size-xs Estimated task size: extra small (a few hours at most) labels Nov 2, 2023
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

The license combination isn't expressed correctly.

.uncrustify.cfg Outdated
@@ -17,6 +18,19 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
#
# This program is free software; you can redistribute it and/or modify it under the terms of the
Copy link
Contributor

Choose a reason for hiding this comment

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

The way the license snippets are concatenated together, it looks like you have to obey both licenses to use the program. This is genuinely unclear, and creates a legal risk for users.

Having only the SPDX license identifier and none of the rest of the boilerplate would be unambiguous. Do we have a requirement to include the rest of the boilerplate in each file?

Do we have some corporate guidance on how to write a dual license? Failing that, does a reputable FOSS-related organization offer such guidance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check this. I agree it's a bit confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OSO guidance is: The recommended license notice is the [SPDX License Identifier](https://spdx.org/licenses/). It is also acceptable to include any short license notice as defined by the license itself. Apache 2.0 and the GPL family are examples of licenses that define their own short license notice.

So having only the SPDX license identifier would be acceptable, and I think better for readability & unambiguity.

CONTRIBUTING.md Outdated

Contributors must accept that their contributions are made under both the Apache-2.0 AND [GPL-2.0-or-later](https://spdx.org/licenses/GPL-2.0-or-later.html) licenses. This enables LTS (Long Term Support) branches of the software to be provided under either the Apache-2.0 or GPL-2.0-or-later licenses.

All new files should include the [Apache-2.0](https://spdx.org/licenses/Apache-2.0.html) standard license header where possible.
All new files should include the [Apache-2.0](https://spdx.org/licenses/Apache-2.0.html) OR [GPL-2.0-or-later](https://spdx.org/licenses/GPL-2.0-or-later.html) standard license header where possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

All files need to mention both licenses or none. If a file only mentions one of the licenses, then that file may only be redistributed under that one license.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, wording is unclear here.

@@ -200,3 +200,347 @@
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

Copy link
Contributor

Choose a reason for hiding this comment

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

There should probably be a bit of text at the top of LICENSE that states that this work is distributed under either license at the recipient's option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the LICENSE file simply contains the license text(s); it does not describe how the software is licensed. Although I agree this would be a useful improvement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we split into two files, LICENSE.apache2.0 and LICENSE.gpl2?

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Nov 2, 2023
Signed-off-by: Dave Rodgman <[email protected]>
Signed-off-by: Dave Rodgman <[email protected]>
Signed-off-by: Dave Rodgman <[email protected]>
@daverodgman daverodgman added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-work labels Nov 2, 2023
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

I have checked that, except as mentioned in my change requests:

  • The license is now clearly conveyed.
  • The SPDX-License-Identifier is correct where specified, and no such was removed.
  • That there are no more mentions of the Apache or GPL license in individual source files, except via the SPDX-License-Identifier.
  • That the commit 16799db "update headers" only makes changes to license blurbs, without removing the copyright statement, and updating the SPDX license identifier.

programs/x509/load_roots.c Outdated Show resolved Hide resolved
library/ssl_tls13_keys.c Outdated Show resolved Hide resolved
library/ssl_tls13_keys.h Outdated Show resolved Hide resolved
@@ -84,11 +84,11 @@ Mbed TLS is well documented, but if you think documentation is needed, speak out
License and Copyright
---------------------

Unless specifically indicated otherwise in a file, Mbed TLS files are provided under the [Apache-2.0](https://spdx.org/licenses/Apache-2.0.html) license. See the [LICENSE](LICENSE) file for the full text of this license.
Unless specifically indicated otherwise in a file, Mbed TLS files are provided under a dual [Apache-2.0](https://spdx.org/licenses/Apache-2.0.html) OR [GPL-2.0-or-later](https://spdx.org/licenses/GPL-2.0-or-later.html) license. See the [LICENSE](LICENSE) file for the full text of these licenses. This means that users may choose which of these licenses they take the code under.
Copy link
Contributor

Choose a reason for hiding this comment

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

We are now releasing everything under a dual license except Everest and p256-m. I assume this is deliberate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For p256-m I wanted to check with @mpg first. For Everest, I don't know the situation. Are we permitted to release under GPL?

Copy link
Contributor

Choose a reason for hiding this comment

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

Everest was contributed through #2073 (eventually merged via ARMmbed/mbed-crypto#140). The Everest files have always had Apache-only header files. However, to the best of my knowledge, the contributor had signed the Contributor License Agreement (CLA) which allowed Arm to distribute the contribution under any other license at Arm's choice. The fact that there was no discussion of the license during the review is not particularly an indication that there was an agreement that Everest would be Apache-only: given our practice at the time, we may have just assumed that the overall license would apply. If there was any discussion between Arm and Microsoft about the license, I either wasn't in on it or have completely forgotten. We would need to dig through legal documents to figure it out for sure though.

In any case, it's not particularly surprising to recipients that the content of a 3rdparty repository might have a different license from the rest of the project, and everything under 3rdparty is optional, so this isn't critical.

Copy link
Contributor

Choose a reason for hiding this comment

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

For p256-m, I'm more than happy to relicense to Apache-or-GPL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mpg, I'll update p256-m.

For Everest: we have never distributed it as GPL, so there's no precedent, and it is marked as Apache 2.0. It is definitely quite possible that the authors did not realise/intend for it to be under a GPL license. On that basis I don't feel comfortable putting it under the dual license without consulting the authors. I will leave it as Apache 2.0 unless the situation changes. @wintersteiger - we would be grateful for your input here.

Signed-off-by: Dave Rodgman <[email protected]>
@daverodgman daverodgman force-pushed the license-change branch 2 times, most recently from 378bd8b to fac3954 Compare November 3, 2023 09:30
Signed-off-by: Dave Rodgman <[email protected]>
@daverodgman daverodgman removed needs-work needs-reviewer This PR needs someone to pick it up for review labels Nov 3, 2023
@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Nov 3, 2023
@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Nov 3, 2023
Signed-off-by: Dave Rodgman <[email protected]>
@daverodgman
Copy link
Contributor Author

Sorry for the churn, I spotted a few non-standard bits in some more headers.

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@daverodgman daverodgman added needs-review Every commit must be reviewed by at least two team members, and removed approved Design and code approved - may be waiting for CI or backports labels Nov 3, 2023
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Nov 3, 2023
Signed-off-by: Dave Rodgman <[email protected]>
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm mentioned this pull request Nov 3, 2023
3 tasks
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Nov 3, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 4, 2023
@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label Nov 4, 2023
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Nov 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 4, 2023
@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Nov 4, 2023

This pull request has passed the PR tests on the merge queue but the merge queue failed due to a misconfiguration. I'm going ahead and merging immediately.

@gilles-peskine-arm gilles-peskine-arm merged commit 0c29963 into Mbed-TLS:development Nov 4, 2023
1 check passed
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 component-docs Docs / website issues filed here for tracking priority-very-high Highest priority - prioritise this over other review work size-xs Estimated task size: extra small (a few hours at most)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants