Skip to content

Add certificate loading APIs with content-based format detection - #1140

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:feat/certRebase
Open

Add certificate loading APIs with content-based format detection#1140
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:feat/certRebase

Conversation

@yosuke-wolfssl

@yosuke-wolfssl yosuke-wolfssl commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

wolfSSH has a single entry point for loading key files, wolfSSH_ReadKey_file(), which sniffs the format from the file's content. Certificates had no equivalent: wolfSSH_CTX_UseCert_buffer() and wolfSSH_CTX_AddRootCert_buffer() are buffer-only and require the caller to already know PEM from DER. As a result five call sites across apps/ and examples/ hand-roll file reading plus a blind try-PEM-then-retry-ASN1 dance, and apps/wolfsshd/wolfsshd.c carries a TODO asking for exactly this helper.

Fix (src/ssh.c)

New public API; the form is detected from the content, so there is no format argument.

int wolfSSH_ReadCert_buffer(const byte* in, word32 inSz, byte** out,
        word32* outSz, const byte** outType, word32* outTypeSz,
        byte* flavor, void* heap);
int wolfSSH_ReadCert_file(const char* name, /* same out-params */ ...);
int wolfSSH_CTX_UseCert_file(WOLFSSH_CTX* ctx, const char* name);
int wolfSSH_CTX_AddRootCert_file(WOLFSSH_CTX* ctx, const char* name);
Input form Result
PEM CERTIFICATE (header may follow an openssl text dump) DER, flavor = WOLFSSH_CERT_FLAVOR_X509
DER X.509 DER, flavor = WOLFSSH_CERT_FLAVOR_X509
*-cert-v01@openssh.com line blob, flavor = WOLFSSH_CERT_FLAVOR_OSSH

outType is the wire algorithm name derived from the certificate rather than a compile-time flag. An RFC 6187 x509v3-* line is a public key carrying a chain, not a certificate, so it is declined here — wolfSSH_ReadKey_buffer() with WOLFSSH_FORMAT_SSH already reads that form.

Certificate identification is now consistent (src/internal.c). The old IdentifyCert() names the key inside a certificate, so it is renamed IdentifyCertKey(), and a new IdentifyCert() returns the x509v3-* wire algorithm, rejecting a key type that has no such name with WS_INVALID_ALGO_ID. SetHostCertificate() now takes certId, which lets wolfSSH_ProcessBuffer() apply the same rule. Previously wolfSSH_CTX_UseCert_buffer() accepted such a certificate, stored it under a plain key id, and advertised an unusable host-key algorithm.

This is the library half; converting the five existing call sites is a follow-up PR.

Tests (tests/api.c)

test_wolfSSH_ReadCert_buffer, _file, test_wolfSSH_CTX_UseCert_file, _AddRootCert_file, and an OpenSSH round-trip. Negative cases cover a truncated PEM, a bare DER header, a DER and a PEM private key, an SSH public key line, an x509v3-* line, and a certificate with no x509v3 name — for which keys/server-cert-ed25519.{pem,der} is added, generated by renewcerts.sh.

Verification

  • make check passes with --enable-certs, --enable-ossh-certs, both, and neither.
  • Clean under gcc-13 -Werror across the CI configurations, including small-stack and Zephyr defines.
  • ASan + UBSan clean.
  • Certificate/private-key pairing in the CTX is unchanged for both load orders; in a build without an x509v3-* name for the key type, the read and CTX paths now agree on rejecting the certificate.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 4, 2026
Copilot AI lite review requested due to automatic review settings August 4, 2026 05:33

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

Adds certificate-loading APIs that mirror wolfSSH_ReadKey_file() by detecting certificate format from content, while also tightening certificate algorithm identification and fixing an SSH public key parsing edge case.

Changes:

  • Introduces wolfSSH_ReadCert_{buffer,file}() and wolfSSH_CTX_{UseCert,AddRootCert}_file() (content-sniffed PEM/DER and OpenSSH cert-line support where enabled).
  • Refactors internal certificate identification to return the wire x509v3-* algorithm (and reject unmappable certs).
  • Fixes DoSshPubKey() handling for SSH public key lines that do not end with a trailing newline; adds API test coverage for new cert APIs and the newline edge case.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
wolfssh/ssh.h Declares new public certificate read/file APIs and WS_CertFlavors.
wolfssh/internal.h Exposes IdentifyCert() for internal callers under WOLFSSH_CERTS.
src/ssh.c Implements new cert read APIs, adds cert form sniffing, refactors file reading helper, fixes DoSshPubKey() termination.
src/internal.c Splits “identify key inside cert” vs “identify cert wire algo”; adjusts host certificate slot handling logic.
tests/api.c Adds coverage for new cert APIs and validates no-trailing-newline SSH public key parsing.
keys/server-key-ed25519-cert.pem Adds Ed25519 private key fixture used to generate the unmappable X.509 cert.
keys/server-cert-ed25519.pem Adds Ed25519 X.509 cert fixture used to exercise unmappable x509v3-* rejection.
keys/renewcerts.sh Extends renewal script to generate the new Ed25519 cert/key fixtures.
keys/include.am Distributes new key/cert fixtures via Automake EXTRA_DIST.
Suppressed comments (1)

src/internal.c:2364

  • SetHostCertificate() always uses destIdx after the scan loop, but destIdx is guaranteed to equal ctx->privateKeyCount at that point. That means an existing certificate slot (certIdx) is never reused/replaced; instead a new entry is appended each time, leading to duplicate cert slots, leaked old certs, and eventual WS_CTX_KEY_COUNT_E once the key table fills.

Use certIdx when it was found (replace-in-place), and only use privateKeyCount as the insertion index when no existing cert slot exists.

    }

    if (destIdx >= WOLFSSH_MAX_PVT_KEYS) {
        ret = WS_CTX_KEY_COUNT_E;
    }

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

@yosuke-wolfssl
yosuke-wolfssl force-pushed the feat/certRebase branch 2 times, most recently from a3593b5 to 4e61b9a Compare August 4, 2026 05:51

@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 #1140

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 tests/api.c
Comment thread tests/api.c

@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 #1140

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 src/ssh.c
Comment thread src/ssh.c

@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 #1140

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 tests/api.c Outdated
Comment thread src/ssh.c
Comment thread tests/api.c Outdated
Comment thread src/ssh.c

@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 #1140

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

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