Report a failed PEM decode as WS_PARSE_E, not WS_BAD_FILE_E - #1151
Report a failed PEM decode as WS_PARSE_E, not WS_BAD_FILE_E#1151yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines wolfSSH’s certificate/key loading error semantics so malformed PEM bodies are consistently reported as WS_PARSE_E (parse failure) rather than WS_BAD_FILE_E (file I/O failure), especially for buffer-based APIs that never touch the filesystem.
Changes:
- Change PEM decode failure returns from
WS_BAD_FILE_EtoWS_PARSE_Ein the PEM certificate decode path (DoPemCert) and the generic buffer processing PEM arm (wolfSSH_ProcessBuffer). - Document the intended split of
WS_BAD_FILE_E(file read problems only) vsWS_PARSE_E(malformed but recognized content) in the public header. - Add negative tests to pin the corrected return codes for malformed PEM certificate/key bodies across several APIs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wolfssh/ssh.h | Documents the intended separation between file I/O failures and decode/parse failures for cert reading APIs. |
| src/ssh.c | Updates the certificate PEM decode failure to return WS_PARSE_E and clarifies the debug log message. |
| src/internal.c | Updates generic PEM decode failure handling in wolfSSH_ProcessBuffer() to return WS_PARSE_E. |
| tests/api.c | Adds regression tests ensuring malformed PEM bodies return WS_PARSE_E for buffer-based cert/key loading APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
363f8dd to
6de64a0
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1151
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
6de64a0 to
3e1db09
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1151
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
3e1db09 to
252ea8d
Compare
|
Hi @ejohnstown , |
Problem
WS_BAD_FILE_E(-1019) meant two unrelated things: "I could not read the file"(
ReadFileIntoBuffer()) and "the PEM body was malformed" (DoPemCert()insrc/ssh.c,wolfSSH_ProcessBuffer()insrc/internal.c).So
wolfSSH_ReadCert_buffer()— a public buffer entry point that opens no file— answered -1019 for a PEM whose base64 body will not decode. A caller could
not tell "the bytes you gave me are not a certificate" from "your file would
not read" without knowing which form it handed in.
WS_PARSE_E(-1005) isalready the answer for malformed input everywhere else in this reader,
including the DER arm two functions away.
Inherited style rather than a new mistake:
wolfSSH_ProcessBuffer()has donethis for years and
DoPemCert()matched its neighbour when PR #1140 added it.Fix (
src/ssh.c)"The PEM body would not decode" now answers
WS_PARSE_Efrom every entrypoint, for certificate, CA and private-key buffers alike.
WS_BAD_FILE_Eisleft meaning only file I/O.
DoPemCert(),src/ssh.cWS_BAD_FILE_EWS_PARSE_EwolfSSH_ProcessBuffer()PEM arm,src/internal.cWS_BAD_FILE_EWS_PARSE_EMeasured against the built library, before → after:
wolfssh/ssh.hgained a note naming the split. No caller in the treediscriminates on these codes; the only
== WS_BAD_FILE_Ecomparison is insrc/wolfsftp.cand never reaches these paths.Tests (
tests/api.c)This path had no coverage at all, so all six assertions are new pins rather
than moved ones — added to
test_wolfSSH_ReadCert_buffer,test_wolfSSH_CTX_UseCert_buffer,test_wolfSSH_CTX_AddRootCert_file(theBUFTYPE_CAcase, as no_buffertest exists) andtest_wolfSSH_CTX_UsePrivateKey_buffer_pem. The existing half-PEM case stayspinned at
WS_BAD_FILETYPE_Eso sniff-rejection and decode-rejection remaindistinguishable.
Verification
make check: 11 passed, 0 failed, 1 skipped (network-dependentexternal.test).-Werrorclean across 6 configs, certs-on and certs-off.