Repository navigation
refactor(engine): replace magic numbers with named constants - #802
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the refactoring or documented fallback change; tests were not rerun.
No open issues 🎉
What changed in this PR
Replaces magic numbers in the AZIHSM OpenSSL engine with named constants and shared helpers, addressing #731.
Changes:
- Centralizes OpenSSL return-code handling, memory cleanup, and index registration.
- Names domain values and shares constants across engine, resiliency, and tests.
- Changes the documented missing-method RSA control fallback from
0toCTRL_UNSUPPORTED.
| File | Description |
|---|---|
| plugins/ossl_engine/integration-tests/openssl-cli/testfiles/hkdf/hkdf.sh | Uses the named buffer size. |
| plugins/ossl_engine/integration-tests/openssl-cli/testfiles/env.sh | Defines the CLI buffer-size constant. |
| plugins/ossl_engine/integration-tests/openssl-capi/src/engine_capi_tests.rs | Names derive-size expectations. |
| plugins/ossl_engine/integration-tests/masked-keygen/src/main.rs | Reuses secret-file permissions. |
| plugins/ossl_engine/azihsm_ossl_engine/src/sign.rs | Uses shared return-code checking. |
| plugins/ossl_engine/azihsm_ossl_engine/src/rsaload.rs | Reuses return-code and index helpers. |
| plugins/ossl_engine/azihsm_ossl_engine/src/rsaimport.rs | Uses the unwrapping-key size constant. |
| plugins/ossl_engine/azihsm_ossl_engine/src/logging.rs | Formats permissions from their constant. |
| plugins/ossl_engine/azihsm_ossl_engine/src/lib.rs | Reuses resiliency file permissions. |
| plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs | Shares file limits and OpenSSL helpers. |
| plugins/ossl_engine/azihsm_ossl_engine/src/keygen.rs | Uses shared return-code checking. |
| plugins/ossl_engine/azihsm_ossl_engine/src/integration.rs | Exposes secret-file permissions. |
| plugins/ossl_engine/azihsm_ossl_engine/src/hkdf.rs | Names AES sizes; derives HMAC size from digest. |
| plugins/ossl_engine/azihsm_ossl_engine/src/engine_impl.rs | Names the incompatible-version result. |
| plugins/ossl_engine/azihsm_ossl_engine/src/derive.rs | Uses shared OpenSSL cleanup. |
| plugins/ossl_engine/azihsm_ossl_engine/src/context/tests.rs | Reuses constants in tests. |
| plugins/ossl_engine/azihsm_ossl_engine/src/context.rs | Names credential and unwrapping-key sizes. |
| plugins/ossl_engine/azihsm_ossl_engine_sys/build.rs | Allows OpenSSL PSS salt-length constants. |
| plugins/ossl_engine/azihsm_ossl_engine_resiliency/src/lib.rs | Exposes shared storage constants. |
| plugins/ossl_engine/azihsm_ossl_engine_resiliency/src/config.rs | Reuses permission constants in tests. |
| plugins/ossl_engine/azihsm_ossl_engine_resiliency/src/callbacks.rs | Exposes the OBK length constant. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/rsa_pkey_method.rs | Names RSA conventions and updates control fallback. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/rsa_method.rs | Uses shared return-code handling. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/pkey_method.rs | Reuses result and derive-size constants. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/method_table.rs | Exposes derive sizes and names dispatch results. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/mem.rs | Adds shared OpenSSL cleanup. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/lib.rs | Exports memory helpers and derive sizes. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/hkdf_method.rs | Uses named modes, sizes, and result helpers. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/exdata.rs | Centralizes index registration. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs | Adds result helpers and deferred error conversion. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/engine.rs | Avoids eager error-message allocation. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/cipher_method.rs | Shares control constants and result helpers. |
| plugins/ossl_engine/azihsm_ossl_engine_core/src/asn1_method.rs | Names encoding values and reuses helpers. |
Review effort: Balanced
Christian Walter (walterchris)
force-pushed
the
engine/magic-numbers
branch
from
October 21, 2026 06:39
3e5642d to
b7b0a99
Compare
Christian Walter (walterchris)
force-pushed
the
engine/magic-numbers
branch
from
October 26, 2026 20:42
b7b0a99 to
24cc55a
Compare
Check OpenSSL 1/0 results with ossl_check/ossl_ok and RetCode, ctrl results with ctrl_ok (any positive value is success), and name the ctrl conventions and wildcards. Use OpenSSL's own values where they exist (RSA_PSS_SALTLEN_*, EVP_PKEY_HKDEF_MODE_*, NID_undef) and name the rest: RSA key sizes, PSS overhead, credential length, AES key sizes and the derive size-query answers. Define shared values once: the secret file mode, file-size cap and OBK length in the resiliency crate, ex_data index registration and OPENSSL_free in core. Tests use the constants they restate. The RSA pkey ctrl now returns CTRL_UNSUPPORTED, like the EC and HKDF methods, when the built-in method is missing. Signed-off-by: Christian Walter <christian.walter@9elements.com>
Christian Walter (walterchris)
force-pushed
the
engine/magic-numbers
branch
from
October 26, 2026 22:41
24cc55a to
bba7ce9
Compare
Rajesh Gali (rajesh-gali)
approved these changes
Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #731. Removes the magic numbers from the engine's production code. No behaviour change except one unreachable path (below).
Changes
ossl_check/ossl_ok/RetCode(coreerror.rs).ossl_checkis nowpuband converts its error only on failure, so a&'static strmessage costs nothing on success.ctrl_ok: any positive value is success (GET_RSA_OAEP_LABEL, for one, returns a length).RetCode::Fail; void cleanups use().CTRL_UNSUPPORTED(-2),CTRL_NOT_IMPLEMENTED(-1),ANY_KEYTYPE/ANY_OPTYPE(-1).RSA_PSS_SALTLEN_*, now allowlisted; the hand-copied ones are gone.EVP_PKEY_HKDEF_MODE_*andNID_undef.DEFAULT_RSA_KEYGEN_BITS(OpenSSL's default) andRSA_UNWRAPPING_KEY_BITS(the HSM's fixed key): both 2048, different meanings.PSS_ENCODING_OVERHEAD,CREDENTIALS_SIZE/_HEX_SIZE,AES_KEY_BITS,PKCS8_VERSION_V1,FILE_OUTPUT_SIZE_HINT.SECRET_FILE_MODE,MAX_STORAGE_FILE_SIZEandOBK_LENlive in the resiliency crate; the engine re-exports them.new_ex_index()replaces three copies of the ex_data index registration, andopenssl_free()replaces four copies ofOPENSSL_free.kdflenin the Rust tests, the C-API tests andhkdf.sh(from the HKDF over masked secrets through the engine #694 review).Behaviour change
When OpenSSL's built-in method is missing, the RSA pkey ctrl returned 0. It now returns
CTRL_UNSUPPORTED(-2), like the EC and HKDF methods. This path is not reachable in practice.Testing
All run in the u2004 container (OpenSSL 1.1.1f):
cargo fmt --checkcargo clippy --all-targets -- -D warningsfor engine, engine+mock, the integration-test crates and resiliency