From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 19 Apr 2025 13:32:07 +0000 (-0400)
Subject: polyval: comments throughout.
X-Git-Tag: tor-0.4.9.3-alpha~49^2~10
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=01d934960c78c2a2bb6977debc737b5ed463b9ff;p=thirdparty%2Ftor.git

polyval: comments throughout.
---

diff --git a/src/ext/polyval/polyval.c b/src/ext/polyval/polyval.c
index 7c6df7ab08..e0e0c6d597 100644
--- a/src/ext/polyval/polyval.c
+++ b/src/ext/polyval/polyval.c
@@ -5,31 +5,88 @@
  * \file polyval.h
  * \brief Implementation for polyval universal hash function.
  *
- * XXXX write more.
+ * Polyval, which was first defined for AES-GCM-SIV, is a
+ * universal hash based on multiplication in GF(2^128).
+ * Unlike the more familiar GHASH, it is defined to work on
+ * little-endian inputs, and so is more straightforward and efficient
+ * on little-endian architectures.
+ *
+ * In Tor we use it as part of the Counter Galois Onion relay
+ * encryption format.
  **/
 
+/* Implementation notes:
+ *
+ * Our implementation is based on the GHASH code from BearSSL
+ * by Thomas Pornin.  There are three implementations:
+ *
+ * pclmul.c -- An x86-only version, based on the CLMUL instructions
+ * introduced for westlake processors in 2010.
+ *
+ * ctmul64.c -- A portable contant-time implementation for 64-bit
+ * processors.
+ *
+ * ctmul.c -- A portable constant-time implementation for 32-bit
+ * processors with an efficient 32x32->64 multiply instruction.
+ *
+ * Note that the "ctmul" implementations are only constant-time
+ * if the corresponding CPU multiply and rotate instructions are
+ * also constant-time.  But that's an architectural assumption we
+ * make in Tor.
+ */
+
 #include "ext/polyval/polyval.h"
 
 #include <string.h>
 
 typedef pv_u128_ u128;
 
+/* ========
+ * We declare these functions, to help implement polyval.
+ *
+ * They have different definitions depending on our representation
+ * of 128-bit integers.
+ */
+/**
+ * Read a u128-bit little-endian integer from 'bytes',
+ * which may not be aligned.
+ */
 static inline u128 u128_from_bytes(const uint8_t *bytes);
+/**
+ * Store a u128-bit little-endian integer to 'bytes_out',
+ * which may not be aligned.
+ */
 static inline void u128_to_bytes(u128, uint8_t *bytes_out);
-static inline void pv_xor(polyval_t *, u128);
+/**
+ * XOR a u128 into the y field of a polyval_t struct.
+ *
+ * (Within the polyval struct, perform "y ^= v").
+ */
+static inline void pv_xor_y(polyval_t *, u128 v);
+/**
+ * Initialize any derived fields in pv.
+ */
 static inline void pv_init_extra(polyval_t *pv);
 
-/* Functions which we expect our multiply implementation to declare. */
+/* ========
+ * The function which we expect our backend to implement.
+ */
 /**
  * Within the polyval struct, perform "y *= h".
+ *
+ * (This is a carryless multiply in the Polyval galois field)
  */
 static void pv_mul_y_h(polyval_t *);
 
+/* =====
+ * Endianness conversion for big-endian platforms
+ */
 #ifdef WORDS_BIG_ENDIAN
 #ifdef __GNUC__
 #define bswap64(x) __builtin_bswap64(x)
 #define bswap32(x) __builtin_bswap32(x)
 #else
+/* The (compiler should optimize these into a decent bswap instruction) */
 static inline uint64_t
 bswap64(uint64_t v)
 {
@@ -63,7 +120,6 @@ bswap32(uint64_t v)
 #define convert_byte_order32(x) (x)
 #endif
 
-
 #ifdef PV_USE_PCLMUL
 
 #include "ext/polyval/pclmul.c"
@@ -79,7 +135,7 @@ u128_to_bytes(u128 val, uint8_t *bytes_out)
   _mm_storeu_si128((u128*)bytes_out, val);
 }
 static inline void
-pv_xor(polyval_t *pv, u128 v)
+pv_xor_y(polyval_t *pv, u128 v)
 {
   pv->y = _mm_xor_si128(pv->y, v);
 }
@@ -111,7 +167,7 @@ u128_to_bytes(u128 val, uint8_t *bytes_out)
   memcpy(bytes_out + 8, &hi, 8);
 }
 static inline void
-pv_xor(polyval_t *pv, u128 val)
+pv_xor_y(polyval_t *pv, u128 val)
 {
   pv->y.lo ^= val.lo;
   pv->y.hi ^= val.hi;
@@ -145,7 +201,7 @@ u128_to_bytes(u128 val, uint8_t *bytes_out)
   memcpy(bytes_out, v, 16);
 }
 static inline void
-pv_xor(polyval_t *pv, u128 val)
+pv_xor_y(polyval_t *pv, u128 val)
 {
   for (int i = 0; i < 4; ++i) {
     pv->y.v[i] ^= val.v[i];
@@ -169,7 +225,7 @@ void
 polyval_add_block(polyval_t *pv, const uint8_t *block)
 {
   u128 b = u128_from_bytes(block);
-  pv_xor(pv, b);
+  pv_xor_y(pv, b);
   pv_mul_y_h(pv);
 }
 void
diff --git a/src/ext/polyval/polyval.h b/src/ext/polyval/polyval.h
index 5072f05619..3a137794c6 100644
--- a/src/ext/polyval/polyval.h
+++ b/src/ext/polyval/polyval.h
@@ -69,13 +69,39 @@ typedef struct polyval_t {
   pv_u128_ y;
 } polyval_t;
 
+/**
+ * Length of a polyval key, in bytes.
+ */
 #define POLYVAL_KEY_LEN 16
+/**
+ * Length of a polyval block, in bytes.
+ */
 #define POLYVAL_BLOCK_LEN 16
+/**
+ * Length of a polyval tag (output), in bytes.
+ */
 #define POLYVAL_TAG_LEN 16
 
+/**
+ * Initialize a polyval instance with a given key.
+ */
 void polyval_init(polyval_t *, const uint8_t *key);
+/**
+ * Update a polyval instance with a new 16-byte block.
+ */
 void polyval_add_block(polyval_t *, const uint8_t *block);
+/**
+ * Update a polyval instance with 'n' bytes from 'data'.
+ * If 'n' is not evenly divisible by 16, pad it at the end with zeros.
+ *
+ * NOTE: This is not a general-purpose padding construction;
+ * it can be insecure if your are using it in context where the input length
+ * is variable.
+ */
 void polyval_add_zpad(polyval_t *, const uint8_t *data, size_t n);
+/**
+ * Copy the 16-byte tag from a polyval instance into 'tag_out'
+ */
 void polyval_get_tag(const polyval_t *, uint8_t *tag_out);
 /**
  * Reset a polyval instance to its original state,
@@ -83,5 +109,4 @@ void polyval_get_tag(const polyval_t *, uint8_t *tag_out);
  */
 void polyval_reset(polyval_t *);
 
-
 #endif