Skip to content

Adopt the certificate loading APIs in the examples and apps - #1150

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/examples
Open

Adopt the certificate loading APIs in the examples and apps#1150
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/examples

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

Each example and app hand-rolled its own certificate file loading: apps/wolfssh/common.c and examples/client/common.c had a load_der_file(), examples/tpmcertserver had TpmCcLoadFile(), echoserver used load_file(). All were DER-only and none detected the file's format. The client's public-key algorithm name was also a compile-time constant:

static const byte publicKeyType[] = "x509v3-ecdsa-sha2-nistp256";

so using an RSA certificate meant flipping an #if 0 block and rebuilding.

Fix

wolfSSH_ReadCert_file() and wolfSSH_CTX_AddRootCert_file() (already on master) replace the local readers. PEM or DER is detected from the file's content, and the algorithm name sent on the wire comes from the certificate rather than a build flag.

File Change
apps/wolfssh/common.c load_der_file()wolfSSH_ReadCert_file(); drops hardcoded publicKeyType
examples/client/common.c same, plus ClientLoadCA()wolfSSH_CTX_AddRootCert_file()
examples/echoserver/echoserver.c root CA via _file; -J PEM list via wolfSSH_ReadCert_buffer()
examples/tpmcertserver/tpmcertclient.c drops TpmCcLoadFile()
apps/wolfsshd/wolfsshd.c keeps the _buffer API — see below

wolfsshd deliberately keeps _buffer. Its files are read through the getBufferFromFile() trust gate, which the _file helpers would bypass. It gains explicit handling instead: an empty HostCertificate or TrustedUserCAKeys file is rejected rather than silently retried as another format, and an OpenSSH-flavored host certificate returns WS_UNIMPLEMENTED_E instead of being reported as loaded.

Build matrix. The cert _file call sites in the client and echoserver now carry the same !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM guard their declarations have in ssh.h; two adjacent wolfSSH_ReadKey_file() guards were widened to match. echoserver's -J list now requires WOLFSSH_CERTS — it is documented as loading an X.509 PEM cert, which is unusable without it.

Net 240 lines removed, 151 added.

Verification

  • Clean under -Werror across 6 gcc-13 configs (enable-all, sftp-only, scp-only, zephyr, small-stack, default).
  • make check: 11 passed, 1 skipped (external), 0 failed.
  • Guard probes with negative controls: WOLFSSH_CERTS with NO_FILESYSTEM, and with WOLFSSH_USER_FILESYSTEM, both fail before the guards and are clean after.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 6, 2026
Copilot AI lite review requested due to automatic review settings August 6, 2026 23:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR standardizes certificate loading across examples/apps by switching from local, DER-only file readers to wolfSSH’s certificate-loading APIs that auto-detect PEM vs DER and extract the correct on-wire algorithm from the certificate.

Changes:

  • Replace ad-hoc certificate file loaders with wolfSSH_ReadCert_* and wolfSSH_CTX_AddRootCert_* helpers.
  • Update echoserver/client build guards to match the _file API availability constraints.
  • Keep wolfsshd on _buffer loading via the trust-gated file reader and add explicit format/empty-file handling.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
examples/tpmcertserver/tpmcertclient.c Drops custom file loader and uses wolfSSH_CTX_AddRootCert_file() directly.
examples/echoserver/echoserver.c Uses cert parsing helpers for -J PEM list and loads CA via _file with tighter build guards.
examples/client/common.c Uses wolfSSH_ReadCert_file() and wolfSSH_CTX_AddRootCert_file(); removes hardcoded algorithm selection.
apps/wolfsshd/wolfsshd.c Adds explicit cert decoding/validation while preserving trust-gated buffer loading.
apps/wolfssh/common.c Replaces binary loader with a NUL-terminating text loader and adopts wolfSSH_ReadCert_file() / _AddRootCert_file().
Suppressed comments (1)

apps/wolfsshd/wolfsshd.c:567

  • The CA-keys path calls wolfSSH_ReadCert_buffer() and uses WOLFSSH_CERT_FLAVOR_* unconditionally inside a #if defined(WOLFSSH_OSSH_CERTS) || defined(WOLFSSH_CERTS) region. If a build enables WOLFSSH_OSSH_CERTS but not WOLFSSH_CERTS, this will fail to compile (missing symbols/types). Guard the X.509 parsing branch with #ifdef WOLFSSH_CERTS (similar to the hostCert branch above), and in the !WOLFSSH_CERTS case either treat the file as OpenSSH-style CA keys (when WOLFSSH_OSSH_CERTS) or emit a clear error.
            byte* der = NULL;
            word32 derSz = 0;
            const byte* type = NULL;
            word32 typeSz = 0;
            byte flavor = WOLFSSH_CERT_FLAVOR_UNKNOWN;

            wolfSSH_Log(WS_LOG_INFO, "[SSHD] Using CA keys file %s", caCert);
            data = getBufferFromFile(caCert, &dataSz, heap,
                WOLFSSHD_LOAD_TRUST);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread examples/tpmcertserver/tpmcertclient.c
Comment thread apps/wolfssh/common.c
Comment thread apps/wolfssh/common.c
Comment thread apps/wolfssh/common.c
Comment thread apps/wolfssh/common.c
Comment thread examples/client/common.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1150

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/wolfsshd.c Outdated
@yosuke-wolfssl
yosuke-wolfssl marked this pull request as ready for review August 7, 2026 02:46

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1150

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.

Comment thread examples/client/common.c
@yosuke-wolfssl

Copy link
Copy Markdown
Contributor Author

Hi @ejohnstown ,
This is one of the follow-ups.
Could you please review it ?

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.

4 participants