Skip to content

Conversation

@LinuxJedi
Copy link
Member

@LinuxJedi LinuxJedi commented Aug 12, 2025

FIPS doesn't have scrypt. This implements PBKDF2 instead. Enabled by default at 600,000 rounds for FIPS.

600,000 being the current OWASP recommendation for SHA256 HMAC.

#define NO_MD5
#endif

/* FIPSv5 has no scrypt or pbkdf2 and SHA256 is not strong enough */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just make this the default for FIPS, since scrypt won't be in the boundary in any planned version. Also FIPS v6.0 is dev now, but it will move later.

src/internal.c Outdated
enum wc_HashType hashT;

#ifdef HAVE_FIPS
if (kLen < HMAC_FIPS_MIN_KEY)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add some more logging here as this error will be hit often.

src/internal.c Outdated
#endif

hashT = wc_HashTypeConvert(hashType);
hLen = wc_HashGetDigestSize(hashT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding error check for hLen here

Copy link
Contributor

@SparkiDev SparkiDev left a comment

Choose a reason for hiding this comment

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

PBKDF2 is a FIPS algorithm.
Use wolfSSL API.

@LinuxJedi
Copy link
Member Author

PBKDF2 is a FIPS algorithm. Use wolfSSL API.

We can't, it isn't FIPSv5. It was concluded in an engineering call that this would be OK. Is it not?

@LinuxJedi LinuxJedi changed the title Add PBKDF2 option for password hashing Add PBKDF2 in FIPS for password hashing Aug 14, 2025
FIPS doesn't have scrypt. This implements PBKDF2 instead. Enabled by
default at 600,000 rounds for FIPS.

600,000 being the current OWASP recommendation for SHA256 HMAC.
@LinuxJedi
Copy link
Member Author

Moved to use wolfSSL's PBKDF2, it does actually exist in FIPSv5 builds.

cmsutil and ssltap are fixed in pr #136. This will need rebasing on that when merged.

int hashLen, WP11_Slot* slot)
{
#ifdef HAVE_SCRYPT
#ifdef HAVE_FIPS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding a new macro for force this mode as well... There are many embedded systems that would prefer this as scrypt is a memory hog (like 1 MB).

#endif

/* FIPS has no scrypt and SHA256 is not strong enough */
#if defined(HAVE_FIPS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to #ifndef PBKDF2_ITERATIONS.

@LinuxJedi
Copy link
Member Author

After discussions with @kaleb-himes and @MarkAtwood, we won't be doing this.

@LinuxJedi LinuxJedi closed this Aug 20, 2025
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.

4 participants