Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add wolfSSL_Init to csr example, required for fips ecc #270

Merged
merged 4 commits into from
May 23, 2023

Conversation

jpbland1
Copy link
Contributor

ZD 16126

@jpbland1 jpbland1 requested a review from dgarske May 23, 2023 15:50
@jpbland1 jpbland1 self-assigned this May 23, 2023
@jpbland1 jpbland1 marked this pull request as ready for review May 23, 2023 15:56
@dgarske dgarske self-assigned this May 23, 2023
@@ -166,6 +166,13 @@ int TPM2_CSR_ExampleArgs(void* userCtx, int argc, char *argv[])
argc--;
}

/* init wolfssl, required for fips */
rc = wolfSSL_Init();
Copy link
Contributor

@dgarske dgarske May 23, 2023

Choose a reason for hiding this comment

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

This is great, but a couple things.

  1. If building with WOLFCRYPT_ONLY this needs to be wolfCrypt_Init().
  2. Use the wolfSSL values WOLFSSL_SUCCESS.
  3. If you are going to call init you also need to call _Cleanup()... For WOLFCRYPT_ONLY this needs to be wolfCrypt_Cleanup()

@dgarske dgarske removed their assignment May 23, 2023
@jpbland1 jpbland1 requested a review from dgarske May 23, 2023 16:15
@@ -166,9 +166,30 @@ int TPM2_CSR_ExampleArgs(void* userCtx, int argc, char *argv[])
argc--;
}

/* init wolfssl, required for fips */
#ifdef WOLFCRYPT_ONLY
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic should be in wolfTPM2_Init and wolfTPM2_Cleanup? I am pretty sure these init/cleanups are already called. Perhaps the issue is I only call the wolfCrypt_Init version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, when I try to do just wolfCrypt_Init it fails with the rng error, I have to call wolfSSL_Init

Copy link
Contributor

Choose a reason for hiding this comment

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

Please swap out the logic you added in csr.c, but put into tpm2.c and add the #ifdef WOLFCRYPT_ONLY logic. Call wolfSSL_Init by default unless WOLFCRYPT_ONLY is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/wolfSSL/wolfssl/blob/master/src/ssl.c#L6421
I think this is why, I'm going to call wolfSSL_Init when not WOLFCRYPT_ONLY and call wolfCrypt_Init and wc_SetSeed_Cb when it is

@jpbland1 jpbland1 requested a review from dgarske May 23, 2023 17:31
@jpbland1
Copy link
Contributor Author

So you don't need to call wolfSSL_Init, I just needed to add the wc_SetSeed_Cb call if WC_RNG_SEED_CB is defined

@dgarske dgarske merged commit a92732d into wolfSSL:master May 23, 2023
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.

2 participants