-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Remove discussions of MBEDTLS_USE_PSA_CRYPTO in standalone documentation #9818
base: development
Are you sure you want to change the base?
Remove discussions of MBEDTLS_USE_PSA_CRYPTO in standalone documentation #9818
Conversation
MBED_TLS_USE_PSA_CRYPTO is now always enabled we need to remove documentation discussing cases when it is disabled. Signed-off-by: Janos Follath <[email protected]>
MBED_TLS_USE_PSA_CRYPTO is now always enabled we need to remove documentation discussing cases when it is disabled. Signed-off-by: Janos Follath <[email protected]>
MBED_TLS_USE_PSA_CRYPTO is now always enabled we need to remove documentation discussing cases when it is disabled. Signed-off-by: Janos Follath <[email protected]>
This document is describes the testing strategy for the `MBEDTLS_USE_PSA_CRYPTO` option. This option is now always on, can't be disabled and the corresponding behaviour is the only library behaviour. Signed-off-by: Janos Follath <[email protected]>
MBED_TLS_USE_PSA_CRYPTO is now always enabled we need to remove documentation discussing cases when it is disabled. The goal is not to update the document, only to remove MBED_TLS_USE_PSA_CRYPTO, while making a minimal local context of the occurrance up to date and sensible. Signed-off-by: Janos Follath <[email protected]>
MBED_TLS_USE_PSA_CRYPTO is now always enabled we need to remove documentation discussing cases when it is disabled. Signed-off-by: Janos Follath <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly looks good to me. In particular, I don't see other files that need changing.
However, there are several places where we're left with a document that isn't meaningful any longer, or at least would need a major rewrite, because it opposes a legacy world and a PSA world, and this opposition no longer exists. I lean towards just removing these documents or passages.
There are a couple of places where the text wasn't quite right for 3.6, so before we forget, let's fix it: #9823.
@@ -3,7 +3,7 @@ PSA migration strategy for hashes and ciphers | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an architecture document focusing on how parts of the code base can accommodate both builds with PSA crypto disabled and builds with driver-only mechanisms. Going forward, this coexistence is no longer relevant. So I think we should just remove this file (like testing.md
).
The document does explain why some parts of md and cipher are the way they are. In the future, we'll want to remove legacy code paths and keep only the PSA code paths. But for that, it isn't particularly useful to know how the dual code paths came about, or what constraints they had to obey. Those constraints no longer apply.
docs/use-psa-crypto.md
Outdated
This document describes how PSA Crypto is used in the X.509 and TLS libraries | ||
from a user's perspective. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this document still relevant? I'm on the fence. Should we keep updating it gradually or just remove it now?
The original purpose of this document was to answer the following question in more detail than the documentation of MBEDTLS_USE_PSA_CRYPTO
in config.h
: as a user, what do I gain and lose if I enable MBEDTLS_USE_PSA_CRYPTO
? This question is no longer relevant.
In principle, everything that's relevant from a user's perspective should be listed in the API documentation (attached to the relevant configuration option or library function). And we have the maintainer's view in docs/architecture/psa-migration/psa-limitations.md
. Is it still useful to have this user-facing document? At this point, it looks to me like a loose collection of facts. But maybe this loose collection is still somewhat useful. Perhaps it should become a chapter in psa-transition.md
?
If we do keep this document, there are a few parts that need further work that is technically in scope here, and some where the further work will technically be in scope when we remove the legacy crypto functions from the API. But it's probably easier to do all the changes at once, so we could stop here for now and file an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on what you are saying and looking at the documents involved in detail, I think it is probably best to remove it:
- General considerations section: not relevant as you mentioned above
- New APIs/API extensions: these are not new or extensions anymore.
Also, for detailed information this section refers to the API
documentation, which contains all the information the user needs. - Internal changes: these are discussed in detail in docs/architecture/psa-migration/psa-limitations.md.
docs/use-psa-crypto.md
Outdated
operations and its public part can be exported. | ||
**API function:** `mbedtls_pk_setup_opaque()` - can be used to wrap a PSA key | ||
pair into a PK context. The key can be used for private-key operations and its | ||
public part can be exported. | ||
|
||
**Benefits:** isolation of long-term secrets, use of PSA Crypto drivers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this document from the perspective of a new user, rather than someone migrating from non-PSA APIs:
The text opposes two scenarios, but what are they? “Benefits” compared to what? Below, “opt-in”, “new API”: as opposed to what?
This is an architecture document focusing on how parts of the code base can accommodate both builds with PSA crypto disabled and builds with driver-only mechanisms. Going forward, this coexistence is no longer relevant. The document does explain why some parts of md and cipher are the way they are. In the future, we'll want to remove legacy code paths and keep only the PSA code paths. But for that, it isn't particularly useful to know how the dual code paths came about, or what constraints they had to obey. Those constraints no longer apply. Signed-off-by: Janos Follath <[email protected]>
This is an architecture document focusing on how PSA APIs can be mixed with non-PSA APIs, notably including PK (and in fact, it's mostly about PK, since we didn't identify work to be done in other areas). It is not really relevant in 4.0/1.0, where the goals will be different — to do without low-level legacy APIs. Signed-off-by: Janos Follath <[email protected]>
The original purpose of this document was to answer the following question in more detail than the documentation of MBEDTLS_USE_PSA_CRYPTO in config.h: as a user, what do I gain and lose if I enable MBEDTLS_USE_PSA_CRYPTO? This question is no longer relevant. - General considerations section: not relevant as mentioned above - New APIs/API extensions: these are not new or extensions anymore. Also, for detailed information this section refers to the API documentation, which contains all the information the user needs. - Internal changes: these are discussed in detail in docs/architecture/psa-migration/psa-limitations.md. Signed-off-by: Janos Follath <[email protected]>
Some sentences or paragraphs became confusing or meaningless after removing USE_PSA and only fixing the local context/semantics. Fix the semantics where needed and remove parts that became meaningless. Signed-off-by: Janos Follath <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except that there are now dangling references to the newly deleted documents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still references to use-psa-crypto.md
in docs/architecture/psa-migration/strategy.md
and docs/driver-only-builds.md
.
As of Mbed TLS 3.2, most of (G1) and all of (G2) is implemented. For a more | ||
detailed account of what's implemented, see `docs/use-psa-crypto.md`, where new | ||
APIs are about (G2), and internal changes implement (G1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/use-psa-crypto.md
is now being deleted. We are effectively finalizing the transition from “all is legacy-only except what MBEDTLS_USE_PSA_CRYPTO
supports” to “all is PSA except for a few limitations”. So now I think the relevant reference is docs/architecture/psa-migration/psa-limitations.md
for how we've only achieved most of (G1) and not all.
@@ -215,8 +200,7 @@ consequence these are not supported in builds without `MBEDTLS_ECDSA_C`. | |||
|
|||
Similarly, there is no PSA support for interruptible ECDH operations so these | |||
are not supported without `ECDH_C`. See also limitations regarding | |||
restartable operations with `MBEDTLS_USE_PSA_CRYPTO` in [its | |||
documentation](use-psa-crypto.md). | |||
restartable operations in [the documentation about using PSA Crypto](use-psa-crypto.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With use-psa-crypto.md
deleted: In the documentation of MBEDTLS_ECP_RESTARTABLE
, or in docs/architecture/psa-migration/psa-limitations.md
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still references to md-cipher-dispatch.md
in docs/architecture/psa-migration/strategy.md
. I wonder if strategy.md
itself should be deleted? It's mainly about a plan that was driven by backward compatibility constraints that no longer exist in 4.0, although some of it is still of interest to explain why 4.0 is the way it is (and more complicated than one would expect based on the APIs that are left in 4.0). It isn't not referenced from anywhere except one comment in docs/architecture/psa-migration/outcome-analysis.sh
.
Description
Remove discussions of MBEDTLS_USE_PSA_CRYPTO in API documentation. Resolves partially #9632.
PR checklist
Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.