Re: [Rats] WGLC for draft-ietf-rats-tpm-based-network-device-attest

Hi all,

please find below a review of 
I-D.ietf-rats-tpm-based-network-device-attest-04. There are no 
fundamental issues with this I-D. If the nits and comments are taken 
into account and addressed where applicable or feasible (next to other 
WGLC reviews, of course), I support moving this I-D forward. In general, 
the I-D evolved quite nicely. It is well written and provides an 
appropriate framing / basis for multiple other documents in the working 
group.

First comment is the "biggest issue" (but a dependency to the 
architecture I-D that is still TBD, actually, so it is a comment).

Viele Grüße,

Henk

COMMENT
"A number of terms are reused from [I-D.ietf-rats-architecture]."
If the recent introduction of the term Reference Value Provider and 
corresponding elevation of Reference Value find consensus in the group, 
that will have an effect on the usage of Endorser and reference 
values/measurements in this document. This could result in quite a few 
precise incisions wrt rewording - but nothing invasive to structure or 
content.
--

COMMENT
Definition of Remote Attestation: While in itself correct that might not 
be complete? Are you sure you want to use "claims" only as the basis of 
this definition and not "Evidence"? The definition does not refer to any 
kind of measures to ensure trustworthiness associated with the Claims 
created (I'd use generated for consistency), conveyed and appraised.
--

COMMENT
"The goal of attestation" seems to imply boot-time integrity, yet it 
does not introduce the term. The rest of the document is more open on 
the exact scope and sometime seems to lean on the side of run-time 
integrity even - until Section 1.6.1.
The "in-scope" must be made more clear at the very beginning.
--

NIT
"Network(ing) Equipment" is capitalized quite often, but not defined as 
a specific term?

--
COMMENT
Section 1.4 speaks of services, but Platform Identity and Software 
Measurement are not services per-se. Maybe "services providing..." or 
something similar?
--

NIT
OLD:
"Creation of Evidence is the process whereby"

NEW:
"Generation of Evidence is the process whereby"
--

COMMENT
"Software used to boot a device can be described as a chain of 
measurements, anchored at the start by a Root of Trust for Measurement, 
that normally ends when the system software is loaded."

The description of this 3rd major process deviates from the other ones 
as it does not start with its "name".
--

NIT
All devices here are Attesters, right? Maybe add a sentence about that and:
OLD:
"of the identity of devices that make up their network"

NEW:
"of the identity of Attesters that make up their network"
--

NIT
OLD:
"attesting device's TPM"

NEW:
"Attester's TPM"
--

NiT
"Root of Trust for Measurement"
Capitalized but not introduces or defined? A forward reference to an 
Appendix might be okay here.
--

COMMENT
"As the Verifier and Relaying Party roles are often combined within 
RIV," comes a little bit a as a surprise for the reader. If this is 
vital here, it should be elaborated on in the text above.
--

NIT
"(for example, TCG Platform Certificates [Platform-Certificates])"
First use of acronym and not introduced.
--

NIT
"to the system in which verification will take place"
Why not simply say Verifier as that term is pulled in in Section 1.1?
--

COMMENT
"as network equipment is often required to "self-configure"
Inconsistent capitalization (sometimes also, e.g., "Networking 
Equipment" or "Network equipment") throughout the text. Maybe use 
"network intermediate systems" or define Network Equipment at the top of 
the text, too?
Includes the NIT: very long sentence
--

NIT
"Remote Attestation is a very general problem"
I really hope that remote attestation is not a problem :-)
--

NIT
"This solution is for use in non-privacy-preserving applications"
This item misses a period at the end.
--

NIT
OLD:
"but are not considered in this revision of the document."

NEW:
"but are not considered in this document."
--

COMMENT
"The measurement process generally follows the Chain of Trust model used 
in Measured Boot, where each stage of the system measures the next one"

This item should refer to the Layered Attestation concept defined in the 
architecture I-D. If you stick with the term Chain of Trust (and 
Measured Boot, analogously) that again needs at minimum a reference and 
should be elaborated on beforehand in the text above.

Also, as the next item refers to a Verifier, why does this item not 
refer to Attester?
--

COMMENT
"retrieve the logs and a copy of the digests collected by hashing each 
software object"
Is there a suitable reference for this procedure?
--

COMMENT
"TPM's attestation public key"
Maybe refer to the Authentication Secret ID in the reference interaction
models I-D?
--

COMMENT
"comparing digests in the log with known-good values"
First use and not set into context. Maybe use RIM or golden measurements 
as these were already introduced? We already established that there are 
too many synonyms here. At the very least:

reference values,
nominal values,
golden measurements,
known good values

Resolving this mess should not be hidden in this document, though :-) 
Here we only need consistent usage of one selected term or put the terms 
used in this document in relation.
--

COMMENT
Figure 1 basically is a specialized view of Layered Attestation (as 
indicated in a comment above). Capturing that helps, I think.
--

NIT
"These settings may have vendor defaults, but often can be changed by 
administrators, who may want to verify via attestation that the settings 
they intend are still in place."
Colloquial; better: operational state vs. intended state
--

COMMENT
In Figure 2: This table indicated that boot-time integrity goes beyond 
PCR 7 and requries IMA - as is elaborated on in upcoming text that 
follows. Maybe highlight the demarcation line when "boot-time" ends ad 
"run-time" begins with respect to PCR 9 & 10??
--

COMMENT
"Designers must recognize that some PCRs may cover log entries that a 
particular Verifier considers critical and other log entries that are 
not considered important, so differing PCR values may not on their own 
constitute a check for authenticity."
For me that is hard to grasp by its own. Maybe an example could be quite 
beneficial here?
--

COMMENT
Section 2.2.: It's actually four keys as both are asymmetric key-pairs, 
isn't it? At the moment the identity key refers to the pub-key of the 
first pair (embedded in a cert) and the attestation key refers to the 
priv-key of the second pair (which a cert also exists for containing the 
corresponding pub-key), I think. If a certificate is mentioned in the 
same context as a key: is the key then always referring to the priv-key 
and the certificate implicitly referring to the pub-key?

Also, why should the DevID be protected? Do you mean the corresponding 
priv-key or actually the certificate with pub-key that is intended to be 
presented to other RATS roles? If so, the reader probably needs to 
understand why.
--

COMMENT
"Attestation Key (AK) and its x.509 certificate should parallel the 
DevID" This seems to be a vital part here and is captured in only one 
sentence. Maybe elaborate a bit on what "parallel" means and provide an 
exhaustive well-defined list of elements/extensions that must be 
aligned? "i.e." implies some wriggle room, but this is already an 
exhaustive list, correct?
--

NIT
OLD:
"mechanism whereby an Administrator"

NEW:
"mechanism whereby an administrator"
--

COMMNENT
"1.  The Attesting Device"
Why not use Attester here (applies also to other places, this is the 
3rd, actually)?
--

COMMENT
"that will retrieve the information"
Why not "evidence and measurement logs"?
--

NIT
Figure 3 / Section 2.3.: "Device under attestation" reads a bit funny. 
Maybe a "reference" to target of evaluation or something similar in the 
leading text would iron that out a little bit?
--

COMMENT
"The IAK cert contains the same identity information as the DevID [...], 
but it's a type of key that can be used to sign a TPM Quote." I cannot 
parse this sentence. What is it trying to say? You are not trying to say 
that the pub-key in the IAK cert is used to sign a TPM Quote, I think.
--

COMMENT
Section 2.4.1.: There are two types of "RIM" referenced and described in 
this I-D, I think.

Reference Integrity Measurements
and
Reference Integrity Manifests

Both can be easily confused as their meaning is very similar. I think it 
would help to disambiguate them earlier in the text or in this section 
at the latest.
--

