Skip to content

HMAC added to dgst#257

Open
aidankeefe2022 wants to merge 1 commit into
wolfSSL:mainfrom
aidankeefe2022:HMAC-SHA-Key-Fix
Open

HMAC added to dgst#257
aidankeefe2022 wants to merge 1 commit into
wolfSSL:mainfrom
aidankeefe2022:HMAC-SHA-Key-Fix

Conversation

@aidankeefe2022

@aidankeefe2022 aidankeefe2022 commented Jun 23, 2026

Copy link
Copy Markdown
Member

Added new flags to the dgst sub command.
Created 3 helper functions to make the HMAC and sign/verify functionalities easy to read and use.
Added end-to-end tests for HMAC RFC Vectors using hex key and plain text key.
Changed input to allow for data to be passed to stdin for dgst.

@aidankeefe2022 aidankeefe2022 self-assigned this Jun 23, 2026
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 23, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #257

Scan targets checked: wolfclu-bugs, wolfclu-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/sign-verify/clu_dgst_setup.c Outdated
aidankeefe2022
aidankeefe2022 commented Jun 23, 2026
View reviewed changes
Comment thread src/sign-verify/clu_dgst_setup.c Outdated
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 23, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #257

Scan targets checked: wolfclu-bugs, wolfclu-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/sign-verify/clu_dgst_setup.c Outdated
@aidankeefe2022 aidankeefe2022 force-pushed the HMAC-SHA-Key-Fix branch 2 times, most recently from ee5ebd0 to 1883df8 Compare June 23, 2026 20:17
aidankeefe2022
aidankeefe2022 commented Jun 23, 2026
View reviewed changes
Comment thread src/sign-verify/clu_dgst_setup.c Outdated
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 23, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #257

Scan targets checked: wolfclu-bugs, wolfclu-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/sign-verify/clu_dgst_setup.c Outdated
aidankeefe2022
aidankeefe2022 commented Jun 23, 2026
View reviewed changes
Comment thread src/sign-verify/clu_dgst_setup.c Outdated
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 23, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #257

Scan targets checked: wolfclu-bugs, wolfclu-src

No new issues found in the changed files. ✅

@cconlon cconlon requested a review from Copilot June 23, 2026 22:43
Copilot AI reviewed Jun 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds HMAC support to the existing dgst subcommand (alongside sign/verify), including new CLI flags, a streaming HMAC helper, and end-to-end RFC vector tests. It also updates dgst input handling so data can be supplied via stdin when no positional input file is present.

Changes:

  • Add -hmac / -mackey flags to dgst and update help text + stdin/positional input handling.
  • Introduce a streaming HMAC implementation (wolfCLU_hmacHash) and an HMAC dispatch helper in dgst setup.
  • Add end-to-end Python tests covering RFC vectors, output-to-file behavior, and key parsing edge cases.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
wolfclu/clu_optargs.h Adds new option IDs for dgst HMAC flags.
wolfclu/clu_header_main.h Exposes the new wolfCLU_hmacHash() API (with updated includes/docs).
src/tools/clu_funcs.c Implements streaming HMAC helper used by dgst.
src/sign-verify/clu_dgst_setup.c Extends dgst to support HMAC mode and stdin input dispatch.
tests/dgst/dgst-test.py Adds HMAC RFC vector and behavior tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfclu/clu_header_main.h
Comment thread src/sign-verify/clu_dgst_setup.c
Comment thread src/tools/clu_funcs.c
Comment thread src/tools/clu_funcs.c
Comment thread tests/dgst/dgst-test.py
Comment thread src/sign-verify/clu_dgst_setup.c
billphipps
billphipps previously approved these changes Jun 24, 2026
View reviewed changes

@billphipps billphipps left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot comments are pretty good. A few minor nits, but this looks like a great addition!

Comment thread src/tools/clu_funcs.c
/* wc_HashType values are not contiguous, so map each one explicitly.
* Cast to int so unrelated hash types don't trip -Wswitch-enum. */
switch ((int)alg) {
case WC_HASH_TYPE_MD5:

@billphipps billphipps Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to check for NO_MD5 or other conditional compilation flags?

@aidankeefe2022 aidankeefe2022 Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--enable-wolfclu option for wolfSSL turns on MD5 so we are good for now! And for the other hash alg guards it is a pretty pervasive issue Ill probably make a separate PR that guards all of the hash algs because it touches a lot of files. --disable-hmac won't build with --enable-wolfclu so we are safe there.

Comment thread src/sign-verify/clu_dgst_setup.c
/* return WOLFCLU_SUCCESS on success */
int wolfCLU_dgst_setup(int argc, char** argv)
{
#ifndef WOLFCLU_NO_FILESYSTEM

@billphipps billphipps Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrmm... If the input is stdin, the output is stdout, and all key materials are on command line, why do we need a filesystem? Ok to punt to a later feature request

@aidankeefe2022 aidankeefe2022 Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very true! This is an upgrade we could make across a ton of functionalities so if it comes up in the future it would be easy to do it all at once.

Comment thread src/sign-verify/clu_dgst_setup.c Outdated
*sep = '\0'; /* terminate the type token */
macKeyVal = sep + 1; /* value is the remainder, ':' included */

if (XSTRCMP(hmacKey, "hexkey") == 0) {

@billphipps billphipps Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using XSTRNCMP since you know the length of "hexkey"

@aidankeefe2022 aidankeefe2022 Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment thread src/sign-verify/clu_dgst_setup.c
Comment on lines +85 to +86
WOLFCLU_LOG(WOLFCLU_L0, "\t-mackey key:value for plain text "
"hexkey:value to have value encoded\n");

@billphipps billphipps Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WOLFCLU_LOG(WOLFCLU_L0, "\t-mackey key:value for plain text "
"hexkey:value to have value encoded\n");
WOLFCLU_LOG(WOLFCLU_L0, "\t-mackey key:value for plain text hmac key"
"hexkey:value for hex-encoded hmac key\n");

@aidankeefe2022 aidankeefe2022 Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@aidankeefe2022
added HMAC to dgst
48c3ba3 
added invalid argument tests

increased plain test key size for FIPs build

fix arg name conflict

github fixes

skoll fixes
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 24, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #257

Scan targets checked: wolfclu-bugs, wolfclu-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/tools/clu_funcs.c
@aidankeefe2022

aidankeefe2022 commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Jenkins retest this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left review comments
@billphipps billphipps Awaiting requested review from billphipps

At least 1 approving review is required to merge this pull request.

Assignees

@billphipps billphipps

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@aidankeefe2022 @billphipps @wolfSSL-Fenrir-bot @cconlon @wolfSSL-Bot