Skip to content

crypto/cryptosoft: Add support for PBKDF2#18608

Open
PruteanuVlad wants to merge 1 commit intoapache:masterfrom
PruteanuVlad:add_pbkdf2_support
Open

crypto/cryptosoft: Add support for PBKDF2#18608
PruteanuVlad wants to merge 1 commit intoapache:masterfrom
PruteanuVlad:add_pbkdf2_support

Conversation

@PruteanuVlad
Copy link
Contributor

Summary

This adds support for PBKDF2 (SHA1 and SHA256) while leveraging the existing infrastructure for HMAC.
I tried to take advantage of the existing HMAC code as much as possible.
Test app has been added in this PR: apache/nuttx-apps#3437

Modified files:

boards/xtensa/esp32/esp32-devkitc/configs/crypto/defconfig - enables the test app
crypto/cryptodev.c - treats the new PBKDF2 cases and handles new struct field
crypto/cryptosoft.c - handles the actual algorithm implementation
include/crypto/cryptodev.h - new defines for pbkdf2

Impact

This allows users to derive keys using PBKDF2.

##Testing

Development was done using ESP32 DevkitC.
Building was done on Ubuntu 24.04 VM.
For testing, I ran the PBKDF2 test from the PR mentioned above.

nsh> pbkdf2
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA1 success
hmac PBKDF2 SHA256 success
hmac PBKDF2 SHA256 success
hmac PBKDF2 SHA256 success
hmac PBKDF2 SHA256 success
hmac PBKDF2 SHA256 success
hmac PBKDF2 SHA256 success

@github-actions github-actions bot added Size: S The size of the change in this PR is small Area: Crypto Board: xtensa labels Mar 25, 2026
@ThePassionate
Copy link
Contributor

Hi, thanks for adding PBKDF2 support — this is a useful addition to the crypto stack! I have a few suggestions below, mostly around cleanup.

  1. swcr_freesession: missing PBKDF2 — memory leak

    swcr_freesession() does not handle CRYPTO_PBKDF2_HMAC_SHA1 / CRYPTO_PBKDF2_HMAC_SHA256. These algorithms go through the authcommon path in swcr_newsession(), which allocates both sw_ictx and sw_octx. Without corresponding cases in swcr_freesession(), neither buffer is freed on session teardown — this is a memory leak.

  2. targetlen added to struct crypt_op but never referenced

@simbit18
Copy link
Contributor

Hi @PruteanuVlad please fix

====================================================================================
Configuration/Tool: sim/crypto
2026-03-25 22:54:34
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Building NuttX...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 36922    0 36922    0     0  51101      0 --:--:-- --:--:-- --:--:-- 51101
100  143k    0  143k    0     0   192k      0 --:--:-- --:--:-- --:--:--  560k

100 5740k    0 5740k    0     0  4058k      0 --:--:--  0:00:01 --:--:-- 8255k
cryptosoft.c: In function 'swcr_process':
Error: cryptosoft.c:2057:13: error: implicit declaration of function 'swcr_pbkdf2' [-Werror=implicit-function-declaration]
 2057 |             swcr_pbkdf2(crp, crd, sw, crp->crp_buf);
      |             ^~~~~~~~~~~
cryptosoft.c: In function 'swcr_pbkdf2':
Error: cryptosoft.c:2119:21: error: pointer targets in assignment from 'uint8_t *' {aka 'unsigned char *'} to 'caddr_t' {aka 'char *'} differ in signedness [-Werror=pointer-sign]
 2119 |   crp_dummy.crp_mac = macbuf;
      |                     ^
Error: cryptosoft.c:2126:48: error: implicit declaration of function 'htonl' [-Werror=implicit-function-declaration]
 2126 |       *(uint32_t *)(saltblk + crp->crp_ilen) = htonl(blocknum);
      |                                                ^~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:98: cryptosoft.o] Error 1
make[1]: Target 'libcrypto.a' not remade because of errors.
make: *** [tools/LibTargets.mk:95: crypto/libcrypto.a] Error 2
make: Target 'all' not remade because of errors.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 385: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  [1/1] Normalize sim/crypto
====================================================================================

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

@PruteanuVlad please update the Documentation to include this algorithm as supported

This adds support for PBKDF2 (SHA1 and SHA256) while leveraging
the existing infrastructure for HMAC.

Signed-off-by: Vlad Pruteanu <pruteanuvlad1611@yahoo.com>
@github-actions github-actions bot added Size: M The size of the change in this PR is medium and removed Size: S The size of the change in this PR is small labels Mar 26, 2026
@PruteanuVlad
Copy link
Contributor Author

Thanks for the review 😄
I went through a few revisions so there were some missed leftovers, but I cleaned it up now.

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

Labels

Area: Crypto Board: xtensa Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants