Exposed SHA3 through EVP API by havishr · Pull Request #485 · corretto/amazon-corretto-crypto-provider

havishr · 2025-07-22T15:51:49Z

Changes: Implemented hash_evp_template.cpp.template, which exposes the EVP_MD API. Exposed SHA3_224, SHA3_256, SHA3_384, and SHA3_512. Allows for the use of these algorithms in an isolated context at the moment.

Choices: Chose to use the specific constants described in the csrc files for each algorithm because they are not all exposed through EVP_MD. Manually copying the context from the Java state to EVP_MD_CTX and vice versa. Is there a better way of doing this?

Testing: All testing was done the same as the other hash functions. Individual classes for each algorithm that contain KATs and the use HashFunctionTester. Did CAVP testing with the files from https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/secure-hashing. Had to set a minimum Java version for HashFunctionTester because SUN JDK8 does not expose SHA3 so trying to compare the implementations throws a NoSuchAlgorithmException.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

WillChilds-Klein

Looking good so far!

WillChilds-Klein · 2025-08-04T14:52:57Z

+    addService("MessageDigest", "SHA3-224", "SHA3224Spi");
+    addService("MessageDigest", "SHA3-256", "SHA3256Spi");
+    addService("MessageDigest", "SHA3-384", "SHA3384Spi");
+    addService("MessageDigest", "SHA3-512", "SHA3512Spi");


note to reviewers -- i confirmed that these 4 service names align with JCA-standard names for SHA3

https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html

geedo0 · 2025-08-08T18:11:13Z

While I don't think anyone here (or anywhere) uses Eclipse anymore, I don't think this PR is the place to remove it.

geedo0 · 2025-08-08T18:21:59Z

+#include "generated-headers.h"
+#include "util.h"
+
+


Style nit: Here and elsewhere in this file there is superfluous whitespace. Try to stick to just 1 newline, there's really no benefit for using more than that.

geedo0 · 2025-08-08T18:23:44Z

+// Transform DIGEST_NAME from "sha3_224" format to "SHA3224" format for JNI class names
+// This macro needs to be defined for each digest algorithm


This seems like an anti-pattern for what should be a template. You're basically requiring that we make a change to the template each time you need some new algorithm. This should be pulled up.

geedo0 · 2025-08-08T18:37:27Z

+#define DIGEST_NAME sha3_224 
+#define DIGEST_LENGTH 28
+#define DIGEST_BLOCK_SIZE 144
+#define MD_CTX_SIZE 400


It would be good to define these constants somewhere, like a comment in the template.

Where did you get the DIGEST_BLOCK_SIZE values from?

MD_CTX_SIZE seems like it would be the same for all the hashes right? Maybe pull this down into the template layer. Better yet, replace it with a sizeof call on the actual type to avoid any drift between LC and this template..

geedo0 · 2025-08-08T18:40:30Z

+        raii_env env(pEnv);
+
+        SecureBuffer<CTX,1> ctx;
+        const size_t scratchSize = DIGEST_BLOCK_SIZE; // Size is arbitrarily chosen


geedo0 · 2025-08-14T15:31:39Z

+    MessageDigest digest = getDigest();
+
+    assertArrayEquals(
+        Hex.decodeHex(getTestVector().toCharArray()), digest.digest("testing".getBytes()));


This is an odd way to lay things out. We should factor out the pre-image "testing" similar to the "TestVector" otherwise, implementers of this class need to implicitly know that their pre-image must always be the String "testing".

geedo0 · 2025-08-14T15:33:17Z

