gh-ARMmbed/mbed-crypto
Watch
1
0
Fork
You've already forked mbed-crypto
 0
Code Issues 36 Pull requests 1 Releases 7 Activity

API: minor corrections after beta 2 #83

Merged
gilles-peskine-arm merged 7 commits from psa-api-1.0-beta-beta2_minor_fixes into psa-api-1.0-beta 2019-03-12 08:33:23 -05:00
Conversation 11 Commits 7 Files changed 3 +126 -64
gilles-peskine-arm commented 2019-03-11 09:14:08 -05:00 (Migrated from github.com)
  • Minor corrections noticed after beta 2 was finalized.
  • Specify psa_generator_import_key for most key types.
* Minor corrections noticed after beta 2 was finalized. * Specify `psa_generator_import_key` for most key types.
yanesca (Migrated from github.com) reviewed 2019-03-11 09:39:54 -05:00
include/psa/crypto.h
@ -2894,0 +2901,4 @@
* - #PSA_KEY_TYPE_DERIVE;
* - #PSA_KEY_TYPE_HMAC.
*
* - For ECC keys on a Montgomery elliptic curve
yanesca (Migrated from github.com) commented 2019-03-11 09:38:03 -05:00

It looks like this should be (bits +7) / 8 bytes.

It looks like this should be `(bits +7) / 8` bytes.
gilles-peskine-arm (Migrated from github.com) reviewed 2019-03-11 11:34:46 -05:00
include/psa/crypto.h
@ -2894,0 +2901,4 @@
* - #PSA_KEY_TYPE_DERIVE;
* - #PSA_KEY_TYPE_HMAC.
*
* - For ECC keys on a Montgomery elliptic curve
gilles-peskine-arm (Migrated from github.com) commented 2019-03-11 11:34:45 -05:00

Pff, bits * 8, bits / 8…

It's actually bits / 8 and not (bits + 7) / 8 since bits is a multiple of 8 for symmetric keys.

But this function should also be specified for other key types. Implementations don't have to support every key type, but if they do then they should follow a standard. I've rewritten the specification of this function to define the behavior for every standard key type other than RSA.

Pff, `bits * 8`, `bits / 8`… It's actually `bits / 8` and not `(bits + 7) / 8` since `bits` is a multiple of 8 for symmetric keys. But this function should also be specified for other key types. Implementations don't have to support every key type, but if they do then they should follow a standard. I've rewritten the specification of this function to define the behavior for every standard key type other than RSA.
yanesca (Migrated from github.com) reviewed 2019-03-11 11:55:12 -05:00
yanesca (Migrated from github.com) left a comment
No description provided.
Looks good to me.
gilles-peskine-arm (Migrated from github.com) reviewed 2019-03-11 12:04:19 -05:00
include/psa/crypto.h
@ -2894,0 +2901,4 @@
* - #PSA_KEY_TYPE_DERIVE;
* - #PSA_KEY_TYPE_HMAC.
*
* - For ECC keys on a Montgomery elliptic curve
gilles-peskine-arm (Migrated from github.com) commented 2019-03-11 12:04:19 -05:00

I pushed another commit to make the specification unambiguous for DH, DSA and ECC (Weierstrass). I chose a method that is codified by NIST, so that it's possible to be FIPS-compliant when implementing this function.

I pushed another commit to make the specification unambiguous for DH, DSA and ECC (Weierstrass). I chose a method that is codified by NIST, so that it's possible to be FIPS-compliant when implementing this function.
yanesca (Migrated from github.com) reviewed 2019-03-12 05:14:29 -05:00
include/psa/crypto.h
@ -2894,0 +2921,4 @@
* The byte string drawn from the generator is interpreted as specified
* for the output produced by psa_export_key().
* The following key types defined in this specification follow this scheme:
*
yanesca (Migrated from github.com) commented 2019-03-12 05:03:48 -05:00

The method described in the standard is more relaxed in some cases (for example when generating DH keys the standard allows going for shorter private keys than the size of the prime). I think writing something like "This method allows for compliance with NIST and FIPS standards, see:" would be more accurate.

The method described in the standard is more relaxed in some cases (for example when generating DH keys the standard allows going for shorter private keys than the size of the prime). I think writing something like "This method allows for compliance with NIST and FIPS standards, see:" would be more accurate.
yanesca (Migrated from github.com) commented 2019-03-12 05:14:16 -05:00

In case of ECC the boundary is the order of the curve's base point (here I use the term curve in the sense of the set of cryptographic domain parameters and not as a mathematical object, but I assume that is what we do throughout this documentation).

