Skip to content

Remove redundant declarations and update docs #727

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.

Sign up for GitHub

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

Draft
bjwtaylor wants to merge 2 commits into
base: development
Choose a base branch
from
Draft

Remove redundant declarations and update docs #727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb9421c
Add Guards to Doc only functions
Mar 26, 2026
48d6411
Add ChangeLog
Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ChangeLog.d/add-doxygen-only-to-crypto.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
API changes
* Change crypto.h so the functions duplicated in crypto_structs are internal
only.
40 changes: 40 additions & 0 deletions include/psa/crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ psa_status_t psa_crypto_init(void);
* @{
*/

#ifdef __DOXYGEN_ONLY__
Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm Apr 3, 2026

Choose a reason for hiding this comment

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

Marking some function prototypes as Doxygen-only doesn't make sense. The reason to have prototypes is so that the C compiler sees them.

The reason some static inline functions have a prototype in crypto.h and a definition in crypto_struct.h is that the prototypes are the same for everyone, but the function definitions are not. By default, TF-PSA-Crypto presents itself a library. But it can also be integrated as a client and server, in which case the integrator makes some changes:

  • On the client side:
    • Define MBEDTLS_PSA_CRYPTO_CLIENT but not MBEDTLS_PSA_CRYPTO_C.
    • Edit crypto_struct.h to change the definitions of operation structures, typically to have an operation handle instead of cryptographic material.
    • Provide implementations of the PSA API functions (excluding the static inline functions that manipulate a local struct) that makes remote procedure calls to the server.
  • On the server side:
    • Probably define MBEDTLS_PSA_CRYPTO_SPM and likely MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER.
    • Implement the server side of the remote procedure calls.

While the definitions of the struct constructors in crypto_struct.h are customizable, their prototypes are not. Hence the prototypes live in crypto.h. And the prototypes must be visible to the C compiler.

Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm Apr 3, 2026

Choose a reason for hiding this comment

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

That being said, is the current code really what it should be? No, there are two things that I never got around to fixing.

  • psa_key_attributes_t is not supposed to be user-serviceable, unlike operation structures. So it should be in crypto_types.h, and its inline accessor functions should not be defined in crypto_struct.h.
  • The definitions of the struct constructor functions (psa_xxx_init()) don't actually need to be in crypto_struct.h, because their textual definition is actually the same for everyone. They're defined in terms of the corresponding PSA_XXX_INIT macro. Only the macro needs to be customized if you customize the struct layout.

We could fix this. It would be an incompatible change for integrators who customize crypto_struct.h. We don't explicitly promise any backward compatibility for such custom integrations, so I think it would be ok to do what I just wrote in the development branch. But I don't think we should do it in an LTS branch unless there's a compelling reason, and I don't see a compelling reason.

Copy link
Copy Markdown
Contributor Author

@bjwtaylor bjwtaylor Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

OK, thanks for your comments. Let's put this one on hold pending design review of the original issue. There is also a related PR here: Mbed-TLS/mbedtls#10659. It's the same issue, but not the same solution. Let me know what you think about this one, maybe we just decide we don't want to fix these warnings?

/** \def PSA_KEY_ATTRIBUTES_INIT
*
* This macro returns a suitable initializer for a key attribute structure
Expand All @@ -120,7 +121,9 @@ psa_status_t psa_crypto_init(void);
/** Return an initial value for a key attributes structure.
*/
static psa_key_attributes_t psa_key_attributes_init(void);
#endif

#ifdef __DOXYGEN_ONLY__
/** Declare a key as persistent and set its key identifier.
*
* If the attribute structure currently declares the key as volatile (which
Expand All @@ -147,8 +150,10 @@ static psa_key_attributes_t psa_key_attributes_init(void);
*/
static void psa_set_key_id(psa_key_attributes_t *attributes,
mbedtls_svc_key_id_t key);
#endif

#ifdef MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER
#if defined(__DOXYGEN_ONLY__)
/** Set the owner identifier of a key.
*
* When key identifiers encode key owner identifiers, psa_set_key_id() does
Expand All @@ -166,7 +171,9 @@ static void psa_set_key_id(psa_key_attributes_t *attributes,
static void mbedtls_set_key_owner_id(psa_key_attributes_t *attributes,
mbedtls_key_owner_id_t owner);
#endif
#endif

#ifdef __DOXYGEN_ONLY__
/** Set the location of a persistent key.
*
* To make a key persistent, you must give it a persistent key identifier
Expand Down Expand Up @@ -195,7 +202,9 @@ static void mbedtls_set_key_owner_id(psa_key_attributes_t *attributes,
*/
static void psa_set_key_lifetime(psa_key_attributes_t *attributes,
psa_key_lifetime_t lifetime);
#endif

#ifdef __DOXYGEN_ONLY__
/** Retrieve the key identifier from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -210,7 +219,9 @@ static void psa_set_key_lifetime(psa_key_attributes_t *attributes,
*/
static mbedtls_svc_key_id_t psa_get_key_id(
const psa_key_attributes_t *attributes);
#endif

#ifdef __DOXYGEN_ONLY__
/** Retrieve the lifetime from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -223,7 +234,9 @@ static mbedtls_svc_key_id_t psa_get_key_id(
*/
static psa_key_lifetime_t psa_get_key_lifetime(
const psa_key_attributes_t *attributes);
#endif

#ifdef __DOXYGEN_ONLY__
/** Declare usage flags for a key.
*
* Usage flags are part of a key's usage policy. They encode what
Expand All @@ -242,7 +255,9 @@ static psa_key_lifetime_t psa_get_key_lifetime(
*/
static void psa_set_key_usage_flags(psa_key_attributes_t *attributes,
psa_key_usage_t usage_flags);
#endif

#ifdef __DOXYGEN_ONLY__
/** Retrieve the usage flags from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -255,7 +270,9 @@ static void psa_set_key_usage_flags(psa_key_attributes_t *attributes,
*/
static psa_key_usage_t psa_get_key_usage_flags(
const psa_key_attributes_t *attributes);
#endif

#ifdef __DOXYGEN_ONLY__
/** Declare the permitted algorithm policy for a key.
*
* The permitted algorithm policy of a key encodes which algorithm or
Expand Down Expand Up @@ -288,8 +305,10 @@ static psa_key_usage_t psa_get_key_usage_flags(
*/
static void psa_set_key_algorithm(psa_key_attributes_t *attributes,
psa_algorithm_t alg);
#endif


#ifdef __DOXYGEN_ONLY__
/** Retrieve the algorithm policy from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -302,7 +321,9 @@ static void psa_set_key_algorithm(psa_key_attributes_t *attributes,
*/
static psa_algorithm_t psa_get_key_algorithm(
const psa_key_attributes_t *attributes);
#endif

#ifdef __DOXYGEN_ONLY__
/** Declare the type of a key.
*
* This function overwrites any key type
Expand All @@ -319,8 +340,10 @@ static psa_algorithm_t psa_get_key_algorithm(
*/
static void psa_set_key_type(psa_key_attributes_t *attributes,
psa_key_type_t type);
#endif


#ifdef __DOXYGEN_ONLY__
/** Declare the size of a key.
*
* This function overwrites any key size previously set in \p attributes.
Expand All @@ -337,7 +360,9 @@ static void psa_set_key_type(psa_key_attributes_t *attributes,
*/
static void psa_set_key_bits(psa_key_attributes_t *attributes,
size_t bits);
#endif

#ifdef __DOXYGEN_ONLY__
/** Retrieve the key type from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -349,7 +374,9 @@ static void psa_set_key_bits(psa_key_attributes_t *attributes,
* \return The key type stored in the attribute structure.
*/
static psa_key_type_t psa_get_key_type(const psa_key_attributes_t *attributes);
#endif

#ifdef __DOXYGEN_ONLY__
/** Retrieve the key size from key attributes.
*
* This function may be declared as `static` (i.e. without external
Expand All @@ -361,6 +388,7 @@ static psa_key_type_t psa_get_key_type(const psa_key_attributes_t *attributes);
* \return The key size stored in the attribute structure, in bits.
*/
static size_t psa_get_key_bits(const psa_key_attributes_t *attributes);
#endif

/** Retrieve the attributes of a key.
*
Expand Down Expand Up @@ -939,6 +967,7 @@ psa_status_t psa_hash_compare(psa_algorithm_t alg,
* Implementation details can change in future versions without notice. */
typedef struct psa_hash_operation_s psa_hash_operation_t;

#ifdef __DOXYGEN_ONLY__
/** \def PSA_HASH_OPERATION_INIT
*
* This macro returns a suitable initializer for a hash operation object
Expand All @@ -948,6 +977,7 @@ typedef struct psa_hash_operation_s psa_hash_operation_t;
/** Return an initial value for a hash operation object.
*/
static psa_hash_operation_t psa_hash_operation_init(void);
#endif

/** Set up a multipart hash operation.
*
Expand Down Expand Up @@ -1213,9 +1243,11 @@ typedef struct psa_xof_operation_s psa_xof_operation_t;
* of type #psa_xof_operation_t.
*/

#ifdef __DOXYGEN_ONLY__
/** Return an initial value for a XOF operation object.
*/
static psa_xof_operation_t psa_xof_operation_init(void);
#endif

/** Set up a multipart XOF (extendable-operation function) operation.
*
Expand Down Expand Up @@ -1493,9 +1525,11 @@ typedef struct psa_mac_operation_s psa_mac_operation_t;
* #psa_mac_operation_t.
*/

#ifdef __DOXYGEN_ONLY__
/** Return an initial value for a MAC operation object.
*/
static psa_mac_operation_t psa_mac_operation_init(void);
#endif

/** Set up a multipart MAC calculation operation.
*
Expand Down Expand Up @@ -1906,9 +1940,11 @@ typedef struct psa_cipher_operation_s psa_cipher_operation_t;
* type #psa_cipher_operation_t.
*/

#ifdef __DOXYGEN_ONLY__
/** Return an initial value for a cipher operation object.
*/
static psa_cipher_operation_t psa_cipher_operation_init(void);
#endif

/** Set the key for a multipart symmetric encryption operation.
*
Expand Down Expand Up @@ -2424,9 +2460,11 @@ typedef struct psa_aead_operation_s psa_aead_operation_t;
* type #psa_aead_operation_t.
*/

#ifdef __DOXYGEN_ONLY__
/** Return an initial value for an AEAD operation object.
*/
static psa_aead_operation_t psa_aead_operation_init(void);
#endif

/** Set the key for a multipart authenticated encryption operation.
*
Expand Down Expand Up @@ -3411,9 +3449,11 @@ typedef struct psa_key_derivation_s psa_key_derivation_operation_t;
* object of type #psa_key_derivation_operation_t.
*/

#ifdef __DOXYGEN_ONLY__
/** Return an initial value for a key derivation operation object.
*/
static psa_key_derivation_operation_t psa_key_derivation_operation_init(void);
#endif

/** Set up a key derivation operation.
*
Expand Down