poly1305 internals: Eliminate "opaque" poly1305_state hack.

Cargo.toml

@@ -111,7 +111,6 @@ include = [
     "include/ring-core/base.h",
     "include/ring-core/check.h",
     "include/ring-core/mem.h",
-    "include/ring-core/poly1305.h",
     "include/ring-core/target.h",
     "include/ring-core/type_check.h",
     "src/**/*.rs",

crypto/poly1305/poly1305.c

@@ -16,10 +16,10 @@
 // (https://github.com/floodyberry/poly1305-donna) and released as public
 // domain.
 
-#include <ring-core/poly1305.h>
+#include <ring-core/base.h>
 
 #include "../internal.h"
-
+#include "ring-core/check.h"
 
 #if defined(__GNUC__) || defined(__clang__)
 #pragma GCC diagnostic ignored "-Wsign-conversion"
@@ -28,28 +28,22 @@
 
 static uint64_t mul32x32_64(uint32_t a, uint32_t b) { return (uint64_t)a * b; }
 
+// Keep in sync with `poly1305_state_st` in ffi_fallback.rs.
 struct poly1305_state_st {
-  uint32_t r0, r1, r2, r3, r4;
+  alignas(64) uint32_t r0;
+  uint32_t r1, r2, r3, r4;
   uint32_t s1, s2, s3, s4;
   uint32_t h0, h1, h2, h3, h4;
   uint8_t key[16];
 };
 
-OPENSSL_STATIC_ASSERT(
-    sizeof(struct poly1305_state_st) + 63 <= sizeof(poly1305_state),
-    "poly1305_state isn't large enough to hold aligned poly1305_state_st");
-
-static inline struct poly1305_state_st *poly1305_aligned_state(
-    poly1305_state *state) {
-  dev_assert_secret(((uintptr_t)state & 63) == 0);
-  return (struct poly1305_state_st *)(((uintptr_t)state + 63) & ~63);
-}
-
 // poly1305_blocks updates |state| given some amount of input data. This
 // function may only be called with a |len| that is not a multiple of 16 at the
 // end of the data. Otherwise the input must be buffered into 16 byte blocks.
 static void poly1305_update(struct poly1305_state_st *state, const uint8_t *in,
                             size_t len) {
+  debug_assert_nonsecret((uintptr_t)state % 64 == 0);
+
   uint32_t t0, t1, t2, t3;
   uint64_t t[5];
   uint32_t b;
@@ -142,8 +136,9 @@ static void poly1305_update(struct poly1305_state_st *state, const uint8_t *in,
   goto poly1305_donna_mul;
 }
 
-void CRYPTO_poly1305_init(poly1305_state *statep, const uint8_t key[32]) {
-  struct poly1305_state_st *state = poly1305_aligned_state(statep);
+void CRYPTO_poly1305_init(struct poly1305_state_st *state, const uint8_t key[32]) {
+  debug_assert_nonsecret((uintptr_t)state % 64 == 0);
+
   uint32_t t0, t1, t2, t3;
 
   t0 = CRYPTO_load_u32_le(key + 0);
@@ -180,10 +175,8 @@ void CRYPTO_poly1305_init(poly1305_state *statep, const uint8_t key[32]) {
   OPENSSL_memcpy(state->key, key + 16, sizeof(state->key));
 }
 
-void CRYPTO_poly1305_update(poly1305_state *statep, const uint8_t *in,
+void CRYPTO_poly1305_update(struct poly1305_state_st *state, const uint8_t *in,
                             size_t in_len) {
-  struct poly1305_state_st *state = poly1305_aligned_state(statep);
-
   // Work around a C language bug. See https://crbug.com/1019588.
   if (in_len == 0) {
     return;
@@ -192,8 +185,7 @@ void CRYPTO_poly1305_update(poly1305_state *statep, const uint8_t *in,
   poly1305_update(state, in, in_len);
 }
 
-void CRYPTO_poly1305_finish(poly1305_state *statep, uint8_t mac[16]) {
-  struct poly1305_state_st *state = poly1305_aligned_state(statep);
+void CRYPTO_poly1305_finish(struct poly1305_state_st *state, uint8_t mac[16]) {
   uint32_t g0, g1, g2, g3, g4;
   uint32_t b, nb;
 

crypto/poly1305/poly1305_arm.c

@@ -15,18 +15,20 @@
 // This implementation was taken from the public domain, neon2 version in
 // SUPERCOP by D. J. Bernstein and Peter Schwabe.
 
-#include <ring-core/poly1305.h>
+#include <ring-core/base.h>
 
 #include "../internal.h"
 
 
 #pragma GCC diagnostic ignored "-Wsign-conversion"
-#pragma GCC diagnostic ignored "-Wcast-align"
 
+// Keep in sync with ffi_arm_neon.rs
 typedef struct {
   uint32_t v[12];  // for alignment; only using 10
 } fe1305x2;
 
+// TODO: static asserts on sizes and alignments of fe1305x2.
+
 #define addmulmod openssl_poly1305_neon2_addmulmod
 #define blocks openssl_poly1305_neon2_blocks
 
@@ -174,23 +176,26 @@ static void fe1305x2_frombytearray(fe1305x2 *r, const uint8_t *x, size_t xlen) {
 
 static const alignas(16) fe1305x2 zero;
 
+// Keep in sync with ffi_arm_neon.rs
 struct poly1305_state_st {
-  uint8_t data[sizeof(fe1305x2[5]) + 128];
+  alignas(16) fe1305x2 r;
+  fe1305x2 h;
+  fe1305x2 c;
+  fe1305x2 precomp[2];
+
+  uint8_t data[128];
+
   uint8_t buf[32];
   size_t buf_used;
   uint8_t key[16];
 };
 
-OPENSSL_STATIC_ASSERT(
-    sizeof(struct poly1305_state_st) + 63 <= sizeof(poly1305_state),
-    "poly1305_state isn't large enough to hold aligned poly1305_state_st.");
+// TODO: static asserts on sizes and alignments of fe1305x2.
 
-void CRYPTO_poly1305_init_neon(poly1305_state *state, const uint8_t key[32]) {
-  struct poly1305_state_st *st = (struct poly1305_state_st *)(state);
-  fe1305x2 *const r = (fe1305x2 *)(st->data + (15 & (-(int)st->data)));
-  fe1305x2 *const h = r + 1;
-  fe1305x2 *const c = h + 1;
-  fe1305x2 *const precomp = c + 1;
+void CRYPTO_poly1305_init_neon(struct poly1305_state_st *st, const uint8_t key[32]) {
+  fe1305x2 *const r = &st->r;
+  fe1305x2 *const h = &st->h;
+  fe1305x2 *const precomp = &st->precomp[0];
 
   r->v[1] = r->v[0] = 0x3ffffff & load32(key);
   r->v[3] = r->v[2] = 0x3ffff03 & (load32(key + 3) >> 2);
@@ -209,13 +214,11 @@ void CRYPTO_poly1305_init_neon(poly1305_state *state, const uint8_t key[32]) {
   st->buf_used = 0;
 }
 
-void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in,
+void CRYPTO_poly1305_update_neon(struct poly1305_state_st *st, const uint8_t *in,
                                  size_t in_len) {
-  struct poly1305_state_st *st = (struct poly1305_state_st *)(state);
-  fe1305x2 *const r = (fe1305x2 *)(st->data + (15 & (-(int)st->data)));
-  fe1305x2 *const h = r + 1;
-  fe1305x2 *const c = h + 1;
-  fe1305x2 *const precomp = c + 1;
+  fe1305x2 *const h = &st->h;
+  fe1305x2 *const c = &st->c;
+  fe1305x2 *const precomp = &st->precomp[0];
 
   if (st->buf_used) {
     size_t todo = 32 - st->buf_used;
@@ -257,12 +260,11 @@ void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in,
   }
 }
 
-void CRYPTO_poly1305_finish_neon(poly1305_state *state, uint8_t mac[16]) {
-  struct poly1305_state_st *st = (struct poly1305_state_st *)(state);
-  fe1305x2 *const r = (fe1305x2 *)(st->data + (15 & (-(int)st->data)));
-  fe1305x2 *const h = r + 1;
-  fe1305x2 *const c = h + 1;
-  fe1305x2 *const precomp = c + 1;
+void CRYPTO_poly1305_finish_neon(struct poly1305_state_st *st, uint8_t mac[16]) {
+  fe1305x2 *const r = &st->r;
+  fe1305x2 *const h = &st->h;
+  fe1305x2 *const c = &st->c;
+  fe1305x2 *const precomp = &st->precomp[0];
 
   addmulmod(h, h, precomp, &zero);
 

src/aead/poly1305.rs

@@ -38,14 +38,6 @@ impl Key {
     }
 }
 
-// Keep in sync with `poly1305_state` in ring-core/poly1305.h.
-//
-// The C code, in particular the way the `poly1305_aligned_state` functions
-// are used, is only correct when the state buffer is 64-byte aligned.
-#[repr(C, align(64))]
-struct poly1305_state([u8; OPAQUE_LEN]);
-const OPAQUE_LEN: usize = 512;
-
 pub(super) enum Context {
     #[cfg(all(target_arch = "arm", target_endian = "little"))]
     ArmNeon(ffi_arm_neon::State),

src/aead/poly1305/ffi_arm_neon.rs

@@ -15,23 +15,58 @@
 
 #![cfg(all(target_arch = "arm", target_endian = "little"))]
 
-use super::{poly1305_state, Key, Tag, KEY_LEN, OPAQUE_LEN, TAG_LEN};
+use super::{Key, Tag, KEY_LEN, TAG_LEN};
 use crate::{c, cpu::arm::Neon};
 use core::num::NonZeroUsize;
 
 // XXX/TODO(MSRV): change to `pub(super)`.
 pub(in super::super) struct State {
-    state: poly1305_state,
+    state: poly1305_state_st,
     neon: Neon,
 }
 
+// TODO: Is 16 enough?
+#[repr(C, align(16))]
+struct poly1305_state_st {
+    r: fe1305x2,
+    h: fe1305x2,
+    c: fe1305x2,
+    precomp: [fe1305x2; 2],
+
+    data: [u8; data_len()],
+
+    buf: [u8; 32],
+    buf_used: c::size_t,
+    key: [u8; 16],
+}
+
+const fn data_len() -> usize {
+    128
+}
+
+#[derive(Clone, Copy)]
+#[repr(C)]
+struct fe1305x2 {
+    v: [u32; 12], // for alignment; only using 10
+}
+
 impl State {
     pub(super) fn new_context(Key { key_and_nonce }: Key, neon: Neon) -> super::Context {
         prefixed_extern! {
-            fn CRYPTO_poly1305_init_neon(state: &mut poly1305_state, key: &[u8; KEY_LEN]);
+            fn CRYPTO_poly1305_init_neon(state: &mut poly1305_state_st, key: &[u8; KEY_LEN]);
         }
         let mut r = Self {
-            state: poly1305_state([0u8; OPAQUE_LEN]),
+            state: poly1305_state_st {
+                r: fe1305x2 { v: [0; 12] },
+                h: fe1305x2 { v: [0; 12] },
+                c: fe1305x2 { v: [0; 12] },
+                precomp: [fe1305x2 { v: [0; 12] }; 2],
+
+                data: [0u8; data_len()],
+                buf: Default::default(),
+                buf_used: 0,
+                key: [0u8; 16],
+            },
             neon,
         };
         unsafe { CRYPTO_poly1305_init_neon(&mut r.state, &key_and_nonce) }
@@ -41,7 +76,7 @@ impl State {
     pub(super) fn update_internal(&mut self, input: &[u8]) {
         prefixed_extern! {
             fn CRYPTO_poly1305_update_neon(
-                state: &mut poly1305_state,
+                st: &mut poly1305_state_st,
                 input: *const u8,
                 in_len: c::NonZero_size_t);
         }
@@ -54,7 +89,7 @@ impl State {
 
     pub(super) fn finish(mut self) -> Tag {
         prefixed_extern! {
-            fn CRYPTO_poly1305_finish_neon(statep: &mut poly1305_state, mac: &mut [u8; TAG_LEN]);
+            fn CRYPTO_poly1305_finish_neon(st: &mut poly1305_state_st, mac: &mut [u8; TAG_LEN]);
         }
         let mut tag = Tag([0u8; TAG_LEN]);
         unsafe { CRYPTO_poly1305_finish_neon(&mut self.state, &mut tag.0) }

src/aead/poly1305/ffi_fallback.rs

@@ -13,22 +13,58 @@
 // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
 // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-use super::{poly1305_state, Key, Tag, KEY_LEN, OPAQUE_LEN, TAG_LEN};
+use super::{Key, Tag, KEY_LEN, TAG_LEN};
 use crate::c;
 use core::num::NonZeroUsize;
 
 // XXX/TODO(MSRV): change to `pub(super)`.
 pub(in super::super) struct State {
-    state: poly1305_state,
+    state: poly1305_state_st,
+}
+
+// Keep in sync with `poly1305_state_st` in poly1305.c
+#[repr(C, align(64))]
+struct poly1305_state_st {
+    r0: u32,
+    r1: u32,
+    r2: u32,
+    r3: u32,
+    r4: u32,
+    s1: u32,
+    s2: u32,
+    s3: u32,
+    s4: u32,
+    h0: u32,
+    h1: u32,
+    h2: u32,
+    h3: u32,
+    h4: u32,
+    key: [u8; 16],
 }
 
 impl State {
     pub(super) fn new_context(Key { key_and_nonce }: Key) -> super::Context {
         prefixed_extern! {
-            fn CRYPTO_poly1305_init(state: &mut poly1305_state, key: &[u8; KEY_LEN]);
+            fn CRYPTO_poly1305_init(state: &mut poly1305_state_st, key: &[u8; KEY_LEN]);
         }
         let mut r = Self {
-            state: poly1305_state([0u8; OPAQUE_LEN]),
+            state: poly1305_state_st {
+                r0: 0,
+                r1: 0,
+                r2: 0,
+                r3: 0,
+                r4: 0,
+                s1: 0,
+                s2: 0,
+                s3: 0,
+                s4: 0,
+                h0: 0,
+                h1: 0,
+                h2: 0,
+                h3: 0,
+                h4: 0,
+                key: [0u8; 16],
+            },
         };
         unsafe { CRYPTO_poly1305_init(&mut r.state, &key_and_nonce) }
         super::Context::Fallback(r)
@@ -39,7 +75,7 @@ impl State {
     pub(super) fn update_internal(&mut self, input: &[u8]) {
         prefixed_extern! {
             fn CRYPTO_poly1305_update(
-                state: &mut poly1305_state,
+                state: &mut poly1305_state_st,
                 input: *const u8,
                 in_len: c::NonZero_size_t);
         }
@@ -51,7 +87,7 @@ impl State {
 
     pub(super) fn finish(mut self) -> Tag {
         prefixed_extern! {
-            fn CRYPTO_poly1305_finish(statep: &mut poly1305_state, mac: &mut [u8; TAG_LEN]);
+            fn CRYPTO_poly1305_finish(statep: &mut poly1305_state_st, mac: &mut [u8; TAG_LEN]);
         }
         let mut tag = Tag([0u8; TAG_LEN]);
         unsafe { CRYPTO_poly1305_finish(&mut self.state, &mut tag.0) }