[core] Adam Roach's Discuss on draft-ietf-core-object-security-09: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

Adam Roach has entered the following ballot position for
draft-ietf-core-object-security-09: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-core-object-security/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

§5 contains the following uses of "SHOULD NOT":

>     *  The 'Partial IV' parameter.  The value is set to the Sender
>        Sequence Number.  All leading zeroes SHALL be removed when
>        encoding the Partial IV.  The value 0 encodes to the byte
>        string 0x00.  This parameter SHALL be present in requests.  In
>        case of Observe (Section 4.2.3.4) the Partial IV SHALL be
>        present in responses, and otherwise the Partial IV SHOULD NOT
>        be present in responses.  (A non-Observe example where the
>        Partial IV is included in a response is provided in
>        Section 7.5.2.)
>
>     *  The 'kid' parameter.  The value is set to the Sender ID.  This
>        parameter SHALL be present in requests and SHOULD NOT be
>        present in responses.  An example where the Sender ID is
>        included in a response is the extension of OSCORE to group
>        communication [I-D.tiloca-core-multicast-oscoap].

As far as I can tell, both "SHOULD NOT" instances describe behavior that is
unnecessary but benign. This usage is inconsistent with the definition of
"SHOULD NOT" in RFC 2119:

4. SHOULD NOT   This phrase, or the phrase "NOT RECOMMENDED" mean that
   there may exist valid reasons in particular circumstances when the
   particular behavior is acceptable or even useful, but the full
   implications should be understood and the case carefully weighed
   before implementing any behavior described with this label.

If the implications are simply "this is unnecessary but benign," then
implementors have no real implications to weigh, and the described behavior
doesn't rise to the level of "SHOULD NOT". If the implications are stronger
than that, then *this* document doesn't contain enough information for
implementors to perform such an evaluation.

In the former case, you can clear this discuss by changing "SHOULD NOT" to "will
not typically". In the latter case, you can clear this discuss by adding enough
information for implementors to be able to make an educated weighing of
implications. I'm also open to other proposals that remedy this use of 2119
language.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I support EKR's DISCUSS.