In case of ECC the boundary is the order of the curve's base point (here I use the term curve in the sense of the set of cryptographic domain parameters and not as a mathematical object, but I assume that is what we do throughout this documentation).
gilles-peskine-arm (Migrated from github.com) reviewed 2019-03-12 05:50:34 -05:00
include/psa/crypto.h
@ -2894,0 +2921,4 @@
* The byte string drawn from the generator is interpreted as specified
* for the output produced by psa_export_key().
* The following key types defined in this specification follow this scheme:
*
gilles-peskine-arm (Migrated from github.com) commented 2019-03-12 05:50:34 -05:00

I tweaked the wording.

I tweaked the wording.
gilles-peskine-arm (Migrated from github.com) reviewed 2019-03-12 05:50:46 -05:00
include/psa/crypto.h
@ -2894,0 +2921,4 @@
* The byte string drawn from the generator is interpreted as specified
* for the output produced by psa_export_key().
* The following key types defined in this specification follow this scheme:
*
gilles-peskine-arm (Migrated from github.com) commented 2019-03-12 05:50:46 -05:00

Indeed. Fixed, thanks.

Indeed. Fixed, thanks.
yanesca (Migrated from github.com) reviewed 2019-03-12 05:52:24 -05:00
include/psa/crypto.h
@ -2894,0 +2938,4 @@
* For these key types, interpret the byte string as integer
* in big-endian order. Discard it if it is not in the range
* [0, *N* - 2] where *N* is the boundary of the private key domain
* (the prime *p* for Diffie-Hellman, the subprime *q* for DSA,
yanesca (Migrated from github.com) commented 2019-03-12 05:52:24 -05:00

Typo: allow -> allows

Typo: allow -> allows
gilles-peskine-arm (Migrated from github.com) reviewed 2019-03-12 05:54:51 -05:00
include/psa/crypto.h
@ -2894,0 +2938,4 @@
* For these key types, interpret the byte string as integer
* in big-endian order. Discard it if it is not in the range
* [0, *N* - 2] where *N* is the boundary of the private key domain
* (the prime *p* for Diffie-Hellman, the subprime *q* for DSA,
gilles-peskine-arm (Migrated from github.com) commented 2019-03-12 05:54:51 -05:00

Amended

Amended
yanesca (Migrated from github.com) reviewed 2019-03-12 05:55:55 -05:00
yanesca (Migrated from github.com) left a comment
No description provided.
Looks good to me.
yanesca (Migrated from github.com) approved these changes 2019-03-12 08:22:24 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
yanesca
Labels
Clear labels   
api-spec
Issue or PR about the PSA specifications

  
bug
Something isn't working

  
CLA requested

   
CLA valid

   
compliance
Discrepancy between the library and the PSA Crypto API specification

  
DO NOT MERGE
The PR is not intended to be merged (yet). Usually used for a review of worked in progress.

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
feature branch
This issue or PR is for a short-term feature branch which doesn't have its own label.

  
good first issue
Good for newcomers

  
HEADER FORMATTING ISSUE

   
help wanted
Extra attention is needed

  
Information

   
invalid
This doesn't seem right

  
Jira status: AWAITING REVIEW

   
Jira status: CLOSED

   
Jira status: DONE

   
Jira status: IN PROGRESS

   
Jira status: IN REVIEW

   
Jira status: OPEN

   
Jira status: REOPENED

   
Jira status: RESOLVED

   
mirrored

   
mirror internally

   
needs: backports
Needs backports to Mbed TLS branches

  
needs: ci
Needs a passing full CI run

  
needs: design review

   
needs: preceding PR
Requires another PR to be merged first

  
needs: review
The pull request is ready for review. This generally means that it has no known issues.

  
needs: work
The pull request needs rework before it can be merged.

  
notabug
The bug is not considered to be a defect, but by design.

  
open for community contribution

   
Product Backlog

   
question
Further information is requested

  
ready for merge
Design and code approved, CI passed, and likewise for backports. Label added by gatekeepers only.

  
security
Security expertise requested in review

  
stale
This PR is not being actively worked on. It can be considered abandoned work.

  
type: bug

   
type: enhancement

   
type: question

   
web pages
The /docs directory in the master branch is shown on https://armmbed.github.io/mbed-crypto

  
wontfix
The cost of fixing the bug or managing it in the bug list is not justified.

  
writing
Documentation wording and formatting

No labels
api-spec
bug
CLA requested
CLA valid
compliance
DO NOT MERGE
duplicate
enhancement
feature branch
good first issue
HEADER FORMATTING ISSUE
help wanted
Information
invalid
Jira status: AWAITING REVIEW
Jira status: CLOSED
Jira status: DONE
Jira status: IN PROGRESS
Jira status: IN REVIEW
Jira status: OPEN
Jira status: REOPENED
Jira status: RESOLVED
mirrored
mirror internally
needs: backports
needs: ci
needs: design review
needs: preceding PR
needs: review
needs: work
notabug
open for community contribution
Product Backlog
question
ready for merge
security
stale
type: bug
type: enhancement
type: question
web pages
wontfix
writing
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-ARMmbed/mbed-crypto!83
No description provided.