Skip to main content

Last Call Review of draft-faltstrom-base45-10
review-faltstrom-base45-10-secdir-lc-rose-2022-02-07-00

Request Review of draft-faltstrom-base45
Requested revision No specific revision (document currently at 12)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2022-03-02
Requested 2022-02-02
Authors Patrik Fältström , Fredrik Ljunggren , Dirk-Willem van Gulik
I-D last updated 2022-02-07
Completed reviews Secdir Last Call review of -10 by Kyle Rose (diff)
Genart Last Call review of -10 by Robert Sparks (diff)
Artart Last Call review of -10 by Harald T. Alvestrand (diff)
Intdir Telechat review of -10 by Ron Bonica (diff)
Assignment Reviewer Kyle Rose
State Completed
Request Last Call review on draft-faltstrom-base45 by Security Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/secdir/YCDcZwPNRdWnDswT3Kv6NOGBYvI
Reviewed revision 10 (document currently at 12)
Result Has issues
Completed 2022-02-07
review-faltstrom-base45-10-secdir-lc-rose-2022-02-07-00
I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the security area directors.
 Document editors and WG chairs should treat these comments just like any other
last call comments.

This document has one Issue. Technically, the described protocol is Ready
primarily because it describes an existing encoding standardized in ISO/IEC
18004. That said, I can't really help but ask a few things about this encoding:

* Why 45? (This is probably really a question for the people who designed QR
codes.) 45^3 = 91125 >> 2^16, which means roughly 30% of strings with length
divisible by 3 comprising base45 characters are invalid encodings. * Why is
Space included as a character? This renders the encoding fragile in many string
handling contexts. * Base64 requires padding only to comply with the spec
as-written: a minor modification of the spec (let's call it base64') could lose
the padding and still produce unambiguous encodings of both content and length:
going from 64 to 45 doesn't have anything to do with remedying this. And, by
contrast with base45, 64^4=256^3, which means all strings with length divisible
by 4 comprising base64 characters would be valid base64' encodings.

But the real issue I have with the document (and one that might be technically
unavoidable, depending how ISO/IEC 18004 is worded) is that an actual covert
channel isn't addressed at all: that of the remaining insignificant bits in
encodings of odd-length strings (a problem that would similarly impact base64'
from above for encodings of strings with length mod 3 !≅ 0). Taking the example
from section 4.4, the final chunk ([33 0]) in the example comes from the 16-bit
value 33 (= 33 + 0 * 45). But all that is needed is that 33 ≅ X mod 256 unless
the decoder enforces that encodings of length not divisible by 3 have 16-bit
values < 256. For example, QED8WEJ6 ultimately decodes to the string "ietf!",
or to "ietf!1", or not at all, depending on the behavior of the decoder: ignore
non-zero second octet in odd-length strings (quite likely), render it based on
the available data (unlikely because, if not special-cased, would result in
decoding "ietf!0" in the example from the document), or reject the encoding.
The last option is the only valid thing to do from a security perspective.

Grammar nit: "For example, it MUST the encoded data"