Update wolfProvider docs following v1.1.0#231
Conversation
aidangarske
left a comment
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: audit + reviewOverall recommendation: REQUEST_CHANGES
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [High] [review] New OpenSSL key generation example uses the wrong API argument —
wolfProvider/src/chapter03.md:56-58 - [High] [review] Component table defines an all-components enum with the wrong name —
wolfProvider/src/chapter05.md:51-52 - [High] [review] Runtime component examples use level-prefixed or undocumented component names —
wolfProvider/src/chapter05.md:73-81 - [Medium] [review] Runtime API is now described as build-time component logging —
wolfProvider/src/chapter05.md:37-61 - [Low] [review] Typo in new replace-default documentation —
wolfProvider/src/chapter03.md:63
Review generated by Skoll
| With stock OpenSSL, it's still possible to access the default provider delivered with OpenSSL, even with the proper configuration. For example, software may call into OpenSSL EVP functions with a specific provider context, similar to how the unit tests work: | ||
|
|
||
| ``` | ||
| osslLibCtx = OSSL_LIB_CTX_new(); |
There was a problem hiding this comment.
🔴 [High] New OpenSSL key generation example uses the wrong API argument · bug
The new example passes osslLibCtx directly to EVP_PKEY_keygen, but EVP_PKEY_keygen operates on an EVP_PKEY_CTX, not an OSSL_LIB_CTX. This PR introduces a non-compilable OpenSSL example in the section explaining provider-context bypasses.
Fix: Update the snippet to use an EVP_PKEY_CTX created from the library context, or make the example explicit pseudocode instead of a C API sequence.
| | WP_LOG_COMP_KE | Key Agreement Algorithms (DH, ECDH) | 0x0020 | | ||
| | WP_LOG_COMP_KDF | Password Based Key Derivation Algorithms | 0x0040 | | ||
| | WP_LOG_COMP_PROVIDER | All provider specific logs | 0x0080 | | ||
| | WP_LOG_COMP_COMP_ALL | Log all components | WP_LOG_COMP_RNG | WP_LOG_COMP_DIGEST | WP_LOG_COMP_MAC | WP_LOG_COMP_CIPHER | WP_LOG_COMP_PK | WP_LOG_COMP_KE | WP_LOG_COMP_KDF | WP_LOG_COMP_PROVIDER | |
There was a problem hiding this comment.
🔴 [High] Component table defines an all-components enum with the wrong name · bug
The PR changes the component enum table to list WP_LOG_COMP_COMP_ALL, but the very next row uses WP_LOG_COMP_ALL as the default value. That leaves the documented all-components enum inconsistent and likely invalid for users copying the table.
Fix: Rename the table entry to the actual all-components enum and keep the default row consistent with it.
| *Note that the environment variables can only control levels and components which have been enabled at build time.* | ||
|
|
||
| ``` | ||
| # Enable info logs only from the ED25519 block. |
There was a problem hiding this comment.
🔴 [High] Runtime component examples use level-prefixed or undocumented component names · bug
The new runtime examples set WOLFPROV_LOG_COMPONENTS with WP_LOG_COMP_ED25519, which is not present in the section's claimed full component list, and with WP_LOG_LEVEL_RSA | WP_LOG_LEVEL_PROVIDER, which uses the log-level prefix for component selection. This makes the new environment-variable examples internally inconsistent and likely unusable.
Fix: Use only valid WP_LOG_COMP_* component names in WOLFPROV_LOG_COMPONENTS, and add any newly supported components such as ED25519/RSA to the table if they really exist.
| ``` | ||
|
|
||
| ## Controlling Component Logging | ||
| ## Controlling Component Logging at build time |
There was a problem hiding this comment.
🟠 [Medium] Runtime API is now described as build-time component logging · convention
The PR retitles the existing wolfProv_SetLogComponents(int mask) guidance as build-time control, but the API shown is a runtime setter. That conflicts with the new following section about runtime environment variables and can mislead readers about when the setting takes effect.
Fix: Retitle this section to describe API-based component selection, or add a separate build-time section that documents actual compile-time controls.
|
|
||
| For many use cases, this is not ideal because it allows calls to silently bypass wolfProvider and utilize OpenSSL cryptographic functions. | ||
|
|
||
| As an alternative, the OpenSSL build created by this repo can optionally disable the stock provider from OpenSSL and replace it with wolfProvider. We belive this to be a more robust way of ensuring wolfProvider is the crypto backend. |
There was a problem hiding this comment.
🔵 [Low] Typo in new replace-default documentation · style
The new paragraph contains belive, which is a typo in user-facing documentation.
Fix: Fix the typo before publishing the updated manual.
3 participants
New and updated documentation about Debian packaging and logging updates.