ietf-openpgp
[Top] [All Lists]

[openpgp] [RFC4880bis PATCH] Clarify CRC-24 C example implementation

2021-03-17 09:56:01
The mismatch between the variable in the sample CRC-24 generation code
and the definition of the generator in the paragraph above has been a
source of confusion for over 20 years (see [0] for the first instance
I could find in the wild).  It has also been raised as an explicit
erratum [1].

[0] https://mailarchive.ietf.org/arch/msg/openpgp/F29_03_iISnOS7DF0PUH-tVVKsI/

[1] https://www.rfc-editor.org/errata/eid5491

One option would be to change the CRC24_POLY variable in the code to
exactly match the generator described above.  This allows the `crc24`
accumulator to overflow, which on most architectures has has no
practical functional change, because of the masking step at the end of
the function.

However, allowing the `crc24` accumulator to overflow *might* cause a
problem in some obscure architecture, as (a) `crc24` is defined as
"long", which is typically a signed type, and (b) left-shift behavior
is undefined for overflowing signed values (and also for negative
signed values) [2].

[2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf § 6.5.7(4)

So allowing the accumulator to overflow in the sample code seems
ill-advised.  This change clarifies both that the accumulator should
be treated as unsigned, and aligns the variable with the textual
description of the generator.

It follows a suggestion first made by Hal Finney 22 years ago [3].

[3] https://mailarchive.ietf.org/arch/msg/openpgp/UMLreIiKtKzXEnPT5ZbcDDjRQX0/
---
 crypto-refresh.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto-refresh.md b/crypto-refresh.md
index 9fbb6eb..9f57a1a 100644
--- a/crypto-refresh.md
+++ b/crypto-refresh.md
@@ -2116,9 +2116,9 @@ The nonzero initialization can detect more errors than a 
zero initialization.
 ## An Implementation of the CRC-24 in "C"
 
     #define CRC24_INIT 0xB704CEL
-    #define CRC24_POLY 0x1864CFBL
+    #define CRC24_GENERATOR 0x864CFBL
 
-    typedef long crc24;
+    typedef unsigned long crc24;
     crc24 crc_octets(unsigned char *octets, size_t len)
     {
         crc24 crc = CRC24_INIT;
@@ -2128,7 +2128,7 @@ The nonzero initialization can detect more errors than a 
zero initialization.
             for (i = 0; i < 8; i++) {
                 crc <<= 1;
                 if (crc & 0x1000000)
-                    crc ^= CRC24_POLY;
+                    crc ^= (CRC24_GENERATOR | 0x1000000L);
             }
         }
         return crc & 0xFFFFFFL;
-- 
2.30.2

_______________________________________________
openpgp mailing list
openpgp(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/openpgp