Skip to content

Add crypto callback support for copy/free operations (SHA-256)#9309

Merged
bigbrett merged 3 commits intowolfSSL:masterfrom
night1rider:CryptoCbCopy
Oct 21, 2025
Merged

Add crypto callback support for copy/free operations (SHA-256)#9309
bigbrett merged 3 commits intowolfSSL:masterfrom
night1rider:CryptoCbCopy

Conversation

@night1rider
Copy link
Contributor

Proposing a general Copy Callback (and probably a Free Callback) for users to be able to use if they have hardware. Ran into a situation where this is more elegant solution than trying to scheme a way to produce copies and handle contexts coming from hardware, along with freeing. The goal would to be able to have independent devCtx's. The hardware can only open up so many "Streams", so I need to keep track of the context and close them down as soon as I am done with the operation.

Looking over wc_Sha256Free we would probably also want to include a callback to free the context there as well.

The way I am currently utilizing the hardware (Very Basic Overview):

graph LR
    A[wc_Sha256Update<br>via CryptoCb] --> B[Start Hardware Stream]
    B --> C{devCtx != NULL?}
    C -->|Yes| D[Import Hardware Context<br/>from devCtx to Hardware]
    C -->|No| E[Fresh Stream<br/>No import needed]
    D --> F[Request Hash Update <br/>Hash/Update]
    E --> F
    F --> G[Wait for Hardware Response]
    G --> H[Free Old DevCtx then<br/>Export Hardware Context<br/>to devCtx]
    H --> I[Close Hardware Stream]
    I --> J[Return Success]
Loading

The way I am currently utilizing the hardware for finishing a sha256 stream:

graph LR
    Start[wc_Sha256Final<br/>via CryptoCb] --> OpenStream[Start hardware Stream]
    OpenStream --> CheckCtx{devCtx != NULL?}
    CheckCtx -->|Yes| Import[Import Hardware Context<br/>Restore hash state]
    CheckCtx -->|No| Init[Fresh Stream<br/>Fresh hash operation]
    Import --> Finalize[Request Finalization]
    Init --> Finalize
    Finalize --> Wait[Wait for Hardware<br/>Operation to Complete]
    Wait --> Close[Close Hardware Stream]
    Close --> FreeCtx{devCtx allocated?}
    FreeCtx -->|Yes| Free[Free devCtx]
    FreeCtx -->|No| End[Return Success<br/>with digest]
    Free --> End
Loading

This is the current situation:
Copy then Free:

graph TD
    subgraph "Current Behavior"
    A[wc_Sha256 Context A<br/>devCtx pointer 0x01] -->|wc_Sha256Copy| B{Shallow Copy}
    B -->|src| C[Context A<br/>devCtx = 0x01]
    B -->|dst| D[Context B<br/>devCtx = 0x01<br/><span style="color:red">SAME POINTER!</span>]
    C -->|wc_Sha256Final| E[Context A Freed<br/>devCTX at 0x01]
    D -->|After wc_Sha256Final: wc_Sha256Update| F[Context B tries to use<br/>devCTX at 0x01<br/><span style="color:red">ALREADY FREED!</span>]
    end
Loading
graph TD
    subgraph "Copy with Crypto Callback"
    A[wc_Sha256 Context A<br/>devCtx pointer 0x01] -->|wc_Sha256Copy| B{Copy<br/>via CryptoCb}
    B -->|src| C[Context A<br/>devCtx = 0x01]
    B -->|dst| D[Context B<br/>devCtx = 0x02<br/><span style="color:green">NEW STREAM!</span>]
    C -->|wc_Sha256Final| E[Context A Freed<br/>dev 0x01]
    D -->|Later: wc_Sha256Update| F[✓ Context B uses<br/>devCtx at 0x02<br/><span style="color:green">Independent Stream</span>]
    end
Loading

@bigbrett
Copy link
Contributor

bigbrett commented Oct 16, 2025

@dgarske just some additional context/background for you...Very interested to hear your thoughts.

One tempting alternative @night1rider and I had discussed would be to modify the existing cryptoCallbacks for algorithms that DO support a "copy" operation to hold dst/src in the info struct and a flag or enum field indicating the type of operations (init/update/final/copy/free).

I think there are pros and cons to both approaches, but we ultimately decided on proposing this one.

Cons to this approach mean maintaining an additional enum type indicating to the caller exactly which type the copy was invoked on (so they know what type they should cast the void* src/dst pointers to in their crypto callback copy handler).

Pros mean we don't break existing customer crypto callbacks by modifying the info structs their applications are currently using (which could fail in all sorts of weird ways given that it is a union type). This was deemed more important that a potentially simpler/cleaner implementation in the other solution path.

We were also hoping to integrate this into the CMD crypto callback, but decided against it for the same reason -didn't want to break existing customer/port cryptoCbs

@night1rider night1rider force-pushed the CryptoCbCopy branch 7 times, most recently from 8d88da3 to 554db10 Compare October 17, 2025 15:36
@night1rider night1rider force-pushed the CryptoCbCopy branch 3 times, most recently from a05613d to 461a67e Compare October 20, 2025 16:01
…so split macros for FREE and COPY Callbacks, and add configure.ac option.
@night1rider
Copy link
Contributor Author

Had to rerun: [OpenWrt test / Compile container (21.02.7)]

@night1rider
Copy link
Contributor Author

Jenkins Please Retest This

@night1rider night1rider marked this pull request as ready for review October 20, 2025 18:56
@devin-ai-integration
Copy link
Contributor

🛟 Devin Lifeguard found 1 likely issues in this PR

  • declare-const-pointers snippet: Change the signature of wc_CryptoCb_Copy so the first copy pointer is
    read-only, e.g. int wc_CryptoCb_Copy(int devId, int algo, int type, const void* src, void* dst);.

@night1rider
please take a look at the above issues which Devin flagged. Devin will not fix these issues automatically.

@dgarske dgarske self-requested a review October 20, 2025 19:33
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Very nice!

@dgarske dgarske removed their assignment Oct 20, 2025
@bigbrett bigbrett merged commit 1134d24 into wolfSSL:master Oct 21, 2025
261 of 262 checks passed
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