Skip to content

Hold algorithm-list names to RFC 4251 format - #1321

Open
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:algo-list-setters
Open

ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:algo-list-setters

Conversation

@ejohnstown

@ejohnstown ejohnstown commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

The SetAlgoList* setters skip unknown names but keep the caller's string, and KEXINIT sends it verbatim. Every name in the list must now be well formed (RFC 4251 section 6, RFC 4250 section 4.6), or the setter returns WS_INVALID_ALGO_ID.

  • A name is 1 to 64 bytes of printable US-ASCII: no space, control code, DEL or high byte, and at most one @, not first or last
  • tests/api.c covers the 64/65-byte bound, each banned byte class, and the @ count and position

Issue: F-15529

Copilot AI balanced review requested due to automatic review settings October 16, 2026 17:14

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 2 of 4 in-scope changed file(s) opened by the reviewer; not opened: wolfssh/internal.h, wolfssh/ssh.h

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

CheckAlgoList() skips unknown names, but the setters keep the caller's
string and SendKexInit() sends it verbatim. Every name in the list now
has to be at most 64 bytes of printable US-ASCII with no space and at
most one '@', not first or last, per RFC 4251 Section 6 and RFC 4250
Section 4.6, or the setter returns WS_INVALID_ALGO_ID.

- internal.h: add WOLFSSH_MAX_ALGO_NAME_SZ for the 64-byte cap
- api.c: cover the 64/65-byte bound, space, tab, DEL, high bytes, the
  at-sign count and an at-sign at either end
- ssh.h: state the name format in the setter contract

Issue: F-15529

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

Scan targets checked: none
Unchanged since last review (not re-run): wolfssh-src, wolfssh-bugs

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

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