-
Notifications
You must be signed in to change notification settings - Fork 918
Add STSAFE-A120 Support #9614
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
base: master
Are you sure you want to change the base?
Add STSAFE-A120 Support #9614
Conversation
|
🛟 Devin Lifeguard found 3 likely issues in this PR
@dgarske |
|
Jenkins retest this please: "AgentOfflineException" |
wolfcrypt/src/port/st/stsafe.c
Outdated
| 0, 0, 4, &readBuf, STSAFE_A_NO_MAC); | ||
|
|
||
| if (status_code == STSAFE_A_OK && readBuf->Length == 4 && | ||
| readBuf->Data[0] == 0x30) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of 0x30, 0x81, and 0x82 use ASN_SEQUENCE | ASN_CONSTRUCTED and the ASN length values. If this is hard set to avoid needed NO_ASN not defined then please add comments as to what the values are here.
| } | ||
|
|
||
| if (rc == STSAFE_A_OK && *pCertLen > 0) { | ||
| step = 223 - (stsafe_a->CrcSupport ? 2 : 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic number here of 223.
wolfcrypt/src/port/st/stsafe.c
Outdated
| r_length = ((uint16_t)signature->Data[0] << 8) | signature->Data[1]; | ||
| s_length = ((uint16_t)signature->Data[2 + r_length] << 8) | | ||
| signature->Data[3 + r_length]; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be bounds checks here on r_length and s_length before use. It's unlikely that the ST function will return invalid or unexpected data with a status_code return of STSAFE_A_OK but better to have a sanity check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
Jenkins retest this please: "AgentOfflineException" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ret = stse_generate_ecc_key_pair(&g_stse_handler, slot, | ||
| (stse_ecc_key_type_t)curve_id, | ||
| STSAFEA_EPHEMERAL_KEY_USAGE_LIMIT, |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro STSAFEA_EPHEMERAL_KEY_USAGE_LIMIT is used but never defined anywhere in the codebase. This will cause a compilation error. This should likely be a numeric literal (e.g., 255) or defined as a macro in the header file.
| if (need_ephemeral_key) { | ||
| /* Key is in slot 1 (for ECDSA), but ECDH requires ephemeral slot. | ||
| * Generate ephemeral key pair for ECDH. Note: This will overwrite any | ||
| * existing key in ephemeral slot, so for bidirectional ECDH, both keys | ||
| * should be generated in ephemeral slot from the start. */ | ||
| stse_ReturnCode_t ret; | ||
| byte ephemeralPubKey[STSAFE_MAX_PUBKEY_RAW_LEN]; | ||
| int key_sz = stsafe_get_key_size(curve_id); | ||
| slot = STSAFE_KEY_SLOT_EPHEMERAL; | ||
|
|
||
| ret = stse_generate_ecc_key_pair(&g_stse_handler, slot, | ||
| (stse_ecc_key_type_t)curve_id, | ||
| STSAFEA_EPHEMERAL_KEY_USAGE_LIMIT, | ||
| ephemeralPubKey); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code block uses A120-specific types (stse_ReturnCode_t) and functions (stse_generate_ecc_key_pair) without #ifdef WOLFSSL_STSAFEA120 guards. This will cause compilation errors when building with WOLFSSL_STSAFEA100 defined. The entire block from lines 1854-1892 should be wrapped in #ifdef WOLFSSL_STSAFEA120 ... #else ... #endif to provide appropriate implementation for both A100 and A120.
| #ifdef WOLFSSL_STSAFEA120 | ||
| stse_ReturnCode_t ret; | ||
| slot = STSAFE_KEY_SLOT_1; /* Use persistent slot for ECDSA signing */ | ||
| ret = stse_generate_ecc_key_pair(&g_stse_handler, slot, | ||
| (stse_ecc_key_type_t)curve_id, | ||
| 255, /* usage_limit for persistent keys */ | ||
| pubKeyRaw); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 255 for usage_limit should be defined as a named constant (e.g., STSAFE_PERSISTENT_KEY_USAGE_LIMIT) for better code maintainability and consistency. This same value should be used throughout the codebase where applicable.
| byte ephemeralPubKey[STSAFE_MAX_PUBKEY_RAW_LEN]; | ||
| int key_sz = stsafe_get_key_size(curve_id); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable key_sz is declared but only used within the STSAFE-A120 specific code path at line 1881. This variable declaration should be moved inside the A120-specific #ifdef block (after adding the missing guards) to avoid unused variable warnings.
| byte ephemeralPubKey[STSAFE_MAX_PUBKEY_RAW_LEN]; | |
| int key_sz = stsafe_get_key_size(curve_id); | |
| byte ephemeralPubKey[STSAFE_MAX_PUBKEY_RAW_LEN]; | |
| #ifdef WOLFSSL_STSAFEA120 | |
| int key_sz = stsafe_get_key_size(curve_id); | |
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Add STSAFE-A120 Support via STSELib
Description
This PR adds support for the ST STSAFE-A120 secure element using the open-source STSELib SDK. The STSAFE-A120 is ST's latest secure element with enhanced cryptographic capabilities and is the successor to the STSAFE-A100/A110 series.
Changes
Files Modified:
wolfcrypt/src/port/st/stsafe.c- Added STSAFE-A120/STSELib implementationwolfssl/wolfcrypt/port/st/stsafe.h- Added type abstractions and curve mappingswolfcrypt/src/wc_port.c- Updated STSAFE initializationwolfcrypt/src/port/st/README.md- Added documentationFeatures
Configuration
Enable with:
Dependencies
API Compatibility
The new implementation maintains API compatibility with the existing STSAFE-A100 code:
stsafe_interface_init()- Initialize devicewolfSSL_STSAFE_CryptoDevCb()- Crypto callback handlerSSL_STSAFE_*callback functions for TLS integrationTesting
Tested on Raspberry Pi 5 with STSAFE-A120 connected via I2C:
Performance (Raspberry Pi 5)
Notes
stse_conf.hstsafe.hdepend on which curves are enabled instse_conf.hRelated
ZD 20780