Martin Thompson raises a number of fairly important points in his review (see
<https://mailarchive.ietf.org/arch/msg/ietf/xtYOveSgAf32EoHVOV_E08brN54>). I
recognize that many of these are fundamental to the design in the document. It
is still worthwhile thinking through them and trying to suss out whether a
redesign by the working group is warranted.

I do want to put a little more meat on Martin's assertion "This doesn't appear
to have any supporting security analysis," as this was something I was going
to highlight myself (and this is related to EKR's DISCUSS as well). Given that
this document seems to be putting together security primitives in a de novo
fashion, I would expect to see something equivalent to draft-ietf-tls-tls13's
Appendix E.

Specific comments follow.

---------------------------------------------------------------------------

§1:

>  ([RFC6347]) for security.  CoAP and HTTP proxies require (D)TLS to be
>  terminated at the proxy

Presumably, this means "CoAP-to-HTTP proxies"? Otherwise, the assertion is
wrong: HTTP proxies do not terminate TLS connections.

---------------------------------------------------------------------------
§1:

>  An implementation supporting this specification MAY only implement
>  the client part, MAY only implement the server part, or MAY only
>  implement one of the proxy parts

Replace "MAY only implement" with "MAY implement only" (in three places)

---------------------------------------------------------------------------

§1.1:

>  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>  "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>  document are to be interpreted as described in [RFC2119].  These
>  words may also appear in this document in lowercase, absent their
>  normative meanings.

This is almost, but not quite, the RFC 8174 boilerplate. Please fix it to match
RFC 8174.

---------------------------------------------------------------------------

§2:

>  +-----+---+---+---+---+-----------------+--------+--------+---------+
>  | No. | C | U | N | R | Name            | Format | Length | Default |
>  +-----+---+---+---+---+-----------------+--------+--------+---------+
>  | TBD | x |   |   |   | Object-Security |  (*)   | 0-255  | (none)  |
>  +-----+---+---+---+---+-----------------+--------+--------+---------+
>      C = Critical,   U = Unsafe,   N = NoCacheKey,   R = Repeatable
>      (*) See below.
>
>                  Figure 3: The Object-Security Option

I get the impression that this is supposed to be extending a table that already
exists somewhere. This document should say which table.

---------------------------------------------------------------------------

§4.1:

>  The CoAP Payload, if present in the original CoAP message, SHALL be
>  encrypted and integrity protected and is thus an Inner message field.
>  See Figure 5.
>
>                      +------------------+---+---+
>                      | Field            | E | U |
>                      +------------------+---+---+
>                      | Payload          | x |   |
>                      +------------------+---+---+
>
>                E = Encrypt and Integrity Protect (Inner)
>                U = Unprotected (Outer)
>
>                  Figure 5: Protection of CoAP Payload

The figure adds nothing to the prose; and is, in fact, harder to understand than
the prose. I would recommend removing it.

---------------------------------------------------------------------------

§4.3:

>  The other CoAP Header fields are Unprotected (Class U).

Presumably this should say "The other currently defined CoAP Header fields...",
right?


---------------------------------------------------------------------------

§5:

>  As specified in [RFC5116], plaintext denotes the data that is
>  encrypted and integrity protected...

Traditionally, data that is encrypted is called "cipher text." I gather from
context that this should probably say "...the data that is to be encrypted...",
right?

---------------------------------------------------------------------------

§5.2:

>  responses, which reduces the size.  For processing instructions (see
>  Section 8).

This final fragment can be turned into a sentence by removing the parentheses.


---------------------------------------------------------------------------

§6 and its subsections:

The use of a bespoke profile of COSE adds implementation complexity to
ostenstibly resource-limited device for what appears to be very little gain. In
the examples given, savings of 4 to 7 bytes are demonstrated, which seems to
hardly warrant the addition of this mechanism. These do not appear to be
degenerate cases, so I can't imagine that compression performance under
real-world conditions would be much better.  Was there an explicit discussion
in the working group regarding this complexity/wire-size trade-off?

---------------------------------------------------------------------------

§6.3.1:

>  1.  Request with kid = 25 and Partial IV = 5

The base of the numbers above isn't indicated, and the reasonable reader may
take it to be 10. Please either change the above line to "...kid = 0x25...",
or change the hex encodings in the examples to h'19'.

---------------------------------------------------------------------------

§7.3:

>  For requests, OSCORE provides weak absolute freshness as the only

The meaning of "weak absolute freshness" doesn't appear to be given anywhere,
and is not evident by combining the normal meanings of those three words. Please
describe the property of "weak absolute freshness" in more detail (or, if this
is a term of art defined elsewhere, a citation would be sufficient).

--------------------------------------------------------------------------

§10.3:

>  o  "" (empty string) if the CoAP Object-Security option is empty, or

Which is it? Is this an empty string on the wire, or is it a string consisting
of "" (that is, the two-byte sequence 0x22 0x22)?

---------------------------------------------------------------------------

§10.3:

>[HTTP request -- Before client object security processing]

This line should be indented to match the rest of the example.

---------------------------------------------------------------------------

§10.3:

>  o  the value of the CoAP Object-Security option (Section 6.1) in
>     base64url encoding (Section 5 of [RFC4648]) without padding (see
>     [RFC7515] Appendix C for implementation notes for this encoding).

...

>    POST http://proxy.url/hc/?target_uri=coap://server.url/ HTTP/1.1
>    Content-Type: application/oscore
>    CoAP-Object-Security: 09 25

The first block of text defines CoAP-Object-Security as a Base64 string. The
second shows an example string of hex digits. Please either redefine the
syntax in the first block, or show a matching syntax in the examples.

This comment applies to §10.4 also.