From af43f0b77a10716921d13c047d1d3c39570cae17 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 25 Jul 2013 18:30:48 +0100 Subject: [PATCH] serpent: Provide little-endian version too, but ours is big 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 --- Makefile.in | 2 +- secnet.8 | 4 +++- serpent.c | 19 ++++++++++++++++--- serpent.h | 6 ++++++ serpentbe.c | 2 ++ transform.c | 35 ++++++++++++++++++----------------- 6 files changed, 46 insertions(+), 22 deletions(-) create mode 100644 serpentbe.c diff --git a/Makefile.in b/Makefile.in index 182204b..d4d3815 100644 --- a/Makefile.in +++ b/Makefile.in @@ -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 diff --git a/secnet.8 b/secnet.8 index 869d297..ef07a76 100644 --- 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 diff --git a/serpent.c b/serpent.c index 3847437..6407c57 100644 --- a/serpent.c +++ b/serpent.c @@ -25,9 +25,22 @@ #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]) { diff --git a/serpent.h b/serpent.h index 19c01fe..857b1b9 100644 --- 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 index 0000000..758d5c9 --- /dev/null +++ b/serpentbe.c @@ -0,0 +1,2 @@ +#define SERPENT_BIGENDIAN +#include "serpent.c" diff --git a/transform.c b/transform.c index 08ddad6..a0665ad 100644 --- a/transform.c +++ b/transform.c @@ -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; nstart+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)"); -- 2.30.2