add TL entries for Open-DICE #41

thomas-fossati · 2024-03-19T21:15:06Z

This PR is a rework of #38 which:

narrows the scope to Open-DICE
requires one TL for each cert in the chain
adds an explicit profile_id field
adds a registry for profile_id's
populates the profile_id's registry with Android entries

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

thomas-fossati · 2024-03-19T21:15:44Z

cc @danh-arm @apalos @jmarinho

source/transfer_list.rst

jmarinho · 2024-03-24T22:17:00Z

source/transfer_list.rst

+   * - tag_id
+     - 0x3
+     - 0x0
+     - The tag_id field must be set to **0x200**.


I'd like to request @manish-pandey-arm to confirm these tag_ids are the best suited, or whether we should use lower values.

I'd say it's too greedy to start a new block of 256. I think TF-A can live with 2 blocks of 16 for now, so we could start this range at 0x120. What do others think?

I strongly agree :-)

Going back to #38 , i can't see the reasoning why this tag was moved from generic range to 0x200 ? It is a good candidate to stay in generic range.
As mentioned by Dan, i don't think TF-A going to consume more than 32 entries, so 0x120 should be a safe value (if we decide to keep in non-generic range)

Both options work for me:

Starting at 0x120

Using the generic range, in which case 8, 9 and 10 are the lowest three that are available - 7 is already taken by the heap carve-outs Add Mbed-TLS crypto heap info entry #45

Let me know what is your preference.

@danh-arm @manish-pandey-arm @jmarinho @apalos

I believe I have addressed all the outstanding comments except for this one.

8-10 seems fine

I would prefer it to be in generic range starting at 0x7 as Mbed-TLS heap has now been moved to TF-A range (0x105) https://github.com/FirmwareHandoff/firmware_handoff/pull/45/files

danh-arm

Please also add a commit message.

I'm OK with this PR in essence. My comments should be easy to address.

danh-arm · 2024-04-03T13:10:50Z

source/transfer_list.rst

+   * - tag_id
+     - 0x3
+     - 0x0
+     - The tag_id field must be set to **0x200**.


I'd say it's too greedy to start a new block of 256. I think TF-A can live with 2 blocks of 16 for now, so we could start this range at 0x120. What do others think?

source/transfer_list.rst

danh-arm · 2024-04-03T13:26:39Z

source/transfer_list.rst

+This entry type holds one certificate of the Open DICE certificate chain.
+
+No specific certificate format is mandated.
+Examples of certificate formats include X.509, CWT, and C509.


Please spell out CWT (or just remove if you agree with my previous point). References would be nice too.

I will expand CWT and add a reference.

danh-arm · 2024-04-03T13:27:57Z

source/transfer_list.rst

+
+This entry type holds one certificate of the Open DICE certificate chain.
+
+No specific certificate format is mandated.


... but the format must be self-describing. Actually the Open DICE spec mandates either x509 or CWT. Wouldn't it be best to just refer to that section of the spec instead of re-specifying here?

I'll add a ref to the relevant section in the Open DICE spec.

danh-arm · 2024-04-03T13:37:20Z

source/transfer_list.rst

+This entry type holds the "Attestation CDI" described in [Open-DICE]_.
+
+The CDI computed by the Sender's boot stage SHALL be added to the TL.
+The CDI received from the previous boot stage (i.e., DICE layer) MUST NOT be forwarded.


I think we need a comment here about what the receiver should do with the profile_id, e.g. use this to generate a CDI that is compatible with the previous layer's CDI as per the Android OpenDICE spec.

I think we need a comment here about what the receiver should do with the profile_id, e.g. use this to generate a CDI that is compatible with the previous layer's CDI as per the Android OpenDICE spec.

What is the behaviour in case $L_{i+1}$ does not implement a profile compatible with layer $L_i$?

danh-arm · 2024-04-03T13:38:18Z

source/transfer_list.rst

+This entry type holds the "Sealing CDI" described in [Open-DICE]_.
+
+The CDI computed by the Sender's boot stage SHALL be added to the TL.
+The CDI received from the previous boot stage (i.e., DICE layer) MUST NOT be forwarded.


Ditto a comment about the profile_id would be good. Or you could put this in the "Profile Identifiers" section.

OK, I'll add it here so that all requirements are kept together.

danh-arm · 2024-04-03T13:40:43Z

source/transfer_list.rst

+   * - profile_id
+     - 0x4
+     - 0x8
+     - the Open DICE profile identifier (See :numref:`opendice_profile_id` for the allowed values)


In the rendered version, this says 2.7.9 but the section is a 4th level heading without numbering. Maybe change the ref to the section name instead?

If I make html the reference is generated correctly.

For the avoidance of doubt, I will change this into a reference to the table itself.

danh-arm · 2024-04-03T13:40:54Z

source/transfer_list.rst

+   * - profile_id
+     - 0x4
+     - 0x8
+     - the Open DICE profile identifier (See :numref:`opendice_profile_id` for the allowed values)


Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

sjg20 · 2024-03-20T05:06:58Z

source/transfer_list.rst

+Entries related to Open DICE
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+These entries contain [Open-DICE]_ information elements that must be transferred between boot stages in a DICE architecture where a DICE Protection Environment (DPE) is not used.


It is good that you have expanded the acronyms but can you do the same for DICE, please!

sjg20 · 2024-03-20T05:07:36Z

source/transfer_list.rst

+
+These entries contain [Open-DICE]_ information elements that must be transferred between boot stages in a DICE architecture where a DICE Protection Environment (DPE) is not used.
+
+This includes the Compound Device Identifier (CDI) used for attestation and the CDI used for sealing, as well as the accumulated certificate chain.


I think it is better to wrap to at most 80cols

sjg20 · 2024-03-20T05:08:34Z

source/transfer_list.rst

+This entry type holds the "Sealing CDI" described in [Open-DICE]_.
+
+The CDI computed by the Sender's boot stage SHALL be added to the TL.
+The CDI received from the previous boot stage (i.e., DICE layer) MUST NOT be forwarded.


Please mention why

sjg20 · 2024-03-20T05:09:34Z

source/transfer_list.rst

+This entry type holds one certificate of the Open DICE certificate chain.
+
+No specific certificate format is mandated.
+Examples of certificate formats include X.509, CWT, and C509.


How do you know which format is being used? Why not select one format?

sjg20 · 2024-04-12T14:11:57Z

source/transfer_list.rst

+   * - tag_id
+     - 0x3
+     - 0x0
+     - The tag_id field must be set to **0x200**.


I strongly agree :-)

sjg20 · 2024-06-18T13:04:11Z

source/transfer_list.rst

+   * - tag_id
+     - 0x3
+     - 0x0
+     - The tag_id field must be set to **0x200**.


8-10 seems fine

sjg20 · 2024-06-18T13:11:00Z

source/transfer_list.rst

+
+This entry type holds the "Attestation CDI" described in [Open-DICE]_.
+
+The CDI computed by the Sender's boot stage SHALL be added to the TL.


This use of SHALL, MUST and MUST NOT seems new for this spec.

I suggest using lower case, since people will either read it or they won't. Capitals don't help with that.

If we say 'must' or 'should' then I think it is important to say why, e.g. because otherwise xxx will happen

sjg20 · 2024-06-18T13:12:53Z

source/transfer_list.rst

+   * - cert
+     - data_size - 0x4
+     - 0xc
+     - Holds the certificate for the next layer, produced by the current layer


'certifcate' is vague...what format are these in? Please add enough detail so that people know how to create the info

add TL entries for Open-DICE

2015de5

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

thomas-fossati temporarily deployed to pr_ci March 19, 2024 21:16 — with GitHub Actions Inactive

apalos previously approved these changes Mar 22, 2024

View reviewed changes

jmarinho reviewed Mar 24, 2024

View reviewed changes

danh-arm requested changes Apr 3, 2024

View reviewed changes

jmarinho requested a review from manish-pandey-arm April 9, 2024 18:02

thomas-fossati and others added 2 commits June 17, 2024 09:33

Merge branch 'FirmwareHandoff:main' into main

2e03b9a

quote DICE spec more carefully

3147252

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

thomas-fossati dismissed apalos’s stale review via 3147252 June 17, 2024 12:18

thomas-fossati added 3 commits June 17, 2024 15:46

add reference to the "Certificate Details" section of Open DICE

12156ed

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

require profile compatibility

f4801c7

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

reference the profile id table rather than the section.

705af27

Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>

thomas-fossati had a problem deploying to pr_ci July 17, 2024 14:16 — with GitHub Actions Failure

sjg20 requested changes Oct 17, 2024

View reviewed changes

danh-arm self-assigned this Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add TL entries for Open-DICE #41

add TL entries for Open-DICE #41

thomas-fossati commented Mar 19, 2024

thomas-fossati commented Mar 19, 2024

jmarinho Mar 24, 2024

danh-arm Apr 3, 2024

sjg20 Apr 12, 2024

manish-pandey-arm Jun 14, 2024

thomas-fossati Jun 17, 2024

thomas-fossati Jun 17, 2024

sjg20 Jun 18, 2024

manish-pandey-arm Jul 29, 2024

danh-arm left a comment

danh-arm Apr 3, 2024

danh-arm Apr 3, 2024

thomas-fossati Jun 17, 2024

danh-arm Apr 3, 2024

thomas-fossati Jun 17, 2024

danh-arm Apr 3, 2024

thomas-fossati Jun 17, 2024

danh-arm Apr 3, 2024

thomas-fossati Jun 17, 2024

danh-arm Apr 3, 2024

thomas-fossati Jun 17, 2024

danh-arm Apr 3, 2024

sjg20 Mar 20, 2024

sjg20 Mar 20, 2024

sjg20 Mar 20, 2024

sjg20 Mar 20, 2024

sjg20 Apr 12, 2024

sjg20 Jun 18, 2024

sjg20 Jun 18, 2024

sjg20 Jun 18, 2024


		This entry type holds one certificate of the Open DICE certificate chain.

		No specific certificate format is mandated.


		These entries contain [Open-DICE]_ information elements that must be transferred between boot stages in a DICE architecture where a DICE Protection Environment (DPE) is not used.

		This includes the Compound Device Identifier (CDI) used for attestation and the CDI used for sealing, as well as the accumulated certificate chain.


		This entry type holds the "Attestation CDI" described in [Open-DICE]_.

		The CDI computed by the Sender's boot stage SHALL be added to the TL.

add TL entries for Open-DICE #41

Are you sure you want to change the base?

add TL entries for Open-DICE #41

Conversation

thomas-fossati commented Mar 19, 2024

thomas-fossati commented Mar 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danh-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment