Subject: [Emu] Iotdir early review of draft-ietf-emu-eap-noob-01

From: Dave Thaler via Datatracker <[email-masked]>
Date: Fri, 12 Jun 2020 17:40:12 -0700

Reviewer: Dave Thaler
Review result: Ready with Issues

A marked up copy with my comments inline, including editorial nits not covered
in this email is at 
https://www.microsoft.com/en-us/research/uploads/prod/2018/06/draft-ietf-emu-eap-noob-01.pdf
(a Word version is also available if requested, but the PDF should suffice).
See change tracking in red throughout the PDF for editorial nits.

Summary of issues:

Applicability
-------------

1) The document states that it does not support static printed registration
codes.  It would benefit from stating the *rationale* for not supporting
things like QR code stickers.  E.g., does the WG believe that such things
are less secure?  The document does say this "also" prevents attacks where
a static secret code would be leaked, but the use of "also" implies that's
not the main rationale.

2) Section 3.2.5 says:
> A peer that has not received an OOB message MUST wait at
> least the server-specified minimum waiting time in seconds

This means that this protocol cannot be easily implemented in IoT devices
that have no relative time clock.  Does EAP itself already have this limitation
regardless of EAP method?  If not, it would be good to call this out, since
this limits applicability to constrained devices.   Maybe add an
"Applicability" section like EAP (RFC 3748) section 1.3 has.

3) Section 3.3.2 says:
> The in-band messages are formatted as JSON objects [RFC8259]

So this limits applicability to constrained IoT devices, since JSON can be
verbose compared to, say, CBOR, and if the IoT device already uses CBOR for
its normal protocol use this requires adding a separate parser for JSON which
may cause code size issues.   Is there a rationale for why CBOR could not be
an option?  E.g., if this protocol is not applicable for constrained devices,
then say so.  (I don’t know whether EAP itself already inherently has
problems that limit its applicability for constrained devices.)

4) Appendix E says:
> The ServerInfo in this case includes a JSON member called ServerUrl
> of the following format with maximum length of 60 characters:

Just an observation: This limits applicability to servers that can get short
hostnames (and paths), which might make it less applicable users in some
cultures/languages.

Interoperability
----------------

5) Section 3.1 says:
> The server and peer MAY implement user reset of 
> the association by deleting the state data from that endpoint.  If an
> endpoint continues to store data about the association after the user
> reset, its behavior SHOULD be equivalent to having deleted the
> association data.

As phrased, there are 3 compliant behaviors:
Behavior 1) Follow the MAY and delete the data
Behavior 2) Don’t follow the MAY, do follow the If, and follow the SHOULD
            and act as if data was deleted
Behavior 3) Don’t follow the MAY, do follow the If, and don’t follow the
            SHOULD and hence act in any other way.  In my experience, such
            undefined behavior can cause interoperability issues.

Is there any reason to permit behavior 3?   Why can’t you make the SHOULD be
a MUST (since the If already makes it conditional on implementations that
don’t follow the MAY).

6) Section 3.2.5 says:
> If the server has not sent any SleepTime
> value, the peer SHOULD wait for an application-specified minimum time
>  (SleepTimeDefault).

Since the minimum time is app-specified as this sentence says, what does it
mean to not follow this SHOULD?  I.e., why is it not a MUST?

7) Section 3.4.2 says:
> The endpoints MAY send updated Realm, ServerInfo and PeerInfo objects
> in the Reconnect Exchange.  

So if this MAY is not followed, does that mean any updates are not sent at
all, or that they are sent via some other mechanism?

8) Appendix C says:
> They contain
> JSON objects whose structure may be specified separately for each
> application and each type of OOB channel.  

It doesn’t look like you’re specifying an IANA registry to contain field
names.  Given that, how do you get interoperability?   If every app and type
of OOB channel specifies a different structure, how do you do capability
negotiation to agree on which structure definition is being used?  (Since
two specifications might use the same field names for very different purposes,
in theory.)   I didn’t see any identifier that can be used as the “type” of
the ServerInfo or PeerInfo data.

Attacks and mitigations
-----------------------

9) Section 3.1 says:
> For example, consider a printer
> (peer) that outputs the OOB message on paper, which is then scanned
> for the server.

Elaborate on this use case… Is the assumption that the printer would
automatically consume paper in response to any unauthenticated message
(hence being a resource consumption attack)?  Or that it would only do so
after getting approval from a human to output the OOB message?

10) Section 6.3 discusses misbinding attacks, but another attack is where a
rogue device keeps generating random IDs (MAC, etc.) in order to cause havoc
with the server, whether that’s a memory attack, or an attempt to cause so
much UI noise to users that it DoS’s the user's ability to add valid devices.
I didn't see such attacks discussed.

Internationalization
--------------------

11) Section 3.4.1, Table 2, says about PeerId:
> UTF-8 string (typically 22 bytes)        

Top of page 17 says “Another way to generate the identifiers is to choose a
random 22-character alphanumeric string” and “alphanumeric” is defined at
https://www.merriam-webster.com/dictionary/alphanumeric as being letters and
numbers.  Unicode of course does not restrict “letters” or “numbers” to ascii.

Note that a UTF-8 character can be anywhere from 1 to 4 bytes long.  A 22
character string could thus be up to 88 bytes long.  The word “typically”
means you think ASCII (1 byte UTF-8) is typical.

If you meant ASCII back on page 17 (and I suspect you did), then say so
there.  Otherwise, change this to “characters”.

12) Section 3.4.3 says:
> including situations where the server does not recognize the PeerId

The server “recognizing” the PeerId implies that it compares it to existing
state, but PeerId is a utf-8 string.  Is comparison to be done byte-wise?
(I.e., it must match exactly, with no differences in NFC vs NFD, or
half-width vs full-width, or case, or punycode-encoded realm vs not, etc.)

If I understand correctly, the peerId is assigned by the server, and sent
back to the same server, and does not need to be entered by a human or be
in any specific form the peer would use natively, and so a byte-wise
comparison should suffice and you can say that explicitly.

13) Table 11 says, about SSIDList and Base64SSIDList:
> JSON array of UTF-8 encoded SSID strings. 
and similarly for SSID in table 12.  And appendix D says:
> If present, the peer MAY use this list as a hint to determine the
> networks where the EAP-NOOB association can be used for access
> authorization

This is pretty vague.   Is the SSID supposed to be normalized in any way
(normalization form, half-width vs full-width, etc.)?

Is the peer responsible for normalizing the SSIDList Unicode strings if it
needs to match the networks?  Or is the server responsible for sending them
in a normalized form that the AP expects?

Miscellaneous
-------------

14) Section 4.3 reserves space for Experimental Use.
https://tools.ietf.org/html/rfc8126#section-4.2 says:
“When code points are set aside for Experimental Use, it's important to make
clear any expected restrictions on experimental scope.  For example, say
whether it's acceptable to run experiments using those code points over
the open Internet or whether such experiments should be confined to more
closed environments.  See [RFC6994] for an example of such considerations.”

15) Table 11 says, about SSIDList:
> List of wireless network identifier (SSID)
> strings used for roaming support, ...                                 

Is table 11 specific to 802.11? Or is this suggesting that SSIDList is a
field not specific to 802.11 SSIDs? Same question on table 12.

16) Table 12 says MACAddress and BSSId are EUI-48 strings.
Are both upper case A-F and lower case a-f allowed?

17) Section 6.4 says:
> Without
> verification by the user or authentication with vendor certificates
> on the application level, the PeerInfo is not authenticated
> information and should not be relied on.

The PeerInfo can be anything, right?   So draft-ietf-rats-eat could be used
as the PeerInfo structure to provide attestability of the information for
example.  I'd suggest at least mentioning that attestation could be used
with the PeerInfo to provide such authentication.  (Optionally add an
informative reference to draft-ietf-rats-eat as a "for example", but ok if
you choose not to reference it.)