crypto: Add Crypto API documentation #983

Patater · 2019-02-28T15:59:39Z

Add a Crypto API page which gives an overview of Mbed Crypto, links to
the PSA Crypto API specification, links to the Crypto repo and its
documents, and explains a bit about how Mbed Crypto is integrated with
Mbed OS for boards with secure environments.

dgreen-arm

I'm not a docs person, but I think I found two grammar issues. Otherwise, looks good to me.

dgreen-arm · 2019-02-28T16:33:59Z

docs/api/security/crypto.md

@@ -0,0 +1,49 @@
+## Crypto
+
+Arm Mbed Crypto is reference implementation of the cryptography interface of


Adding 'the' between 'is' and 'reference' will make the sentence flow better.

Thanks your review.

dgreen-arm · 2019-02-28T16:35:43Z

docs/api/security/crypto.md

+We have adapted and [integrated Mbed Crypto with Mbed
+OS](https://github.com/ARMmbed/mbed-os/blob/master/features/mbedtls/mbedcrypto).
+On PSA platforms that support it, Mbed Crypto is pre-integrated with Mbed OS to
+take advantage of a platform's segmented architecture and isolate cryptographic


I think 'a' should be 'the' here.

Patater · 2019-02-28T16:44:40Z

Rebased to improve grammars.

AnotherButler · 2019-02-28T22:51:20Z

Does this have a code dependency?

AnotherButler · 2019-02-28T22:52:36Z

I don't have access to edit this, even though it's in the docs repo, so I'm going to leave my changes in the form of comments today. If possible, please give me access.

AnotherButler

Please address my comments and queries.

AnotherButler · 2019-02-28T22:53:25Z

docs/api/security/crypto.md

+
+### Other resources
+
+The [Mbed Crypto website](https://github.com/ARMmbed/mbed-crypto) contains [an


Will this website be live by 5.12? If not, let's not call it a website.

AnotherButler · 2019-02-28T22:55:27Z

docs/api/security/crypto.md

+
+The [Mbed Crypto website](https://github.com/ARMmbed/mbed-crypto) contains [an
+overview of the PSA Crypto
+API](https://github.com/ARMmbed/mbed-crypto/blob/psa-api-1.0-beta/docs/PSA_Crypto_API_Overview.pdf),


Please put these in bullet form instead of a list, to match our other pages.

AnotherButler · 2019-02-28T22:56:10Z

docs/api/security/crypto.md

+repository](https://github.com/ARMmbed/mbed-os-example-mbed-crypto).
+
+
+### Configuring Mbed Crypto features


Please move this section above the examples.

AnotherButler · 2019-02-28T22:57:57Z

docs/api/security/crypto.md

+
+### Configuring Mbed Crypto features
+
+The Mbed TLS configuration system configures Mbed Crypto. Refer to [Mbed TLS


... Crypto. Please refer to...

AnotherButler · 2019-02-28T22:59:55Z

docs/api/security/crypto.md

+
+The Mbed TLS configuration system configures Mbed Crypto. Refer to [Mbed TLS
+documentation for how to configure Mbed TLS and Mbed
+Crypto](TLS.md#configuring-mbed-tls-features).


This won't work with the engine. Please use this link (and include the two periods and starting slash):
../apis/tls.html#configuring-mbed-tls-features

AnotherButler · 2019-02-28T23:03:36Z

docs/api/security/crypto.md

+fully ported to Arm Mbed OS. You can read more about this in our [porting
+guide](../contributing/index.html).</span>
+
+


Please remove the extra line between sections.

AnotherButler · 2019-02-28T23:04:25Z

docs/api/security/crypto.md

+
+We have adapted and [integrated Mbed Crypto with Mbed
+OS](https://github.com/ARMmbed/mbed-os/blob/master/features/mbedtls/mbedcrypto).
+On PSA platforms that support it, Mbed Crypto is pre-integrated with Mbed OS to


Please remove the hyphen in "preintegrated".

Or even change "is pre-integrated with" to "comes integrated with".

AnotherButler · 2019-02-28T23:07:14Z

docs/api/security/crypto.md

+We have adapted and [integrated Mbed Crypto with Mbed
+OS](https://github.com/ARMmbed/mbed-os/blob/master/features/mbedtls/mbedcrypto).
+On PSA platforms that support it, Mbed Crypto is pre-integrated with Mbed OS to
+take advantage of the platform's segmented architecture and isolate


"take advantage of" has a negative connotation to me. What do you think about replacing it with "make the most of" or "use"?

I've tried "leverage".

AnotherButler · 2019-02-28T23:07:40Z

docs/api/security/crypto.md

@@ -0,0 +1,49 @@
+## Crypto


Query: Should this be "Mbed Crypto" for proper branding?

TLS used only "TLS", so I did the same. I'll add Mbed.

AnotherButler · 2019-02-28T23:08:46Z

docs/api/security/crypto.md

+## Crypto
+
+Arm Mbed Crypto is the reference implementation of the cryptography interface
+of the Arm Platform Security Architecture (PSA). This is a preview release of


What is a preview release of Mbed Crypto? 5.12? This document? Please clarify.

I ask because I want this document to be maintainable. If this statement will change by the next feature release, we should remove it.

There is no stable PSA Crypto API yet, so as the PSA Crypto API changes (somewhat out of our control), we need to adapt and follow suit in order to pass compliance tests. This may mean API breaks for our users, hence the warning. I'm open to suggestions as to how to word this better.

Patater · 2019-03-01T09:28:00Z

Rebased to address comments raised so far.

AnotherButler

Thanks so much for making those changes 👍 I've left two more comments.

AnotherButler · 2019-03-01T16:17:58Z

docs/api/security/crypto.md

+Arm Mbed Crypto is the reference implementation of the cryptography interface
+of the Arm Platform Security Architecture (PSA). The version of Mbed Crypto
+shipping with Mbed OS is a preview release of Mbed Crypto, provided for
+evaluation purposes only.


Please replace the second sentence with:

Note: The version of Mbed Crypto shipping with Mbed OS is a preview release, provided for evaluation purposes only. Its contents may change in the next release.

(Feel free to update it if you think it's wrong/could be better.) Then, could you please tag @ndevillard for approval? I'd likehim to sign off on our wording for this one.

AnotherButler · 2019-03-01T16:18:44Z

docs/api/security/crypto.md

+
+The [Mbed Crypto project homepage on
+GitHub](https://github.com/ARMmbed/mbed-crypto) contains the following
+resources:


Please add a line after the colon, so the bullets render properly. Then, please add a period at the end of each bullet. (I'd make these changes myself, but I still can't edit your PR directly.)

Patater · 2019-03-04T13:09:41Z

Rebased to make requested changes.

Patater · 2019-03-04T16:42:26Z

Rebased to update with more changes.

AnotherButler

Thanks for all the fixes 👍

AnotherButler · 2019-03-04T20:00:36Z

docs/api/security/crypto.md

+implements PSA Crypto API v1.0b1.</span>
+
+We have adapted and [integrated Mbed Crypto with Mbed
+OS](https://github.com/ARMmbed/mbed-os/blob/master/features/mbedtls/mbedcrypto).


I think this link should be: https://github.com/ARMmbed/mbed-os/tree/master/features/mbedtls/mbed-crypto

AnotherButler · 2019-03-04T20:16:40Z

@Patater @dgreen-arm I'm fine making the remaining fix on my end once the PR is merged.

I'd like to transclude the Doxygen for this API. Do you know where I can find it here?https://os.mbed.com/docs/mbed-os/development/mbed-os-api-doxy/annotated.html

Is it under modules or data structures or somewhere else?

Add a Crypto API page which gives an overview of Mbed Crypto, links to the PSA Crypto API specification, links to the Crypto repo and its documents, and explains a bit about how Mbed Crypto is integrated with Mbed OS for boards with secure environments.

Patater · 2019-03-05T10:31:12Z

Rebased to fix link.

I'm not sure where the Mbed OS Doxygen would be. Our Doxygen is currently only used to generate the PSA Crypto API specification, as already transcluded by https://github.com/ARMmbed/mbed-crypto/blob/psa-api-1.0-beta/docs/PSA_Crypto_API_Reference.pdf

AnotherButler · 2019-03-05T19:16:08Z

@SenRamakri Could someone on your team please help us look into this?

Patater · 2019-03-06T15:54:40Z

Regarding code dependency, yes, there is one. We've merged the necessary code into Mbed OS master, via ARMmbed/mbed-os#9856 and a few previous PRs for Mbed Crypto 1.0.0dN versions (development snapshots).

Patater added the 5.12 label Feb 28, 2019

Patater force-pushed the add-crypto-api branch from 5de50aa to 3580add Compare February 28, 2019 16:07

dgreen-arm requested changes Feb 28, 2019

View reviewed changes

Patater force-pushed the add-crypto-api branch from 3580add to 5f6ac6d Compare February 28, 2019 16:44

dgreen-arm approved these changes Feb 28, 2019

View reviewed changes

AnotherButler suggested changes Feb 28, 2019

View reviewed changes

Patater force-pushed the add-crypto-api branch from 5f6ac6d to 4af795b Compare March 1, 2019 09:27

AnotherButler reviewed Mar 1, 2019

View reviewed changes

GuyWi mentioned this pull request Mar 3, 2019

PSA core 5.12 #986

Closed

Patater force-pushed the add-crypto-api branch from 4af795b to f262e69 Compare March 4, 2019 13:07

Patater force-pushed the add-crypto-api branch from f262e69 to 2178250 Compare March 4, 2019 15:01

AnotherButler approved these changes Mar 4, 2019

View reviewed changes

AnotherButler reviewed Mar 4, 2019

View reviewed changes

Patater force-pushed the add-crypto-api branch from 2178250 to 6ec7ebe Compare March 5, 2019 10:28

AnotherButler merged commit 005af18 into ARMmbed:development Mar 7, 2019

		@@ -0,0 +1,49 @@
		## Crypto

		Arm Mbed Crypto is reference implementation of the cryptography interface of


		### Other resources

		The [Mbed Crypto website](https://github.com/ARMmbed/mbed-crypto) contains [an

		repository](https://github.com/ARMmbed/mbed-os-example-mbed-crypto).


		### Configuring Mbed Crypto features


		### Configuring Mbed Crypto features

		The Mbed TLS configuration system configures Mbed Crypto. Refer to [Mbed TLS

		fully ported to Arm Mbed OS. You can read more about this in our [porting
		guide](../contributing/index.html).</span>

crypto: Add Crypto API documentation #983

crypto: Add Crypto API documentation #983

Uh oh!

Conversation

Patater commented Feb 28, 2019

Uh oh!

dgreen-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater commented Feb 28, 2019

Uh oh!

AnotherButler commented Feb 28, 2019

Uh oh!

AnotherButler commented Feb 28, 2019

Uh oh!

AnotherButler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater commented Mar 1, 2019

Uh oh!

AnotherButler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater commented Mar 4, 2019

Uh oh!

Patater commented Mar 4, 2019

Uh oh!

AnotherButler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnotherButler commented Mar 4, 2019

Uh oh!

Patater commented Mar 5, 2019

Uh oh!

AnotherButler commented Mar 5, 2019

Uh oh!

Patater commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Patater commented Mar 6, 2019 •

edited

Loading