COMMENT
"The log must contain enough information to demonstrate its integrity by 
allowing exact reconstruction of the digest conveyed in the signed quote 
(i.e., PCR values)."
This is an essential piece of the puzzle and in the light of that is 
relatively hard to read. Maybe elaborate more on this characteristic - 
especially how a log exactly can "demonstrate its integrity" without 
being signed.". A forward reference to the Appendix could be a minimal, 
but not pretty, fix.
--

NIT
"The Reference Interaction Model for Challenge-Response-based Remote 
Attestation is based on the standard roles defined in 
[I-D.ietf-rats-architecture]."

Reference for "Reference Interaction Model for Challenge-Response-based 
Remote Attestation" is missing.
--

COMMENT
"The Attestation Identity Key (AIK)"
As it was assumed the an Initial Attestation Key is provisioned, would 
this make it the.... Initial Attestation Identity Key, analogously 
(IAIK)? AIK and IAK could be a bit confusing to the reader, I think.
--

                                                     COMMENT
"On the Attester, measured values are retrieved from the Attester's 
TPM." and "Quotes are retrieved according to TCG TAP Information Model 
[TAP]." somehow reads as two separate steps in sequence, but they 
actually describe the same step (a quote operation on the TPM), right?

In general, the "time(eg)" item is quite difficult to read and I 
recommend a re-write with better structure.
--

NIT
OLD:
"and makes a request attestation data or one or more PCRs from an Attester."

NEW:
"and issues a request for evidence about one or more PCR values to the 
Attester including that nonce."
--

NIT
"While the TAP IM gives[...]" starts a very long sentence.
--

NIT
"If the AIK signature is not correct, or freshness such as that provided 
by the nonce is not included in the response, the device SHOULD NOT be 
trusted."
I am having a hard time parsing this sentence and assume it is broken.
--

NIT
OLD:
"If time(RG)-time(NS) is greater than the threshold in the Appraisal 
Policy for Evidence,"

NEW:
"If time(RG)-time(NS) is greater than the threshold in the Appraisal 
Policy for Evidence that assess its freshness,"
--

COMMENT
"Implementations that use NETCONF MUST do so over a TLS or SSH secure 
tunnel.  Implementations that use RESTCONF transport MAY do so over a
TLS or SSH secure tunnel."
MUST and MAY are in relatively stark contrast. Is that intentional here?
--

NIT
OLD:
"Retrieval of Log Evidence SHOULD be via log interfaces on the network 
device."

NEW:
"Retrieval of Log Evidence SHOULD conducted via log interfaces provided 
by the Attester."
--

COMMENT
OLD:
"Figure 5 above assumes that the Verifier is implicitly trusted, while 
the Attesting device is not."

NEW:
"Figure 5 above assumes that the Verifier is trusted, while the 
Attesting device is not."
Let's avoid bringing the "implicit" discussion here.
--

NIT
OLD:
"Devices may also have to carry Appraisal Policy for Evidence for each 
possible peer device so that each device has everything needed for 
attestation, without having to resort to a central authority."

NEW:
"Devices may also have to carry Appraisal Policy for Evidence for each 
possible peer device so that each device has everything needed for 
remote attestation, without having to resort to a central authority."
--

NIT
OLD:
"to the Equipment's Administrator"

NEW:
"to the equipment's Administrator"
--

COMMENT
"Keys may be compromised."
Which keys? All of them? Basically every key used in this document is 
Attester-located. An Attester does not generate Attestation Results, but 
Evidence. The leading sentence in this Security Consideration section 
scopes it to Attestation Results and not Evidence. That is rather 
surprising. Is it intentional?
I think "Attestation Results from the RIV procedure are subject to a 
number of attacks:" does not really defines the scope that was intended 
to be define here :-)
--

COMMENT
"Two sets of keys are relevant to RIV attestation"
Again I am rather sure I think what keys are actually referred to here, 
but I would like it better, if it is spelled out that it's priv-keys of 
asym key-pairs, explicitly (and if that's a misconception of mine, 
that's an even better reason!).
--

COMMENT
"Keeping keys safe is just part of attestation security"
It's also the critical enabler of trustworthiness (which is why we 
require Endorsements about the capabilities to keep these keys safe).
--

