Public / private distinction in crypto.h, crypto_values.h, crypto_struct.h #339

New issue

Open

opened 2019-12-23 17:32:11 -06:00 by laurencelundblade · 2 comments

laurencelundblade commented

2019-12-23 17:32:11 -06:00

(Migrated from github.com)

Description

It seems clear that everything in crypto.h is public interface and can be depended on when using the PSA Crypto API, but it is not clear for the other files like crypto_values.h and crypto_struct.h.

For example it seems like most of crypto_values.h is public which makes you think the same is true for crypto_struct.h, but it's not. The internals of struct psa_hash_operation_s are clearly private and depend on how MBed works. It is different in the MBed in TF-M than it is in the source in this repository (and that seems perfectly OK).

Since C doesn't allow you to hide stuff in headers, the way to address this is to clearly label stuff that is private in a comment. Here's an example:

struct _QCBOREncodeContext {
   // PRIVATE DATA STRUCTURE
   UsefulOutBuf      OutBuf;  // Pointer to output buffer, its length and
                              // position in it
   uint8_t           uError;  // Error state, always from QCBORError enum
   QCBORTrackNesting nesting; // Keep track of array and map nesting
};

I noticed this when I started to write code against the internal of crypto_struct.h and then saw the difference between this repository and TF-M.

Issue request type

[ ] Question
[X] Enhancement
[ ] Bug

### Description It seems clear that everything in crypto.h is public interface and can be depended on when using the PSA Crypto API, but it is not clear for the other files like crypto_values.h and crypto_struct.h. For example it seems like most of crypto_values.h is public which makes you think the same is true for crypto_struct.h, but it's not. The internals of `struct psa_hash_operation_s` are clearly private and depend on how MBed works. It is different in the MBed in TF-M than it is in the source in this repository (and that seems perfectly OK). Since C doesn't allow you to hide stuff in headers, the way to address this is to clearly label stuff that is private in a comment. Here's an example: ``` struct _QCBOREncodeContext { // PRIVATE DATA STRUCTURE UsefulOutBuf OutBuf; // Pointer to output buffer, its length and // position in it uint8_t uError; // Error state, always from QCBORError enum QCBORTrackNesting nesting; // Keep track of array and map nesting }; ``` I noticed this when I started to write code against the internal of crypto_struct.h and then saw the difference between this repository and TF-M.  ### Issue request type  [ ] Question [X] Enhancement [ ] Bug

ciarmcom commented

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

(Migrated from github.com)

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

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

gilles-peskine-arm commented

2020-03-11 04:52:56 -05:00

(Migrated from github.com)

I'm puzzled by what you're asking here. There's a comment at the top of the file that states that everything in that file is implementation-specific. Do you want us to repeat this comment on every single definition in the file?

I'm puzzled by what you're asking here. There's a comment at the top of the file that states that _everything_ in that file is implementation-specific. Do you want us to repeat this comment on every single definition in the file?