eXtendable Output Functions as first-class citizen

[reneme]

Motivation

Most PQC schemes use XOFs as a building block in one or the other way. This is a proposal to establish XOF as a new public API base class. The only implementation currently provided is SHAKE-128 and SHAKE-256.

Pull Request Dependencies

• Refactoring SHA3: based on new permutation keccak-fips #3673

Find included

• New top-level module "xof" with class XOF as abstract base class
  • start() allows priming certain XOFs with a salt and key (otherwise its a NOOP)
  • update() allows streamed input of an arbitrarily long message
  • output() allows streamed access to the XOF's infinitely long output stream
  • typical implementations won't allow update()s after the first call to output()
• SHAKE_128_XOF and SHAKE_256_XOF as sub-classes of XOF
• cSHAKE_128_XOF and cSHAKE_256_XOF as sub-classes of XOF (for internal use only)
• internal helper functions (roughly) as defined in NIST SP.800-185 Section 2.3
  (those functions can be reused to implement KMAC -- see KMAC, 2nd: added keccak-fips as standalone; added KMAC-256 #3570)

Things to be considered

XOFs based on stream ciphers

Kyber and Dilithium build their "90s-crypto" variants on XOF constructions based on AES-256/CTR. We could look into providing support for such stream-cipher XOFs independent of the CRYSTALS algorithms. (See #3672)

Retrofitting PQC algorithms

That's future work and will be done in a follow-up. (See #3672)

[coveralls]

coverage: 91.667% (-0.04%) from 91.707% when pulling afb70af on Rohde-Schwarz:feature/xof-interface into 28b5423 on randombit:master.

[reneme]

Rebased onto @falko-strenzke's Keccak refactoring as a sanity checks. Findings are in #3673.

[reneme]

The whole Keccak-Refactoring is becoming a mess with several inter-dependent pull requests. Sorry for that. Lets put this one on-hold until #3673 and #3675 are integrated.

Edit: done

[reneme]

Re: XOFs based on stream ciphers

Maybe we should step back and rethink the XOF interface somewhat. Looking at NIST SP.800-185 they define things like KMAC-XOF that is KMAC with a variable output length. I.e. just like the AES/CTR-XOF used in CRYSTALS Kyber/Dilithium, it takes a key (and potentially other customization parameters).

Another aspect is cSHAKE (also NIST SP.800-185), which is basically SHAKE, but with additional domain separation parameters N and S. See here for further discussion.

I think it would be great to incorporate such XOFs off the bat, when introducing a new API.

[reneme]

To my mind an abstract XOF should have two phases:

1. Parameterization
2. Output

The "output phase" is simple and inherently generic. The user just calls ::output() as often as they want to read the indefinite stream of XOF output bytes.

The "parameterization phase" is algorithm-specific, unfortunately. Here are some relevant examples:

Algorithm	Parameters
SHAKE	an arbitrary-length message msg
cSHAKE	arbitrary-lengths N and S, and msg
AES/CTR-XOF	a short iv and key
KMAC-XOF	arbitrary-length key, msg and salt

Reflecting this parameter space in a common XOF base-interface is possible but probably won't result in a convenient API. Except, if the only "common" feature of this interface is the ::output() method which could be shared by all XOFs.

In fact, looking at the existing use cases in Kyber and Dilithium, this is exactly what we need there. Depending on the algorithm mode ("90s" or "modern"), the AES/CTR or SHAKE XOF is parameterized inside a polymorphic method and then handed out as an abstract (pre-parameterized) reference. The common code, that is independent of the specific mode, then just "reads" from the abstract XOF object via ::output().

[reneme]

Given the above-described constraints, it seems reasonable to abandon the ubiquitous ::create("AlgoSpec") factory methods for XOFs. Instead, the user could instantiate the concrete XOF subclass, parameterize it and then (maybe) use it via the abstract XOF base class that just provides the ::output() functionality.

@randombit Does that sound like a reasonable abstraction to you?

[reneme]

Benchmarking the SHAKE-128/256 XOF showed comparable results for update() and output() and correspond to the measured performance of SHA-3(256). No obvious bottlenecks.

	absorb()	squeeze()
SHAKE-128	555MiB/sec	530MiB/sec
SHAKE-256	448MiB/sec	432MiB/sec

[randombit]

Instead, the user could instantiate the concrete XOF subclass, parameterize it and then (maybe) use it via the abstract XOF base class that just provides the ::output() functionality.

I very much dislike this. A lot of effort was spent in 2->3 in removing all of the concrete subclasses from the public API; this minimizes API and ABI surface. This would also complicate the FFI interface.

I don't think CTR mode "XOF" is worth optimizing for. It is a weird corner case that should not be exposed to applications. And it seems Kyber is dropping "90s mode" entirely (#3543) so it is probably going to be removed soonish anyway.

Then we're left with

• SHAKE: arbitrary length msg
• cSHAKE: arbitrary length msg. N is not for us - quoting SP 800-185 "N is a function-name bit string, used by NIST to define functions based on cSHAKE. When no function other than cSHAKE is desired, N is set to the empty string." and so can be ignored. And S is a domain separator and can be instantiated in the usual way eg cSHAKE(256, my_domain_dep)
• KMAC-XOF: arbitrary length msg, key, and salt.

class XOF {
  virtual void start(std::span<const uint8_t> salt = {}, std::span<uint8_t> key = {}) = 0;
  virtual bool valid_salt_length(size_t salt_len) = 0;
  // returns Key_Length_Specification(0) if key is not supported
  virtual Key_Length_Specification key_spec() const = 0;
  virtual void update(std::span<const uint8_t> input);
  //...
};

CTR, which doesn't support inputs, is handled by the funny case that XOF::update throws Not_Implemented.

[falko-strenzke]

• cSHAKE: arbitrary length msg. N is not for us - quoting SP 800-185 "N is a function-name bit string, used by NIST to define functions based on cSHAKE. When no function other than cSHAKE is desired, N is set to the empty string." and so can be ignored. And S is a domain separator and can be instantiated in the usual way eg cSHAKE(256, my_domain_dep)

KMAC needs to set N and S, see the KMAC spec

[reneme]

@randombit Thanks for your opinion. Your points make sense and I agree in general. I'll be AFK until the beginning of September and will come back to this after.

[randombit]

First pass review

[randombit]

This should probably also mention it returns 0 if there is no such value.

What is this used for?

[randombit]

If std::optional wasn't so awkward to use I'd suggest returning an optional here. :/

[reneme]

I changed this to be a pure-virtual, forcing downstream implementations to make up their minds.

[randombit]

If the caller provides a non-empty key here it is just ignored, which is going to be confusing and a possible security issue if someone misuses the API. We should instead throw Invalid_Key_Length if this occurs.

[reneme]

Agreed. Now implemented as the base-implementation of XOF::start(). Also the base methods XOF::key_spec() and XOF::valid_salt_length() now have default implementations that basically disable support for keys and salts by default.

[randombit]

Not restricting the cSHAKE salts seems fine, but we also here will accept a salt with plain SHAKE XOF where this is not defined at all.

[reneme]

Yikes, indeed! The condition above was previously meant to exclude non-cSHAKE instances of this base class. But I must have broken it in some restructuring. Anyway, that's certainly not optimal.

I'll look into giving the cSHAKE variants their own base class and leave the salt logic out of the vanilla SHAKE implementation.

[reneme]

Thanks for the review! I rebased to latest master (after merging the std::span refactorings) and addressed your review comments.

Most notably, I split the SHAKE_XOF and cSHAKE_XOF into two separate base classes (and modules). That results in a handful duplicated lines (::add_data(), ::generate_bytes()) but splits off the salt handling in ::start() much more logically.

Also, I XOF::start() is no virtual method anymore. Instead, it implements the required sanity checks for salt and key and then calls the private-virtual start_msg() method. Subclasses may or may not implement start_msg().

[randombit]

Looks good now. Should we have some tests for cSHAKE as well?

[falko-strenzke]

There are four test vectors from NIST, all of which have an empty value for N.

I just implemented cSHAKE for another libgcrpyt and created two test vectors with non empty N using https://asecuritysite.com/golang/cs. I checked their results for empty N and that was at least fine. In any case, the KMAC test vectors will also verify non-empty N.

  { /* Created with https://asecuritysite.com/golang/cs */
    GCRY_MD_CSHAKE128,
    "00010203",
    "ABC",
    "Email Signature",
    32,
    "5CF74DC523ADC0B97EC3614E703835277E9F818879AA1EAE5B2B4E4472EB6A68" },
  { /* Created with https://asecuritysite.com/golang/cs */
   CSHAKE256,
    "00010203", // data hex
    "ABC", // N as C string
    "Email Signature", // S as C string
    32, // output size in bytes
    "0C34C14C4A56E5FC01BE8C04C759DA61437E86B88DF3E21A934436D427A85E9D"  // expected output hex
}, 

[randombit]

Thanks for checking this @falko-strenzke. @reneme can we get these included here so we have some baseline check on cSHAKE? I guess it is a little complicated by cSHAKE not being exposed to applications so you can't immediately use the normal XOF test.

[reneme]

@falko-strenzke Nice! Thanks for the test vectors. I did look for something "official" for cSHAKE but wasn't successful.

@randombit I hacked the XOF allocation in test_xof.cpp so that it explicitly deals with the internal cSHAKE XOFs when needed. Similarly, I also added vectors for the AES/CTR-XOF in #3672 a while ago.

[randombit]

@reneme looks good thanks. I couldn't find the tests themselves though - are you missing a git add of cshake.vec?

[reneme]

I couldn't find the tests themselves though - are you missing a git add of cshake.vec?

🤦‍♂️

[reneme]

I was fighting some linker visibility issues. Had to move the cSHAKE_XOF::provider() and ::block_size() implementations into the *.cpp file to avoid exposing the Keccak_Permutation in the shared library.

Now everything should turn green. ☺️

[reneme]

🥳