NIT
OLD:
"An unbroken chain of trust is essential to ensuring that blocks of code 
that are taking measurements have been verified before execution (see 
Figure 1."

NEW:
"An unbroken chain of trust is essential to ensuring that blocks of code 
that are taking measurements have been verified before execution (see 
Figure 1.)"

COMMENT
"Requiring results of attestation of the operating software to be signed 
by a key known only to the TPM also removes the need to trust"
Is that different to Attestation Results? It's about... the successful 
use of remote attestation to improve trust in the trustworthiness of 
Attesters, right? If so, I do not think the text expresses that well. If 
not, I don't understand this paragraph, obviously.
--

NIT
OLD:
"The entire device could be spoofed, that is, when the Verifier goes to 
verify a specific device, it might be redirected to a different device."

NEW:
"The entire Attester could be spoofed. If the Verifier goes to appraise 
a specific Attester, it might be redirected to a different Attester."
--

COMMENT
OLD:
"A compromised device could respond with a spoofed Attestation Result, 
that is, a compromised OS could return a fabricated quote."

NEW:
"A compromised Attester could respond with a spoofed Evidence, that is, 
a compromised OS could return a fabricated Quote."
Maybe this is about the Verifier instead? Then Attestation Result would 
make sense, but Quote would not. In general, spoofed Evidence is 
probably a separate problem to fabricated Quote. Why is Quote here used 
without capitalization?
--

COMMENT
"To block a spoofed Attestation Result, the quote generated inside the 
TPM must be signed by a key that's different from the DevID, called an 
Attestation Key (AK)."
This starts to cement my confusion. I start to think Attestation Result 
should be Evidence here. The document never elaborates on how a Quote 
could be included in an Attestation Result or why, I think. While that 
is feasible in some solutions, it seems not in scope of RIV? Maybe I am 
missing something very obvious here.
--

NIT
"[Editor's Note: does this require an OID that says the key is known by 
the CA to be an Attestation key?]"
This question should probably get answered before IESG review.
--

COMMENT
"These two keys and certificates are used together:"
And finally, this seems to talk about pub-key and priv-key again. I 
think this "two keys" alliteration in the overall text should be more 
clear and definitely consistent in intend and wording. Just highlighting 
that to me it reads confusing.
--

COMMENT
"Device owners, however, can use any other mechanism they want to assure 
themselves that Local identity certificates are inserted into the 
intended device"
Why is local capitalized till the end of the section?
--

COMMENT
"In addition to trustworthy provisioning of keys, RIV depends on other 
trust anchors.  (See [SP800-155] for definitions of Roots of Trust.)"
Why is the heading and text talking about trust anchors, but the 
reference and following lost about roots of trusts?
--

COMMENT
Sectio 8.2. should probably highight that an RTM is typically not part 
of the TPM and has to be endorsed independently.
--

COMMENT
Section 8.3.1.: and there I though OS start-up is in scope. Maybe it is 
not? To me, this section conveys the impression that PCR 8-10 as well as 
IMA, as well as log transfer elaborated on in the body of this document 
are illustrating un-specified methods. Is that the intend of 8.3.1.? Or 
does this Section just elaborates on "and with OS entropy kicks in and 
Quotes can only be appraised using Event-Logs and not with PCR KGV anymore"?
--

On 21.09.20 04:06, Nancy Cam-Winget (ncamwing) wrote:
> RATs participants,
> 
> This is a WGLC for draft-ietf-rats-tpm-based-network-device-attest, 
> please provide comments and feedback by October 12, 2020.
> 
> The draft can be found in:
> 
> https://datatracker.ietf.org/doc/draft-ietf-rats-tpm-based-network-device-attest/
> 
> Best, Nancy (on behalf of the RATs chairs)
> 
> 
> _______________________________________________
> RATS mailing list
> RATS@ietf.org
> https://www.ietf.org/mailman/listinfo/rats
> 