Add Internal SHAKE interfaces and their NIST test vectors. #572

manastasova · 2022-07-30T00:44:02Z

Issues:

CryptoAlg-1171

Add SHAKE128 and SHAKE256 internal interface support for later use in the PQ protocols. Add NIST test vector checks.

Description of changes:

Add SHAKE128 and SHAKE256 support. Test the correctness through SHAKE128Test.NISTTestVectors andSHAKE256Test.NISTTestVectors added from NIST Test Vectors for SHAKE https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/secure-hashing.

Detailed description of the applied changes is presented in CryptoAlg-1171?selectedConversation=3abf0a8b-88db-4e0e-b124-9dfc8701c89b.

Testing:

Testing of the correct assembly code integration is performed by corrupting the assembly code asserting failure.

TestVectors SHAKE128
SHAKE128Test.NISTTestVectors

TestVectors SHAKE1256
SHAKE256Test.NISTTestVectors

Keep the SHA3 tests:
TestVectors SHA3-XXX
./crypto/crypto_test --gtest_filter=DigestTest.TestVectors

SpeedTest SHA3-XXX
./tool/bssl speed -filter SHA3-XXX

Exhaustive TestVectors SHA3-XXX
./crypto/crypto_test --gtest_filter=SHA3Test.NISTTestVectors
./crypto/crypto_test --gtest_filter=SHA3Test.NISTTestVectors_SingleShot

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Integrate SHA3 low-level APIs from OpenSSL and high-level APIs following SHA2 APIs structure; Use ./crypto/crypto_test --gtest_filter=DigestTest.TestVectors for running test vectors and ./tool/bssl speed -filter SHA3-256 for performance evaluation

This reverts commit 039bf06.

This reverts commit e314ab9.

Address review comments, replace test vectors with NIST values, and add generated-src

Co-authored-by: dkostic <[email protected]> Co-authored-by: Andrew Hopkins <[email protected]>

…A3_shake

crypto/fipsmodule/sha/internal.h

dkostic · 2022-08-08T17:04:26Z

crypto/fipsmodule/sha/internal.h

+// The data block size increases when the capacity decreases.
+// The data buffer should have at least the maximum 
+// block size bytes to fit any digest length.
+#define SHA3_MIN_CAPACITY_BYTES (KECCAK1600_WIDTH / 8 - SHAKE128_BLOCKSIZE)


few things before we continue:

is this value Keccak specific or SHA3/SHAKE specific? If Keccak specific then I'd prefer Keccak in the name

is there a simpler way to define what capacity is?

can we define capacity in the code comment before explaining what increases/decreases?

crypto/fipsmodule/sha/internal.h

dkostic · 2022-08-08T17:06:19Z

crypto/fipsmodule/sha/sha3.c

+  KECCAK1600_CTX ctx;
+
+  // The SHAKE block size depends on the security level of the algorithm only
+  // It is independent of the digest size


Nit: digest size or output size? I think SHAKE is XOF not MD, right?

True. Sorry for this mistake! Fixed.

dkostic · 2022-08-08T17:06:28Z

crypto/fipsmodule/sha/sha3.c

+  KECCAK1600_CTX ctx;
+
+  // The SHAKE block size depends on the security level of the algorithm only
+  // It is independent of the digest size


same as above

crypto/fipsmodule/sha/sha3.c

dkostic · 2022-08-08T17:18:38Z

crypto/fipsmodule/sha/sha3.c

