Make poly1305 support incremental computation + modernize

[sipa]

Our current Poly1305 code (src/crypto/poly1305.*) only supports computing the entire tag in one go (the poly1305_auth function takes a key and message, and outputs the tag). However, the RFC8439 authenticated encryption (as used in BIP324, see #27634) scheme makes use of Poly1305 in a way where the message consists of 3 different pieces:

• The additionally authenticated data (AAD), padded to 16 bytes.
• The ciphertext, padded to 16 bytes.
• The length of the AAD and the length of the ciphertext, together another 16 bytes.

Implementing RFC8439 using the existing poly1305_auth function requires creating a temporary copy with all these pieces of data concatenated just for the purpose of computing the tag (the approach used in #25361).

This PR replaces the poly1305 code with new code from https://github.com/floodyberry/poly1305-donna (with minor adjustments to make it match our coding style and use our utility functions, documented in the commit) which supports incremental operation, and then adds a C++ wrapper interface using std::byte Spans around it, and adds tests that incremental and all-at-once computation match.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	theStack, stratospher, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

[theStack]

Concept ACK

Verified that the poly1305 implementation introduced in 4abc846 matches poly1305-donna (used https://github.com/openbsd/src/blob/master/sys/crypto/poly1305.c as a reference, which is again based on Andrew Moon's implementation with minor adaptions). Still planning to verify the test vectors with another implementation, likely PyCryptodome.

[theStack]

LGTM overall. Verified the introduced test vectors (40de8ce) with the pyca/cryptography library, which uses OpenSSL in the background (tried with PyCryptodome first, but that didn't allow use of raw Poly1305 without a cipher algorithm). As expected, everything passes with this alternative implementation as well.

Python code

#!/usr/bin/env python3
from cryptography.hazmat.primitives.poly1305 import Poly1305

def test_poly1305(msg, key, tag):
    p = Poly1305(bytes.fromhex(key))
    p.update(bytes.fromhex(msg))
    assert p.finalize() == bytes.fromhex(tag)

test_poly1305("8e993b9f48681273c29650ba32fc76ce48332ea7164d96a4476fb8c531a1186a" +
              "c0dfc17c98dce87b4da7f011ec48c97271d2c20f9b928fe2270d6fb863d51738" +
              "b48eeee314a7cc8ab932164548e526ae90224368517acfeabd6bb3732bc0e9da" +
              "99832b61ca01b6de56244a9e88d5f9b37973f622a43d14a6599b1f654cb45a74" +
              "e355a5",
              "eea6a7251c1e72916d11c2cb214d3c252539121d8e234e652d651fa4c8cff880",
              "f3ffc7703f9400e52a7dfb4b3d3305d9")

total_ctx = Poly1305(bytes.fromhex("01020304050607fffefdfcfbfaf9ffffffffffffffffffffffffffff00000000"))
for i in range(256):
    key = bytes([i]*32)
    msg = bytes([i]*i)
    p = Poly1305(key)
    p.update(msg)
    tag = p.finalize()
    total_ctx.update(tag)
assert total_ctx.finalize() == bytes.fromhex("64afe2e8d6ad7bbdd287f97c44623d39")

test_poly1305("000000000000000000000094000000000000b07c4300000000002c002600d500" +
              "00000000000000000000000000bc58000000000000000000c9000000dd000000" +
              "00000000000000d34c000000000000000000000000f9009100000000000000c2" +
              "4b0000e900000000000000000000000000000000000e00000027000074000000" +
              "0000000003000000000000f1000000000000dce2000000000000003900000000" +
              "0000000000000000000000000000000000000000000000520000000000000000" +
              "000000000000000000000000009500000000000000000000000000cf00826700" +
              "000000a900000000000000000000000000000000000000000079000000000000" +
              "0000de0000004c000000000033000000000000000000000000002800aa000000" +
              "00003300860000e000000000",
              "6e543496db3cf677592989891ab021f58390feb84fb419fbc7bb516a60bfa302",
              "7ea80968354d40d9d790b45310caf7f3")
test_poly1305("0000005900000000c40000002f00000000000000000000000000000029690000" +
              "0000e8000037000000000000000000000000000b000000000000000000000000" +
              "000000000000000000000000001800006e0000000000a4000000000000000000" +
              "00000000000000004d00000000000000b0000000000000000000005a00000000" +
              "0000000000b7c300000000000000540000000000000000000000000a00000000" +
              "00005b0000000000000000000000000000000000002d00e70000000000000000" +
              "000000000000003400006800d700000000000000000000360000000000000000" +
              "00eb000000000000000000000000000000000000000000000000000028000000" +
              "37000000000000000000000000000000000000000000000000000000008f0000" +
              "000000000000000000000000",
              "f0b659a4f3143d8a1e1dacb9a409fe7e7cd501dfb58b16a2623046c5d337922a",
              "0e410fa9d7a40ac582e77546be9a72bb")

Left some nits for removing the need for Make{Writable}ByteSpan calls. I think they can also be applied to the TestPoly1305 function.

[theStack]

ACK 4e5c933

[sipa]

@MarcoFalke You may be interested in the Span<std::byte> usage here.

[maflcko]

Sure, happy to leave comments, but I think it may be better to fix them in a follow-up, unless you really want to here.

[stratospher]

tested ACK 4e5c933.

cross checked that this is consistent with poly1305-donna implementation. benchmarks look better compared to previous poly1305_auth code too. (also found this blogpost which explains poly1305's design interesting!)

noticed this tiny difference in poly1305-donna's vs RFC 8439 r - though it might also be some trick for later computation i guess.

• r before clamping: 85:d6:be:78:57:55:6d:33:7f:44:52:fe:42:d5:06:a8
• Clamped r as a number: 806d5400e52447c036d555408bed685

• chopping r into 5 pieces for r[0], r[1], r[2], r[3], r[4]
• can be written as: 806d5, 400e524, 47c036, d555408, bed685
• r in floodyberry's : 806d5, 1003949, 47c036, 3555502, bed685
• where 400e524 = 1003949 + "two zero bits" and d555408 = 3555502 + "two zero bits"

[sipa]

@stratospher There are two approaches for poly1305 implementations out there; one where the key (and the accumulated hash value) are represented using 32-bit limbs, and one where it's represented using 26-bit limbs (so in one r = sum(r[i] * 2^(32*i)) while in the other r = sum(r[i] * 2^(26*i))). Poly1305 is designed to make 32-bit limb designs efficient (the masking out of the bits guarantee some overflows are avoided, as explained in the blogpost you link). However, in all my benchmarks the 26-bit limb design (as is used in the poly1305-donna code) is faster. That's kind of a pity, because it means the masking is pointless (and actually reduces the security of the scheme by a few bits), yet necessary to remain compatible with the poly1305 spec.

[sipa]

@MarcoFalke Thanks for the comments; I think I'm going to create a separate PR with some further code refactoring related to this as well, outside of the critical path of #25361.

[achow101]

ACK 4e5c933

Verified that this was consistent with poly1305-donna.