+  public void cavpShortVectors() throws Throwable {
+    try (final InputStream is = new GZIPInputStream(TestUtil.getTestData(getCavpShortFile()))) {
+      new HashFunctionTester(getAlgorithm()).test(RspTestEntry.iterateOverResource(is));
+    }
+  }
+
+  @Test
+  public void cavpLongVectors() throws Throwable {


These are both the same. You could further parameterize these to accept a list of input files that get fed into the same cavp test case.

geedo0 · 2025-08-14T15:34:34Z

+  private static final String ALGORITHM = "SHA3-384";
+  private static final String NULL_DIGEST =
+      "0c63a75b845e4f7d01107d852e4c2485c51a50aaaa94fc61995e71bbee983a2ac3713831264adb47fb6bd1e058d5f004";
+  private static final String TEST_VECTOR =
+      "e15a44d4e12ac138db4b8d77e954d78d94de4391ec2d1d8b2b8ace1a2f4b3d2fb9efd0546d6fcafacbe5b1640639b005";
+  private static final String CAVP_SHORT_FILE = "SHA3_384ShortMsg.rsp.gz";
+  private static final String CAVP_LONG_FILE = "SHA3_384LongMsg.rsp.gz";


Personally, I'd inline these. Don't like the additional layer of indirection here.

…Context, and updateContextByteArray with appropriate aws-lc functionality

…mplate.cpp

…ng. created the cpp for each hash with the appropriate definitions

…ests for sha3 excluding the HashFunctionTester implementations which need the appropriate zip files of data.

…properly. Adjusted SPI names.

…ack.

…use SHA3 is not exposed in SUN JDK 8 which is neede for TestAPI and TestRandomly

…one test class

…e consolidating functionality.

…cleaning up files

- Fix md_data leak: use scoped EVP_MD_CTX (EVP_MD_CTX_auto) in initContext/updateContextByteArray/updateNativeByteBuffer so the EVP_DigestInit-allocated md_data is freed on every call. - Move the JNI class-name token (JAVA_CLASS_NAME) into each per-algorithm .cpp so the shared template is algorithm-agnostic. - Consolidate MD_CTX_SIZE into the template and document it and DIGEST_BLOCK_SIZE (no public API exposes the internal ctx size). - Explain java_buffer vs bounce_buffer usage; clean up whitespace and the fast-path scratch comment. - Restore the unrelated .project deletion. - Refactor BaseSHATest to take algorithm/pre-image/vectors/CAVP files via constructor; fold duplicate CAVP tests into one file-list test; inline the SHA*Test constants.

codecov · 2026-07-30T15:31:51Z

Codecov Report

❌ Patch coverage is 78.16092% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.54%. Comparing base (613355a) to head (cc81cdb).

Files with missing lines	Patch %	Lines
csrc/hash_evp_template.cpp.template	77.10%	12 Missing and 7 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #485      +/-   ##
============================================
- Coverage     78.55%   78.54%   -0.02%     
+ Complexity     1217     1216       -1     
============================================
  Files           127      128       +1     
  Lines          7756     7843      +87     
  Branches        982      989       +7     
============================================
+ Hits           6093     6160      +67     
- Misses         1133     1145      +12     
- Partials        530      538       +8

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

havishr requested a review from a team as a code owner July 22, 2025 15:51

havishr closed this Jul 22, 2025

havishr reopened this Jul 22, 2025

havishr closed this Jul 22, 2025

havishr reopened this Jul 22, 2025

havishr enabled auto-merge (squash) July 22, 2025 17:52

havishr marked this pull request as draft July 22, 2025 17:57

auto-merge was automatically disabled July 22, 2025 17:57
Pull request was converted to draft

WillChilds-Klein mentioned this pull request Jul 23, 2025

Quash test details for successful tests #486

Merged

havishr marked this pull request as ready for review July 23, 2025 19:54

havishr enabled auto-merge (squash) July 23, 2025 20:24

havishr self-assigned this Jul 24, 2025

havishr marked this pull request as draft July 24, 2025 13:35

auto-merge was automatically disabled July 24, 2025 13:35
Pull request was converted to draft

havishr marked this pull request as ready for review July 24, 2025 14:31

havishr enabled auto-merge (squash) July 24, 2025 14:36

havishr removed their assignment Aug 1, 2025

WillChilds-Klein reviewed Aug 4, 2025

View reviewed changes

WillChilds-Klein reviewed Aug 8, 2025

View reviewed changes

Comment thread csrc/hash_evp_template.cpp.template Outdated

WillChilds-Klein reviewed Aug 8, 2025

View reviewed changes

Comment thread csrc/hash_evp_template.cpp.template Outdated

WillChilds-Klein reviewed Aug 8, 2025

View reviewed changes

Comment thread csrc/hash_evp_template.cpp.template Outdated

WillChilds-Klein previously approved these changes Aug 8, 2025

View reviewed changes

geedo0 reviewed Aug 14, 2025

View reviewed changes

havishr added 7 commits July 30, 2026 14:37

Created hash_evp_template and added getContextSize, getHashSize, init…

3fe96d6

…Context, and updateContextByteArray with appropriate aws-lc functionality

added all the function implementation for the template to hash_evp_te…

96eeaa4

…mplate.cpp

adjusted the build script for the hash spis to include the sha3 hashi…

b0ea093

…ng. created the cpp for each hash with the appropriate definitions

exposed the functionality of all four sha3 algos in accp. wrote the t…

2e9e82d

…ests for sha3 excluding the HashFunctionTester implementations which need the appropriate zip files of data.

Added test files and pointed to them in the new hash tests

e8333e0

Reworked the init, both updates, and finish in the csrc code to work …

f9002c5

…properly. Adjusted SPI names.

Fixed bug in fast digest. All tests passing with regards to the SPIs.

b184b5d

havishr and others added 14 commits July 30, 2026 14:38

Adjusted variable definitions and shifted CTX definition back onto st…

facbfc2

…ack.

Ran spotlessApply

ceb8ba5

Undo .project file change

752456e

Added checks to SHA3 tests to ensure Java Version is at least 11 beca…

7c4341e

…use SHA3 is not exposed in SUN JDK 8 which is neede for TestAPI and TestRandomly

Applied spotless

435a648

Replaced duplicate constant

0ae51df

Flattened repetitive testing logic for different SHA algorithms into …

677a813

…one test class

Applied spotless

eab6bee

Adjusted to base testing class to maintain separate test classes whil…

d72d1f4

…e consolidating functionality.

Ran spotless

91ecf13

Fixed up changes related to comments including unnecessary files and …

589404b

…cleaning up files

Fixed up comments related to cleaning up certain files

2aca8d9

Added comments to clarify buffer usage.

dc7314e

WillChilds-Klein dismissed their stale review via cc81cdb July 30, 2026 14:44

WillChilds-Klein force-pushed the SHA3_Implementation branch from bb80dde to cc81cdb Compare July 30, 2026 14:44

		// Transform DIGEST_NAME from "sha3_224" format to "SHA3224" format for JNI class names
		// This macro needs to be defined for each digest algorithm

		#include "generated-headers.h"
		#include "util.h"

Uh oh!

Conversation

havishr commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WillChilds-Klein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 30, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

havishr commented Jul 22, 2025 •

edited

Loading