crypto_struct.h and such not self-contained #340

New issue

Closed

opened 2019-12-23 17:43:33 -06:00 by laurencelundblade · 2 comments

laurencelundblade commented

2019-12-23 17:43:33 -06:00

(Migrated from github.com)

Description

I know you don't want anyone to include crypto_struct.h directly, but it should still follow this rule: http://wiki.c2.com/?SelfContainedHeaders. That rule is also part of the TF-M coding guidelines.

My intuition is to include the smallest number of lines of headers possible which is why I thought to include crypto_struct.h and not crypto.h. From the compiler errors because crypto_struct.h is not self-contained, I learned quickly this is not the thing to do. Is this part of the plan to stop people from including it? :-)

I'm working on MacOS with Xcode with MBed 2.0, 1.1 and 1.1 integrated in with TF-M.

Issue request type

[ ] Question
[X] Enhancement
[ ] Bug

### Description I know you don't want anyone to include crypto_struct.h directly, but it should still follow this rule: http://wiki.c2.com/?SelfContainedHeaders. That rule is also part of the TF-M coding guidelines. My intuition is to include the smallest number of lines of headers possible which is why I thought to include crypto_struct.h and not crypto.h. From the compiler errors because crypto_struct.h is not self-contained, I learned quickly this is not the thing to do. Is this part of the plan to stop people from including it? :-) I'm working on MacOS with Xcode with MBed 2.0, 1.1 and 1.1 integrated in with TF-M.  ### Issue request type  [ ] Question [X] Enhancement [ ] Bug

ciarmcom commented

2019-12-24 00:44:51 -06:00

(Migrated from github.com)

Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-1035

*Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-1035*

gilles-peskine-arm commented

2020-03-11 04:50:01 -05:00

(Migrated from github.com)

The only reason why psa/crypto.h is divided into several headers is to accommodate builds in different environments that need to implement some things differently, and builds of code that shouldn't have everything.

crypto_types.h and crypto_values.h is separate because drivers of cryptographic hardware need the types and values, but not the API functions.
crypto_types.h and crypto_struct.h are separate because on implementations where the crypto library is in a separate environment that's protected from the application (e.g. a separate process, or a TrustZone secure world, etc.), the application side and the protected environment need different implementations, with a marshalling layer to do the translation (typically the application side has a numeric handle and the unmarshalling looks it up and passes a data pointer to the crypto library proper).
crypto_platform.h is for things that you might need to change if you port the library to a different platform.
crypto_extra.h and crypto_compat.h contain extensions to the standard PSA specification. I think we should have a build option to make them optional at some point so that you can ensure that your application doesn't rely on these extensions. But at this point they aren't optional and you can't include them separately.

We do follow the self-contained header rule as described here:

SelfContainedHeaders are header files that can be included top-most in a translation unit

crypto_struct.h cannot be included top-most in a translation unit. It's only there to be included by crypto.h.

From application code, include crypto.h only. The organization of the other headers can change at any time.

 * \note This file may not be included directly. Applications must
 * include psa/crypto.h.

We will not make crypto_struct.h self-contained since the only place where it should be included is crypto.h.

I can't even think of a good reason to look at crypto_struct.h in application code. This header is solely for implementation details that may change arbitrarily, essentially the struct definitions that gave the file its name. The only reason why these struct definitions are in a header that's included in application code is so that you can allocate these structures on the stack and use sizeof on them, otherwise we'd have made them private.

The only reason why `psa/crypto.h` is divided into several headers is to accommodate builds in different environments that need to implement some things differently, and builds of code that shouldn't have everything. * `crypto_types.h` and `crypto_values.h` is separate because drivers of cryptographic hardware need the types and values, but not the API functions. * `crypto_types.h` and `crypto_struct.h` are separate because on implementations where the crypto library is in a separate environment that's protected from the application (e.g. a separate process, or a TrustZone secure world, etc.), the application side and the protected environment need different implementations, with a marshalling layer to do the translation (typically the application side has a numeric handle and the unmarshalling looks it up and passes a data pointer to the crypto library proper). * `crypto_platform.h` is for things that you might need to change if you port the library to a different platform. * `crypto_extra.h` and `crypto_compat.h` contain extensions to the standard PSA specification. I think we should have a build option to make them optional at some point so that you can ensure that your application doesn't rely on these extensions. But at this point they aren't optional and you can't include them separately. We _do_ follow the self-contained header rule as described here: > SelfContainedHeaders are header files that can be included top-most in a translation unit `crypto_struct.h` cannot be included top-most in a translation unit. It's only there to be included by `crypto.h`. From application code, include `crypto.h` only. The organization of the other headers can change at any time. ``` * \note This file may not be included directly. Applications must * include psa/crypto.h. ``` We will not make `crypto_struct.h` self-contained since the only place where it should be included is `crypto.h`. I can't even think of a good reason to _look_ at `crypto_struct.h` in application code. This header is solely for implementation details that may change arbitrarily, essentially the struct definitions that gave the file its name. The only reason why these struct definitions are in a header that's included in application code is so that you can allocate these structures on the stack and use `sizeof` on them, otherwise we'd have made them private.