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"