Skip to main content

Last Call Review of draft-ietf-sframe-enc-06
review-ietf-sframe-enc-06-secdir-lc-wallace-2024-02-17-00

Request Review of draft-ietf-sframe-enc
Requested revision No specific revision (document currently at 09)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2024-02-15
Requested 2024-02-01
Authors Emad Omara , Justin Uberti , Sergio Garcia Murillo , Richard Barnes , Youenn Fablet
I-D last updated 2024-02-17
Completed reviews Genart Last Call review of -07 by Linda Dunbar (diff)
Secdir Last Call review of -06 by Carl Wallace (diff)
Artart Last Call review of -06 by Valery Smyslov (diff)
Intdir Telechat review of -07 by Suresh Krishnan (diff)
Assignment Reviewer Carl Wallace
State Completed
Request Last Call review on draft-ietf-sframe-enc by Security Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/secdir/E3Sb_jfrZpAZZ99CisCHiopjHTQ
Reviewed revision 06 (document currently at 09)
Result Has nits
Completed 2024-02-17
review-ietf-sframe-enc-06-secdir-lc-wallace-2024-02-17-00
This is a very well written document. A few minor comments are below.

In section 4.3, the language for Key or Key Length and Counter or Counter
Length is a little difficult to parse. Dropping the commas around "minus one"
would help or maybe splitting into two sentences. "If the X flag is set to 0,
this field contains the Key ID. Else, this field contains the key ID length
minus one."

In 4.4, the section title should probably be Encryption Scheme.

In 4.4.1, the first sentence of the fifth paragraph should start "A given
base_key MUST NOT be used...". There may be a few other places where "key" is
used where "base_key" would be more clear (like the "Adding a key for sending",
etc. in the example API, in the title of section 4.4.1, etc.).

In 4.4.2, there is a typo - encrytion.

Throughout, it may be worth indicating which pseudocode method names are not
expanded upon in the draft, i.e., encode_big_endian, encode_sframe_header.
Maybe prepend with an underscore and note that practice.

In table 1, given Nk is used as a table header, it may be best to introduce the
composite key notion in section 4.4 (even if only via a forward reference to
4.5.1). It's a bit confusing on a first read to see 48 for the first three rows
on a first read. Maybe defining Nka in 4.4.4 would be sufficient.

In section 9.1, if only one sender can use a base_key for encryption, why is
the KID noted as important to avoiding reuse of (base_key, KID, CTR)
combinations? Is the assignment of KIDs to ensure KID values are unique across
set of senders in addition to unique base_key values?