Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POLYVAL universal hash function for AES-GCM(-SIV) #13

Merged
merged 4 commits into from
Aug 26, 2019
Merged

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Aug 13, 2019

POLYVAL is a GHASH-like universal hash function defined over GF(2^128). It's a sort of "mirror image" of GHASH optimized for little endian architectures. As such, it can be used as the core of GHASH.

This is an implementation I wrote awhile ago based on (at the time) draft-irtf-cfrg-gcmsiv (now RFC 8452 Section 3). It is definitely unfinished and has never been run against test vectors is presently failing the test vectors, so for now ASSUME IT IS BROKEN AND SHOULD NOT BE USED passes the test vectors!

The implementation uses Shay Gueron's techniques for efficient finite field multiplication with PCLMULQDQ, including a Montgomery fast reduction method. For background on how these work, see this excellent blog post by QuarksLab:

https://blog.quarkslab.com/reversing-a-finite-field-multiplication-optimization.html

When using these techniques, it's still not as fast as I'd like, but certainly acceptable performance (edit: updated):

test bench1_10    ... bench:          18 ns/iter (+/- 1) = 555 MB/s
test bench2_100   ... bench:          85 ns/iter (+/- 7) = 1176 MB/s
test bench3_1000  ... bench:         797 ns/iter (+/- 126) = 1254 MB/s
test bench3_10000 ... bench:       7,558 ns/iter (+/- 733) = 1323 MB/s

It contains a fallback implementation used when PCLMULQDQ isn't available (gated under the off-by-default insecure-soft cargo feature) which is both non-constant-time (😱) and abysmally slow:

test bench1_10    ... bench:         206 ns/iter (+/- 20) = 48 MB/s
test bench2_100   ... bench:       1,981 ns/iter (+/- 187) = 50 MB/s
test bench3_1000  ... bench:      19,648 ns/iter (+/- 1,907) = 50 MB/s
test bench3_10000 ... bench:     194,441 ns/iter (+/- 21,198) = 51 MB/s

polyval/src/lib.rs Outdated Show resolved Hide resolved
@tarcieri
Copy link
Member Author

tarcieri commented Aug 13, 2019

I'd like to finish the GHASH support as part of this PR, as that's the main thing that makes it useful. Perhaps it would be worth defining a ghash or gmac crate which uses polyval as a dependency. (Update: punting and will follow up in a separate PR)

RFC 8452 Appendix A contains ample information about how to implement GHASH in terms of POLYVAL, and also test vectors that can be used for both.

Here is a slightly modified/reformatted version of the text from RFC 8452 Appendix A which describes the conversions and test vectors for them:

-- snip --

Functions

  • mulX_GHASH: function that takes a 16-byte string, converts it to an element of GHASH's field using GHASH's convention, multiplies it by x, and converts it back to a string.
  • mulX_POLYVAL: function that converts a 16-byte string to an element of POLYVAL's field using POLYVAL's convention, multiplies it by x, and converts it back.

mulX_GHASH and mulX_POLYVAL Vectors

  • Vector 1:
    • input: (16-byte string) 01000000000000000000000000000000
    • mulX_GHASH(input): 00800000000000000000000000000000
    • mulX_POLYVAL(input): 02000000000000000000000000000000
  • Vector 2:
    • input: 9c98c04df9387ded828175a92ba652d8
    • mulX_GHASH(input): 4e4c6026fc9c3ef6c140bad495d3296c
    • mulX_POLYVAL(input): 3931819bf271fada0503eb52574ca5f2

Definitions

  • ByteReverse: function that takes a 16-byte string and returns a copy where the order of the bytes has been reversed.
  • POLYVAL(H, X_1, ..., X_n) => ByteReverse(GHASH(mulX_GHASH(ByteReverse(H)),ByteReverse(X_1), ..., ByteReverse(X_n)))
  • GHASH(H, X_1, ..., X_n) => ByteReverse(POLYVAL(mulX_POLYVAL(ByteReverse(H)), ByteReverse(X_1), ..., ByteReverse(X_n)))

