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

Curve25519 fixes #83

Merged
merged 8 commits into from
Nov 4, 2024
Merged

Curve25519 fixes #83

merged 8 commits into from
Nov 4, 2024

Conversation

bigbrett
Copy link
Contributor

@bigbrett bigbrett commented Nov 1, 2024

Fixes curve25519 implementation, changing it to use new wolfCrypt DER API for ser/de instead of raw keys

@bigbrett
Copy link
Contributor Author

bigbrett commented Nov 1, 2024

Just noticed wolfCrypt tests are failing - didn't try that locally. I should turn that on by default. I'll investigate

@bigbrett bigbrett removed the request for review from billphipps November 1, 2024 22:22
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up! I had a couple of questions about asn.h.

src/wh_crypto.c Outdated
@@ -36,6 +36,7 @@
#include "wolfssl/wolfcrypt/types.h"
#include "wolfssl/wolfcrypt/error-crypt.h"
#include "wolfssl/wolfcrypt/asn.h"
#include "wolfssl/wolfcrypt/asn_public.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

asn.h include asn_public.h. Were you getting a compile warning/error without asn_public.h explicitly included?

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'm dumb, removed

src/wh_crypto.c Outdated
Comment on lines 212 to 225
#ifdef HAVE_CURVE25519
#ifdef HAVE_CURVE25519_KEY_IMPORT
WOLFSSL_API int wc_Curve25519PrivateKeyDecode(
const byte* input, word32* inOutIdx, curve25519_key* key, word32 inSz);
WOLFSSL_API int wc_Curve25519PublicKeyDecode(
const byte* input, word32* inOutIdx, curve25519_key* key, word32 inSz);
#endif
#ifdef HAVE_CURVE25519_KEY_EXPORT
WOLFSSL_API int wc_Curve25519PrivateKeyToDer(
curve25519_key* key, byte* output, word32 inLen);
WOLFSSL_API int wc_Curve25519PublicKeyToDer(
curve25519_key* key, byte* output, word32 inLen, int withAlg);
#endif
#endif /* HAVE_CURVE25519 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these already declared in asn_public.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, this should have been removed. fixed

billphipps
billphipps previously approved these changes Nov 4, 2024
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

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

LGTM!

@bigbrett bigbrett requested a review from billphipps November 4, 2024 19:37
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

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

Excellent!

@billphipps billphipps merged commit 5e719bc into wolfSSL:main Nov 4, 2024
2 checks passed
jefferyq2 pushed a commit to jefferyq2/wolfHSM that referenced this pull request Nov 10, 2024
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