serpent: Provide little-endian version too, but ours is big
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Thu, 25 Jul 2013 17:30:48 +0000 (18:30 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Thu, 25 Jul 2013 17:30:48 +0000 (18:30 +0100)
Apparently, almost everyone else is using little-endian Serpent.

Since we are going to introduce a new transform, we can make our new
transform have conventional little-endian Serpent.  But we need to
retain the big-endian one for compatibility.

In this patch:
 * Make serpent.h compile-time bytesexual by providing a new definition
   of GETPUT_CP.  We default to (conventional) little-endian.
 * The big-endian and little-endian versions have different names:
   decorate the function names.  serpent.h declares both sets.
 * Provide serpentbe.c which compiles to a .o implementing the big-endian
   version under the new names.  Build only that one.
 * Change all the call sites to use the big-endian Serpent.
 * Mention this issue in the documentation.

Ultimately, no functional change in this patch.

Regarding the endianness of Serpent, Mark Wooding writes:

  I've understood how my Serpent implementation differs from Secnet's, and
  have reproduced [Ian's EAX] test vectors.

  NIST have managed to completely screw up their archive of the AES
  contest pages, but the WayBack Machine works fine -- even on the various
  PDF documents.

  The sorry tale begins with the initial Request for Candidate Algorithm
  Nominations, wherein the specification[1] for the test files unhelpfully
  muddles things by implying a big-endian representation in the test
  vector files, e.g., in 3.1,

  : Each of the possible key basis vectors is tested in this manner, by
  : shifting the "1" a single position at a time, starting at the most
  : significant (left-most) bit position of the key.

  It doesn't help that the original Serpent C implementations in the
  submission package[2] failed to implement the defined API correctly, and
  implicitly coerced BYTE pointer (from the defined entry point
  `blockEncrypt' to an `unsigned long' pointer as an argument to the
  internal `serpent_encrypt' function.  The Java version carefully picks
  words out of its input byte array in a little-endian order.

  Secnet's implementation is derived from this original reference
  implementation.  Originally, (v0.03) it had the same bug (only with
  `uint8_t' and `uint32_t'), but was patched (v0.1.16) to correct the
  obvious dependency on host endianness.  Unfortunately, this patch is
  wrong: it creates a perfectly portable completely-byte-reversed Serpent
  compatible with nothing else.  I've not found many other
  implementations, but where I have, they agree with me and the Java
  reference, and not with Secnet.

  [1] http://web.archive.org/web/20000420040415/http://csrc.nist.gov/encryption/aes/katmct/katmct.htm
  [2] http://www.cl.cam.ac.uk/~rja14/Papers/serpent.tar.gz

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Makefile.in
secnet.8
serpent.c
serpent.h
serpentbe.c [new file with mode: 0644]
transform.c

index 182204b..d4d3815 100644 (file)
@@ -54,7 +54,7 @@ TARGETS:=secnet
 
 OBJECTS:=secnet.o util.o conffile.yy.o conffile.tab.o conffile.o modules.o \
        resolver.o random.o udp.o site.o transform.o netlink.o rsa.o dh.o \
-       serpent.o md5.o version.o tun.o slip.o sha1.o ipaddr.o log.o \
+       serpentbe.o md5.o version.o tun.o slip.o sha1.o ipaddr.o log.o \
        process.o @LIBOBJS@ \
        hackypar.o
 
index 869d297..ef07a76 100644 (file)
--- a/secnet.8
+++ b/secnet.8
@@ -429,7 +429,9 @@ It may be necessary to increase this is if connectivity is poor.
 A \fItransform closure\fR is a reversible means of transforming
 messages for transmission over a (presumably) insecure network.
 It is responsible for both confidentiality and integrity.
-
+.PP
+Note that this uses a big-endian variant of the Serpent block cipher
+(which is not compatible with most other Serpent implementations).
 .SS rsa-private
 \fBrsa-private(\fIPATH\fB\fR[, \fICHECK\fR]\fB)\fR => \fIrsaprivkey closure\fR
 .TP
index 3847437..6407c57 100644 (file)
--- a/serpent.c
+++ b/serpent.c
 #include "serpent.h"
 #include "serpentsboxes.h"
 
+#ifdef SERPENT_BIGENDIAN
+
 #define GETPUT_CP(bytenum) \
     (((basep) + (lenbytes) - (offset) - 4)[(bytenum)])
 
+#define SERPENT_DECORATE(func) serpentbe_##func
+
+#else /* !defined(SERPENT_BIGENDIAN) */
+
+#define GETPUT_CP(bytenum) \
+    (((basep) + (offset))[3-(bytenum)])
+
+#define SERPENT_DECORATE(func) serpent_##func
+
+#endif /* !defined(SERPENT_BIGENDIAN) */
+
 static uint32_t serpent_get_32bit(const uint8_t *basep,
                                  int lenbytes, int offset)
 {
@@ -45,7 +58,7 @@ static void serpent_put_32bit(uint8_t *basep, int lenbytes, int offset, uint32_t
     GETPUT_CP(3) = (char)(value);
 }
 
-void serpent_makekey(struct keyInstance *key, int keyLen,
+void SERPENT_DECORATE(makekey)(struct keyInstance *key, int keyLen,
            const uint8_t *keyMaterial)
 {
     int i;
@@ -105,7 +118,7 @@ void serpent_makekey(struct keyInstance *key, int keyLen,
            key->subkeys[i][j] = k[4*i+j];
 }
 
-void serpent_encrypt(struct keyInstance *key,
+void SERPENT_DECORATE(encrypt)(struct keyInstance *key,
                     const uint8_t plaintext[16], 
                     uint8_t ciphertext[16])
 {
@@ -223,7 +236,7 @@ void serpent_encrypt(struct keyInstance *key,
     serpent_put_32bit(ciphertext,16,12, x3);
 }
 
-void serpent_decrypt(struct keyInstance *key,
+void SERPENT_DECORATE(decrypt)(struct keyInstance *key,
                     const uint8_t ciphertext[16],
                     uint8_t plaintext[16])
 {
index 19c01fe..857b1b9 100644 (file)
--- a/serpent.h
+++ b/serpent.h
@@ -9,11 +9,17 @@ struct keyInstance {
 /*  Function protoypes  */
 void serpent_makekey(struct keyInstance *key, int keyLen,
                     const uint8_t *keyMaterial);
+void serpentbe_makekey(struct keyInstance *key, int keyLen,
+                    const uint8_t *keyMaterial);
 
 void serpent_encrypt(struct keyInstance *key, const uint8_t plaintext[16],
                     uint8_t ciphertext[16]);
+void serpentbe_encrypt(struct keyInstance *key, const uint8_t plaintext[16],
+                    uint8_t ciphertext[16]);
 
 void serpent_decrypt(struct keyInstance *key, const uint8_t ciphertext[16],
                     uint8_t plaintext[16]);
+void serpentbe_decrypt(struct keyInstance *key, const uint8_t ciphertext[16],
+                    uint8_t plaintext[16]);
 
 #endif /* serpent_h */
diff --git a/serpentbe.c b/serpentbe.c
new file mode 100644 (file)
index 0000000..758d5c9
--- /dev/null
@@ -0,0 +1,2 @@
+#define SERPENT_BIGENDIAN
+#include "serpent.c"
index 08ddad6..a0665ad 100644 (file)
@@ -57,8 +57,8 @@ static bool_t transform_setkey(void *sst, uint8_t *key, int32_t keylen)
     }
 #endif /* 0 */
 
-    serpent_makekey(&ti->cryptkey,256,key);
-    serpent_makekey(&ti->mackey,256,key+32);
+    serpentbe_makekey(&ti->cryptkey,256,key);
+    serpentbe_makekey(&ti->mackey,256,key+32);
     ti->cryptiv=get_uint32(key+64);
     ti->maciv=get_uint32(key+68);
     ti->sendseq=get_uint32(key+72);
@@ -122,7 +122,7 @@ static uint32_t transform_forward(void *sst, struct buffer_if *buf,
        message stays a multiple of 16 bytes long.) */
     memset(iv,0,16);
     put_uint32(iv, ti->maciv);
-    serpent_encrypt(&ti->mackey,iv,macacc);
+    serpentbe_encrypt(&ti->mackey,iv,macacc);
 
     /* CBCMAC: encrypt in CBC mode. The MAC is the last encrypted
        block encrypted once again. */
@@ -130,16 +130,16 @@ static uint32_t transform_forward(void *sst, struct buffer_if *buf,
     {
        for (i = 0; i < 16; i++)
            macplain[i] = macacc[i] ^ n[i];
-       serpent_encrypt(&ti->mackey,macplain,macacc);
+       serpentbe_encrypt(&ti->mackey,macplain,macacc);
     }
-    serpent_encrypt(&ti->mackey,macacc,macacc);
+    serpentbe_encrypt(&ti->mackey,macacc,macacc);
     memcpy(buf_append(buf,16),macacc,16);
 
     /* Serpent-CBC. We expand the ID as for CBCMAC, do the encryption,
        and prepend the IV before increasing it. */
     memset(iv,0,16);
     put_uint32(iv, ti->cryptiv);
-    serpent_encrypt(&ti->cryptkey,iv,iv);
+    serpentbe_encrypt(&ti->cryptkey,iv,iv);
 
     /* CBC: each block is XORed with the previous encrypted block (or the IV)
        before being encrypted. */
@@ -149,7 +149,7 @@ static uint32_t transform_forward(void *sst, struct buffer_if *buf,
     {
        for (i = 0; i < 16; i++)
            n[i] ^= p[i];
-       serpent_encrypt(&ti->cryptkey,n,n);
+       serpentbe_encrypt(&ti->cryptkey,n,n);
        p=n;
     }
 
@@ -194,12 +194,12 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
        *errmsg="msg not multiple of cipher blocksize";
        return 1;
     }
-    serpent_encrypt(&ti->cryptkey,iv,iv);
+    serpentbe_encrypt(&ti->cryptkey,iv,iv);
     for (n=buf->start; n<buf->start+buf->size; n+=16)
     {
        for (i = 0; i < 16; i++)
            pct[i] = n[i];
-       serpent_decrypt(&ti->cryptkey,n,n);
+       serpentbe_decrypt(&ti->cryptkey,n,n);
        for (i = 0; i < 16; i++)
            n[i] ^= iv[i];
        memcpy(iv, pct, 16);
@@ -209,7 +209,7 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
     macexpected=buf_unappend(buf,16);
     memset(iv,0,16);
     put_uint32(iv, ti->maciv);
-    serpent_encrypt(&ti->mackey,iv,macacc);
+    serpentbe_encrypt(&ti->mackey,iv,macacc);
 
     /* CBCMAC: encrypt in CBC mode. The MAC is the last encrypted
        block encrypted once again. */
@@ -217,9 +217,9 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
     {
        for (i = 0; i < 16; i++)
            macplain[i] = macacc[i] ^ n[i];
-       serpent_encrypt(&ti->mackey,macplain,macacc);
+       serpentbe_encrypt(&ti->mackey,macplain,macacc);
     }
-    serpent_encrypt(&ti->mackey,macacc,macacc);
+    serpentbe_encrypt(&ti->mackey,macacc,macacc);
     if (!consttime_memeq(macexpected,macacc,16)!=0) {
        *errmsg="invalid MAC";
        return 1;
@@ -327,8 +327,9 @@ void transform_module(dict_t *dict)
     /*
      * Serpent self-test.
      * 
-     * This test pattern is taken directly from the Serpent test
-     * vectors, to ensure we have all endianness issues correct. -sgt
+     * This test pattern was taken directly from the Serpent test
+     * vectors, which results in a big-endian Serpent which is not
+     * compatible with other implementations.
      */
 
     /* Serpent self-test */
@@ -336,18 +337,18 @@ void transform_module(dict_t *dict)
            "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\xbb\xcc\xdd\xee\xff"
            "\xff\xee\xdd\xcc\xbb\xaa\x99\x88\x77\x66\x55\x44\x33\x22\x11\x00",
            32);
-    serpent_makekey(&k,256,data);
+    serpentbe_makekey(&k,256,data);
 
     memcpy(plaintext,
            "\x01\x23\x45\x67\x89\xab\xcd\xef\xfe\xdc\xba\x98\x76\x54\x32\x10",
            16);
-    serpent_encrypt(&k,plaintext,ciphertext);
+    serpentbe_encrypt(&k,plaintext,ciphertext);
 
     if (memcmp(ciphertext, "\xca\x7f\xa1\x93\xe3\xeb\x9e\x99"
                "\xbd\x87\xe3\xaf\x3c\x9a\xdf\x93", 16)) {
        fatal("transform_module: serpent failed self-test (encrypt)");
     }
-    serpent_decrypt(&k,ciphertext,plaintext);
+    serpentbe_decrypt(&k,ciphertext,plaintext);
     if (memcmp(plaintext, "\x01\x23\x45\x67\x89\xab\xcd\xef"
                "\xfe\xdc\xba\x98\x76\x54\x32\x10", 16)) {
        fatal("transform_module: serpent failed self-test (decrypt)");