POLYVAL and GHASH Test Vectors

  • H: 25629347589242761d31f826ba4b757b
  • X_1: 4f4f95668c83dfb6401762bb2d01a262
  • X_2: d1a24ddd2721d006bbe45f20d3c9f362
  • POLYVAL(H, X_1, X_2): f7a3b47b846119fae5b7866cf5e5b77e
  • GHASH(H, X_1, X_2): bd9b3997046731fb96251b91f9c99d7a

Calculating POLYVAL with GHASH

  • mulX_GHASH(ByteReverse(H)) => dcbaa5dd137c188ebb21492c23c9b112
  • ByteReverse(GHASH(dcba..., ByteReverse(X_1), ByteReverse(X_2))) => f7a3b47b846119fae5b7866cf5e5b77e
    `

Calculating GHASH with POLYVAL

  • mulX_POLYVAL(ByteReverse(H)) => f6ea96744df0633aec8424b18e26c54a
  • ByteReverse(POLYVAL(f6ea..., ByteReverse(X_1), ByteReverse(X_2))) => bd9b3997046731fb96251b91f9c99d7a

polyval/src/lib.rs Outdated Show resolved Hide resolved
polyval/src/lib.rs Outdated Show resolved Hide resolved
polyval/src/lib.rs Outdated Show resolved Hide resolved
@tarcieri tarcieri force-pushed the polyval branch 6 times, most recently from 8470b83 to b6d62a0 Compare August 14, 2019 15:55
@tarcieri
Copy link
Member Author

tarcieri commented Aug 14, 2019

Added a test with a vector from RFC 8452 Appendix A, which is at least now confirming that the implementation is presently broken.

Now the joy of figuring out what's wrong...

--

Since that's mildly terrifying to say about cryptography code, there's one way we can have extremely high confidence in the implementation's correctness, and that's through formal verification. Fortunately Galois has done all of the hard work there, and have written Cryptol specifications for all of the parts of AES-GCM-SIV including POLYVAL:

https://github.com/GaloisInc/AES-GCM-SIV-proof/blob/master/proof/cryptol-specs/intrinsics.cry

Using Galois' SAW (Software Analysis Workbench), it's possible to formally prove equivalence both to the reference Cryptol implementation and to Shay Gueron's reference C code (as Galois already used SAW to prove formal equivalence between the reference C code and Galois' Cryptol implementation):

https://github.com/GaloisInc/AES-GCM-SIV-proof/blob/master/proof/cryptol-specs/polyval_proof.saw

SAW can only prove equivalence and cannot prove a program is free of e.g. timing sidechannels, however my understanding is it also integrates with ProVerif which has some of those sort of capabilities.

All that said, after I have a basic test vector passing, I will investigate using SAW to prove the implementation's correctness.

@tarcieri tarcieri force-pushed the polyval branch 3 times, most recently from 81b46ac to 80a3f90 Compare August 15, 2019 08:53
@tarcieri
Copy link
Member Author

Okay, the tests vectors are passing! Removing draft but leaving [WIP] as there's lots of cleanup I'd like to do on this PR first.

@tarcieri tarcieri force-pushed the polyval branch 8 times, most recently from 1182f2b to a068859 Compare August 26, 2019 03:46
@tarcieri
Copy link
Member Author

tarcieri commented Aug 26, 2019

@newpavlov well, I'd call this "done" for an initial cut except:

  • It doesn't implement GHASH (yet). I've changed my mind and want to save that for a followup PR.
  • The portable implementation is non-constant-time and gated behind the insecure-soft feature
  • The only "secure" (still needs constant time verification) requires pclmulqdq
  • pclmulqdq appears to be broken on Rust 1.27.0, but passes on stable, so it's presumably something wrong with core::arch support for pclmulqdq in that release

I am going to push a commit bumping MSRV to 1.32.0 in .travis.yml. If that works, I say we just go ahead and do a 2018 edition upgrade on the polyval crate, both for housekeeping reasons and to force at least MSRV 1.32.0 (which I can ensure by eliminating the byteorder crate and replacing it with the built-in conversions).

Separately (ideally from this PR, saved for a follow-up) I will look at adapting the BearSSL implementation of GHASH, which is probably the best we're going to get out of a portable software implementation in terms of both constant-time operation and performance (~150MB/sec):

https://bearssl.org/gitweb/?p=BearSSL;a=blob;f=src/hash/ghash_ctmul64.c

@tarcieri
Copy link
Member Author

tarcieri commented Aug 26, 2019

Confirm it works with MSRV 1.32.0:

https://travis-ci.org/RustCrypto/MACs/builds/576637449

Given that, and the fact we presently need to split the build out anyway to accommodate either RUSTFLAGS or --features=insecure-soft, I will go ahead and to a 2018 edition upgrade and make use of the 1.32+ endianness features.

@tarcieri
Copy link
Member Author

Oof, I guess having any 2018 edition crate in the workspace breaks --all on 1.27.0 since it tries to parse the Cargo.toml even when given --exclude:

https://travis-ci.org/RustCrypto/MACs/jobs/576645142

I can temporarily work around that with a for loop on 1.27.0 only. Ick though.

@tarcieri tarcieri force-pushed the polyval branch 4 times, most recently from ff27089 to b931e84 Compare August 26, 2019 05:34
@tarcieri
Copy link
Member Author

I gave up on attempting a 2018 edition upgrade for polyval and just changed all of its documentation to warn that the MSRV is 1.32.0 (b931e84).

@newpavlov I'd say I'm "done" with this PR, aside from addressing any feedback. Can you review?

I think it'd be a lot better with a portable, "performance"-oriented constant time implementation, and/or if it actually implemented GHASH in terms of POLYVAL, but other than that I think this is a reasonable start.

Implements POLYVAL using Shay Gueron's techniques for efficient field
multiplications using PCLMULQDQ.

More information on these techniques here:

https://blog.quarkslab.com/reversing-a-finite-field-multiplication-optimization.html
@tarcieri
Copy link
Member Author

@newpavlov or if nothing else, are you okay with the changes to .travis.yml here? If you're too busy to review this whole PR I can run it by some other people to critique the implementation

@newpavlov
Copy link
Member

newpavlov commented Aug 26, 2019

@tarcieri
I haven't looked deep into the implementation yet and I think it will be some time before I'll be able to. So you can merge it without waiting on me.

After a cursory look I have just two small nits:

  • Replace MIT/Apache-2.0 with MIT OR Apache-2.0.
  • I think you can remove crypto-mac from dependencies (i.e. move it to dev-dependencies) and create a new-type wrapper around [u8; 16] with an implementation of ConstantTimeEq trait.

are you okay with the changes to .travis.yml here?

As a temporary solution it looks good to me. But we will need to simplify it after completion of 2018 edition migration.

The POLYVAL tests are failing on Rust 1.27.0, but passing on newer
versions, suggesting there may be something amiss with `core::arch`
support for `pclmulqdq` on older Rust versions.

This bumps it up to our ideal MSRV, as 1.32.0+ would allow us to replace
the dependency on the `byteorder` crate with equivalent `core`
functions.
Eliminates the dependency on `crypto-mac` by using a built-in `Tag` type
which impl's `subtle::ConstantTimeEq`.

Ideally this would probably get extracted into some other trait, e.g.
`UniversalHash`, but for now just use concrete types per crate.
@tarcieri
Copy link
Member Author

@newpavlov done. I also made the same changes to the poly1305 crate for consistency.

Perhaps it'd be good to define a UniversalHash trait for this purpose? That's something I could potentially open a RustCrypto/traits PR for.

@tarcieri tarcieri merged commit bd1bcf5 into master Aug 26, 2019
@tarcieri tarcieri deleted the polyval branch August 26, 2019 21:24
@tarcieri tarcieri mentioned this pull request Aug 26, 2019
@newpavlov
Copy link
Member

@tarcieri
Yes, exploring a universal hash trait looks like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants