Replace the matching certificate slot instead of appending a duplicate

[yosuke-wolfssl]

Problem

SetHostCertificate() never replaced an existing certificate. Its search loop
records a matching slot in certIdx, but destIdx is the loop counter and the
loop has no break, so on exit it always equals ctx->privateKeyCount — the
append slot. pvtKey is taken from there, whose publicKeyFmt is always
ID_NONE, making the pvtKey->publicKeyFmt == certId replace branch dead code;
the else branch then discards the search result with certIdx = destIdx;.

Reloading a certificate for an algorithm that already has one therefore appends
a duplicate slot instead of replacing it. KEX selects the signing slot with a
first-match-and-break scan, so the stale certificate at the lower index keeps
being served — a renewed certificate silently has no effect until restart. The
call still returns WS_SUCCESS. Additional consequences: the old DER is never
freed, the same x509v3-* name is emitted twice in
server_host_key_algorithms, and each reload burns a slot until
WOLFSSH_MAX_PVT_KEYS is exhausted.

Sibling SetHostPrivateKey() gets this right — its while loop puts the match
test in the loop condition, so it stops on the matching slot.

Fix (src/internal.c)

Select the destination slot from the search result before using it:

/* Replace the matching slot if the search found one, else append. */
destIdx = HINTISSET(certIdx) ? certIdx : ctx->privateKeyCount;

HINTISSET() is the file's existing sentinel idiom, and certIdx is only ever
assigned an in-range index, so the replace branch becomes reachable and frees
the old DER in place. The append path is unchanged.

Also frees der on the destIdx >= WOLFSSH_MAX_PVT_KEYS path — ownership
transfers from wolfSSH_ProcessBuffer(), which does not free on error. That
matches what SetHostPrivateKey() already does, and the fix makes the path
genuinely reachable.

Closes F-8810.

Tests (tests/api.c)

Extended test_wolfSSH_CTX_UseCert_buffer(), which already loaded the same
certificate as PEM then DER but never checked the bookkeeping. Now asserts
privateKeyCount is unchanged, the slot holds a different pointer, and
publicKeyAlgoCount == 1. Added a table-full case asserting
WS_CTX_KEY_COUNT_E to cover the free-on-error path.

Verification

• make check: 11 passed, 1 skipped, 0 failed (from a clean rebuild).
• Negative control: reverting the one-line fix fails the new assertion with
  1 != 2; injecting a second WFREE on the error path trips ASan at
  SetHostCertificate internal.c:2386, pinning the test to that branch.
• Clean under gcc-13 -Werror across 6 configs, including one with X.509 certs
  disabled.

[Copilot]

Pull request overview

Fixes SetHostCertificate() so reloading a host certificate replaces an existing matching certificate slot (instead of always appending a duplicate), and ensures DER is freed on the “table full” error path. This prevents stale certificates from continuing to be served after reload and avoids slot exhaustion/leaks.

Changes:

• src/internal.c: Choose the destination slot based on the search result (replace vs append) and free der when WOLFSSH_MAX_PVT_KEYS is exceeded.
• tests/api.c: Extend test_wolfSSH_CTX_UseCert_buffer() to assert replacement behavior and cover the table-full error path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/internal.c	Fixes certificate slot selection to enable in-place replacement and frees DER on overflow error path.
tests/api.c	Adds assertions ensuring reload replaces instead of appends, plus a regression check for the overflow/free-on-error path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1152

Scan targets checked: wolfssh-bugs, wolfssh-src

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

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1152

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.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1152

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.