+  // digest length (for SHA3) or the security level for (SHAKE)
+  if (pad == SHA3_PAD_CHAR) {
+    block_size = SHA3_BLOCKSIZE(bit_len);
+  } else {


what if pad is some value that's different from both SHA3_PAD_CHAR and SHAKE_PAD_CHAR?

I added return 0 case since a wrong result will be produced if not. Sorry for this.

dkostic · 2022-08-08T17:34:46Z

crypto/fipsmodule/sha/sha3_test.cc

+    uint32_t digest_length = out_len_ / 8;
+    uint8_t *digest = new uint8_t[digest_length];
+
+    #if !defined(OPENSSL_ANDROID)


didn't we get rid of these in the SHA3 tests? Can we do it here as well?

I will keep the branch updated with main. Sorry for this.

dkostic · 2022-08-11T17:49:27Z

crypto/fipsmodule/sha/internal.h

+// In the sponge construction, the width of the underlying 
+// function minus the rate (the block size in bits).
+// The |SHA3_MIN_CAPACITY_BYTES| corresponds to the 
+// lowest security level capacity for SHA3/SHAKE.


Suggested change

// In the sponge construction, the width of the underlying

// function minus the rate (the block size in bits).

// The |SHA3_MIN_CAPACITY_BYTES| corresponds to the

// lowest security level capacity for SHA3/SHAKE.

// In the sponge construction, capacity is the width of the underlying function

// minus the rate. In case of SHA3 and SHAKE, the underlying function is Keccak

// and the rate is the block size of a specific instantiation of SHA3 or SHAKE.

// |SHA3_MIN_CAPACITY_BYTES| corresponds to the minimum capacity taking into

// account all the instantiations of SHA3 and SHAKE we implement: SHA3-256,

// SHAKE128, SHAKE256. Since the block size of SHAKE128 is the largest among

// those, we compute the minimum capacity by subtracting SHA128 block size from

// Keccak width.

Thank you so much. I have fixed it. I added the source for the capacity definition as NIST FIPS202:
// [NIST FIPS202] Capacity: In the sponge construction,

dkostic · 2022-08-11T17:50:49Z

crypto/fipsmodule/sha/internal.h

+// The data blocks increase when the capacity decreases.
+// The data buffer should have at least the maximum number of
+// block size bytes to fit any SHA3/SHAKE block length.


these lines seem misplaced, so I would delete them from here.

I just fixed that! Sorry.

Co-authored-by: dkostic <[email protected]>

nebeid · 2022-08-16T15:42:19Z

crypto/fipsmodule/sha/sha3_test.cc

@@ -83,8 +83,53 @@ class SHA3TestVector {

  }

+  void NISTTestVectors_SHAKE128() const {
+    uint32_t digest_length = out_len_ / 8;
+    uint8_t *digest = new uint8_t[digest_length];


Can we use std::unique_ptr<uint8_t[]> digest(new uint8_t[digest_length]; here and in previous occurrences? Static analysis is seeing it as resource leak in the previous occurrences. I saw the unique_ptr usage in other test files.

Anastasova added 30 commits June 7, 2022 17:48

Integrate SHA3_256

db50179

Integrate SHA3 low-level APIs from OpenSSL and high-level APIs following SHA2 APIs structure; Use ./crypto/crypto_test --gtest_filter=DigestTest.TestVectors for running test vectors and ./tool/bssl speed -filter SHA3-256 for performance evaluation

Merge lart changes from AWS-LC main

91f5085

remove build & install dirs

ac492a3

change NID_sha3_256 in nid.h

891ebe8

reverse file formatting

1ea1eda

Update build files in generated-src

ee45b14

Update build files in generated-src

b1e4763

add asm keccak file to fipsmodule dir

18eea85

remove keccak1600-armv8.S file

24ae5de

try redefine SHA3_256_ASM

461090d

reverse SHA3_256_ASM

00d1b09

add check for asm in internal.h

5f2d64b

fix redirect stdout >> keccak1600-armv8.S

fac1514

Merge branch 'main' of github.com:manastasova/aws-lc

e8c7be3

rm extra checks for SHA3_256_ASM

12ac405

rm -r generated-src

b277446

Update build files in generated-src

1a0dd40

fix redirect stdout >> keccak1600-armv8.S

bd918e7

fix redirect stdout >> keccak1600-armv8.S

707aa8c

remove double arg check

eca066d

only C SHA3

e089555

rm build/install dir

e0214d3

rm .vscode/settings.json

28c1e59

add generate-src

039bf06

partially addressed comments

e314ab9

Revert "add generate-src"

396e934

This reverts commit 039bf06.

Revert "partially addressed comments"

5355554

This reverts commit e314ab9.

addressed all comments

25166be

Update after review

8e50179

Address review comments, replace test vectors with NIST values, and add generated-src

add check ret value for SHA3_256 and remove SELFTEST code

31f7ed7

manastasova and others added 6 commits August 3, 2022 11:03

disable sha3/shake tests by const; add reduced shake test set

2095700

fix the changed timeout values

cb684cd

Co-authored-by: dkostic <[email protected]> Co-authored-by: Andrew Hopkins <[email protected]>

fix comments and comment lengths

f4cf0c0

fix comments and comment lengths

5e60f6f

disable sha3/shake tests by const; add reduced shake test set

fa42a89

fix comments and comment lengths

3ef02e0

manastasova force-pushed the SHA3_shake branch from cb684cd to 5e60f6f Compare August 5, 2022 01:10

manastasova and others added 4 commits August 4, 2022 18:14

fix comments and comment lengths

2b4cbdd

Merge remote-tracking branch 'refs/remotes/origin/SHA3_shake' into SH…

bc712a6

…A3_shake

Merge branch 'main' into SHA3_shake

19f7552

Update build files in generated-src

8da9b3b

dkostic reviewed Aug 8, 2022

View reviewed changes

add return to PAD check, add comments for capacity

1367a8a

dkostic reviewed Aug 11, 2022

View reviewed changes

manastasova and others added 6 commits August 12, 2022 12:19

Apply suggestions from code review

c3b0dff

Co-authored-by: dkostic <[email protected]>

merge upstream/main; resolve conflicts

ed7376e

fix redefinition of var

feee7b9

reverse int and string reads

29cf3e1

change int16 to int32

d077594

move buffer size comment

7787a0f

manastasova force-pushed the SHA3_shake branch from 48d9e86 to 7787a0f Compare August 15, 2022 22:44

manastasova and others added 2 commits August 15, 2022 15:50

Merge branch 'main' into SHA3_shake

d373ba1

add SHA3_MAX_BLOCKSIZE, rm SHA3_MIN_CAPACITY for buf size

927ab56

nebeid reviewed Aug 16, 2022

View reviewed changes

use std::unique_ptr<uint8_t[]> digest()

b77a3ce

dkostic approved these changes Aug 16, 2022

View reviewed changes

manastasova changed the title ~~Integrate Internal SHAKE APIs with NIST Test Vectors~~ Add Internal SHAKE interfaces and their NIST test vectors. Aug 16, 2022

nebeid approved these changes Aug 18, 2022

View reviewed changes

Merge branch 'main' into SHA3_shake

176f914

nebeid merged commit 5283493 into aws:main Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Internal SHAKE interfaces and their NIST test vectors. #572

Add Internal SHAKE interfaces and their NIST test vectors. #572

manastasova commented Jul 30, 2022 •

edited

Loading

dkostic Aug 8, 2022

dkostic Aug 8, 2022

manastasova Aug 9, 2022

dkostic Aug 8, 2022

manastasova Aug 9, 2022

dkostic Aug 8, 2022

manastasova Aug 9, 2022

dkostic Aug 8, 2022

manastasova Aug 9, 2022

dkostic Aug 11, 2022

manastasova Aug 15, 2022

dkostic Aug 11, 2022

manastasova Aug 15, 2022

nebeid Aug 16, 2022

-// In the sponge construction, the width of the underlying
-// function minus the rate (the block size in bits).
-// The |SHA3_MIN_CAPACITY_BYTES| corresponds to the
-// lowest security level capacity for SHA3/SHAKE.
+// In the sponge construction, capacity is the width of the underlying function
+// minus the rate. In case of SHA3 and SHAKE, the underlying function is Keccak
+// and the rate is the block size of a specific instantiation of SHA3 or SHAKE.
+// |SHA3_MIN_CAPACITY_BYTES| corresponds to the minimum capacity taking into
+// account all the instantiations of SHA3 and SHAKE we implement: SHA3-256,
+// SHAKE128, SHAKE256. Since the block size of SHAKE128 is the largest among
+// those, we compute the minimum capacity by subtracting SHA128 block size from
+// Keccak width.

Add Internal SHAKE interfaces and their NIST test vectors. #572

Add Internal SHAKE interfaces and their NIST test vectors. #572

Conversation

manastasova commented Jul 30, 2022 • edited Loading

Issues:

Description of changes:

Testing:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manastasova commented Jul 30, 2022 •

edited

Loading