Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments from Jürgen Schönwälder #70

Closed
dthaler opened this issue Oct 16, 2019 · 3 comments
Closed

Comments from Jürgen Schönwälder #70

dthaler opened this issue Oct 16, 2019 · 3 comments
Assignees
Labels
ready to close Ready for WG chairs to verify and close

Comments

@dthaler
Copy link
Collaborator

dthaler commented Oct 16, 2019

[DT: Some of the below are already tracked by other GitHub issues, but copying entirety here to make sure I don't miss something]

Schönwälder, Jürgen J.Schoenwaelder@jacobs-university.de wrote:

  • RFC 2119 language

    This seems mainly used in the section about attestations. Do we
    really need RFC 2119 terms in this architecture document? Some of
    the SHALLs do not seem to be really needed.

[DT: above is tracked by issue #69]

  • UA == CA ?

    The introduction introduces UA but then this is only used again once
    in 5.2. Given that section 2 starts with defining CA, is CA the same
    as UA (and hence UA should be replaced with CA)?

[DT: above is tracked by issue #55]

  • root of trust

    This definition remains a bit unclear and it ends with an ad-hoc
    reference to a NIS draft. I searched for RoT in the document but it
    is not widely used. Perhaps this term can even be replaced by simply
    expanding the definitions of TFW a bit?

  • acronyms and terms

    I notice that not all terms for which acronyms are defined at the
    end of section 2 are actually defined terms. Perhaps they should all
    be defined? Missing are TAM (Trusted Application Manager) and SD
    (Security Domain). The later one (SD) in particular would benefit
    from a definition.

[DT: SD is tracked by issue #62]

  • assumptions

    Does it make sense to have a top-level section consisting of just two
    sentences? Perhaps move this into a subsection of the architecture
    section?

  • #notionalarch

    What is #notionalarch? A broken reference to Figure 1?

  • typo caption figure 2

    s/wtih/with/

  • typo 5.5

    s/would required/would require/
    s/recieve/receive/

  • typo 5.7

    s/may different/may be different/

  • trust anchors

    It seems that managing trust anchors will be crucial in order to
    deal with 'crypto gone wrong' scenarios. Is it really smart to leave
    management and updates to proprietary mechanisms? Why can't trust
    anchors not be managed like other TEE data? What is the reason to
    declare this out of scope - too complex? too hard to find agreement?
    Will trust anchor changes cause installed TAs to be removed if they
    lost their trust?

[DT: above is tracked by issue #32]

  • message security

    It may help to clarify what the 'ends' are in the statement
    "messages are signed end-to-end". In particular, is the end the TEEP
    Agent and the TAM?

  • teep broker

    I am not sure what "A shared module in REE comes to meet this need."
    tells me. What is a "shared module"? Perhaps the sentence can simply
    be removed.

    I am not sure the text is really clear. There are two kinds of
    interactions, a CA needing a TA to be made available and CA using a
    TA. I assume the first goes with the TEEP broker but the later via
    REE OS mechanisms. Or does the TEEP broker provide the single
    interface via which CAs request TA services?

  • attestation

    "but the OTrP protocol SHALL allow" - this is the first time OTrP
    shows up and RFC 2119 language is used; did you mean to say a TEEP
    protocol? or something like that?

[DT: above is tracked by issue #17]

  • crypto properties

    Does it really make sense to fix digital signature formats in an
    architecture document? The list of formats will likely change over
    time; so it is good to define MUST for today's blessed list?

[DT: above is tracked by issue #17]

  • figure teep attestation layout

    Perhaps change the layout such that the claims are listed one below
    the other instead of next to each other (this way things may fit
    into the space - yes, I am still using 80 column terminals).

[DT: above is tracked by issue #17]

  • typo attestation flow

    s/Attesations/Attestations/

  • security

    Why does that text say "should be validated" instead of "must be
    validated"?

[DT: above is covered by issue #69]

What is the GetTAInformation function discussed in the security
considerations? It seems to fall out of the air.

dthaler added a commit that referenced this issue Nov 18, 2019
Fixes part of issue #70

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
dthaler added a commit that referenced this issue Nov 23, 2019
Addresses issue #32 and part of issue #70

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
dthaler added a commit that referenced this issue Nov 26, 2019
Align picture with diagrams used in the TEEP WG at IETF 105

THis addresses issues #17, #31, and the part of #70 that talks about
digital signature formats.  Per discussion at IETF 106, the direction is
that the architecture document should explain the relationship between
TEEP and attestation, and leave protocol details to the TEEP protocol
spec. It should NOT discuss attestation details, including anything
about signing with any attestation key, seeding of attestation keys,
or using specific crypto algorithms for attestation.

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
dthaler added a commit that referenced this issue Nov 27, 2019
Most feedback in Juergen's review in issue #70 are addressed by other
PRs that address other github issues.  This PR covers the rest of
issue #70.

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
@dthaler dthaler self-assigned this Nov 27, 2019
@dthaler dthaler added the have proposed text Ready for other editors to review and merge if ok label Nov 27, 2019
@dmwheel1
Copy link
Collaborator

I disagree with removing Root-of-Trust and replacing with TFW, but open to the discussion. TFW is not a commonly used term and RoT is more common in the industry. Perhaps we need to clean up to use the common term.

@dthaler
Copy link
Collaborator Author

dthaler commented Nov 27, 2019

Actually I am thinking that we need to remove TFW from the terminology section and not use it in the document except as an example (like we did with security domain). Unlike the old OTrP which baked in the concept of TFW in the protocol in ways that caused problems for SGX, the TEEP protocol has no such problems and TFW is no different from any other trusted dependency (e.g., OP-TEE, or a launch enclave, or microcode or whatever else).

@dthaler
Copy link
Collaborator Author

dthaler commented Nov 27, 2019

Also the SUIT WG decided to not use the term RoT in the doc. Is there a reason we need it that would not apply to SUIT?

@dthaler dthaler assigned ncamwing and unassigned dthaler Dec 7, 2019
@dthaler dthaler added ready to close Ready for WG chairs to verify and close and removed have proposed text Ready for other editors to review and merge if ok labels Dec 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to close Ready for WG chairs to verify and close
Projects
None yet
Development

No branches or pull requests

4 participants