ietf-openpgp
[Top] [All Lists]

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

2021-03-18 03:50:59
On Wed, 17 Mar 2021 22:54, Daniel Kahn Gillmor said:

i'd be fine with either one.  afaict, "unsigned long" is always at least
at least a 32-bit unsigned integer, and is more typical idiomatic C, but
if the WG prefers uint_least32_t i would not object.  (technically the

This is a protocol, specification and not an implementation description.
Thus this should be considred as Pseudo code.

Anyway, I am in favor of applying Hal Finney's old suggestion (patch 39,
attached).


Salam-Shalom,

   Werner


-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.
From 998f55481cd4101eefe45350ded408dbfdd16438 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <dkg(_at_)fifthhorseman(_dot_)net>
Date: Wed, 17 Mar 2021 09:54:16 -0400
Subject: [PATCH] Clarify CRC-24 C example implementation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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;
-- 
GitLab

Attachment: signature.asc
Description: PGP signature

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