Repository navigation
fix: harden Telnet login stream boundaries and trace redaction - #10
Merged
Merged
Conversation
Scan available buffered chunks instead of checking every byte prefix. Defer narrative failures and expiry messages until line completion, preserve post-prompt data, bound partial matching, and add stream regressions and fuzz coverage.
Protect incoming and generated subnegotiation parameters as well as wire/application data. Keep redaction active during stream-wrapper setup and failed-login cleanup.
Continue through the real IAC SE across reads, honor escaped IAC, preserve following application data and valid events, and cover split points with regression and fuzz tests.
There was a problem hiding this comment.
🟡 Changes recommended
Authentication failures can be misclassified as successful logins when line endings arrive in a later read, while valid long custom prompts and structured bracket prompts can be rejected.
3 open issues
What changed in this PR
This PR hardens Telnet authentication and stream handling without changing the public API.
Changes:
- Matches login messages at logical-line boundaries, bounds partial prompts, and preserves buffered session data.
- Centralizes login trace redaction, including subnegotiation payloads and failed-login cleanup.
- Preserves framing after oversized subnegotiations, with regression tests, fuzz targets, and a benchmark.
| File | Description |
|---|---|
transport.go |
Delegates payload redaction to the tracer. |
trace.go |
Centralizes byte and subnegotiation payload redaction. |
trace_secrecy_test.go |
Tests login secrecy and cleanup-write redaction. |
login.go |
Adds boundary-aware scanning, banner filtering, and timeout-race handling. |
login_text.go |
Extracts streaming normalization and ignores overlong lines. |
login_stream_test.go |
Tests chunk boundaries and session-data preservation; adds fuzzing and benchmarking. |
docs/login-stream.md |
Documents stream behavior, limitations, and diagnostics. |
decoder.go |
Discards oversized subnegotiations through their actual terminator. |
decoder_recovery_test.go |
Tests and fuzzes framing recovery. |
client.go |
Enables redaction before wrapping and replays consumed prompt terminators. |
Review effort: Balanced
| } | ||
| } | ||
|
|
||
| result, err := m.handlePrompt(writer, text.current(), trace) |
Comment on lines
+190
to
+191
| if len(line) > maxPartialPromptBytes || !utf8.Valid(line) { | ||
| return loginPending, nil |
Comment on lines
+264
to
+265
| if isBannerLikeShellPrompt(string(line)) { | ||
| return false |
Owner
Author
|
已逐条复现本 PR 的三条 Copilot 评论,并基于合并后的 master
新增用例先在未修复实现上确认失败,修复后通过,并覆盖分开的 CR/LF、所有拆分点、EOF/超时、自定义规则重叠及长提示。#11 尚未合并,本 PR 的线程暂不标记为已解决。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


背景
基于最新
master的5f31db2d5c7351b1f8a5424128b8d638d02415c7(已合并 #9)。修复审查中发现的流式前缀误判、登录 Trace 子协商参数泄露,以及超长子协商破坏 framing 的问题。不新增公开 API、配置字段或通用 Expect 接口;不修改 Koko。
1. 批量扫描,并区分完整消息与未换行提示
bufio.Reader。用Peek(1)在缓冲区为空时触发填充,再扫描Peek(Buffered()),只Discard已消费的字节。ReadByte后对每个字符前缀执行全部正则;不等待凑满 4096 字节,也不引入固定 sleep。:/:结束。Authentication failed count: 0、Password expired policy: disabled、New password policy: 90 days等文本在前缀处提前失败。登录成功 >、密码错误>、系统警告 >和[Warning]#被默认规则当成 shell。不丢失 prompt 后的数据
只丢弃已消费的 chunk 前缀,后续数据仍留在同一个 reader。为了确认完整行而消费的 CR/LF(最多两字节)会在会话读取前补回。例如
router#\r\nALARM登录后仍可读取\r\nALARM,已匹配 prompt 不重复回放。说明:无换行文本提示仍然有固有歧义,缓冲区大小或固定延时不能提供协议级认证保证。异常设备应使用精确的平台提示配置;本 PR 不声称识别任意文本协议。
2. 在 tracer 层统一执行登录脱敏
SB参数统一脱敏。NEW_ENVIRON USER不再绕过字节日志脱敏,通过packet/event日志输出用户名。WrapStream前启用脱敏;失败后的 writerClose/flush 仍保持脱敏。3. 超长 SB 丢弃到真实结束符
ErrSubnegotiationTooLong,进入 discard 状态,而不是直接恢复普通数据状态。IAC SE;IAC IAC是转义负载,不会错误结束子协商。提交拆分
fix: match login events at stream boundariesfix: redact all login trace payloads centrallyfix: discard oversized subnegotiations without losing framing验证
新增端到端/表驱动测试覆盖:1/2/7/1024/4096 字节分块、每个拆分点、失败/过期统计 Banner、中文 Banner、无换行交互、Continue、EOF/超时、UTF-8/ANSI、长行、prompt 后数据、环境变量子协商日志及失败清理写入。
新增两个 fuzz target:
新增
BenchmarkLoginLongBanner,覆盖 1 KiB、4 KiB、64 KiB Banner。新增回归用例已在本地 Go 1.23.2 上通过 race 重复执行、vet 和两组 5 秒 fuzz。全量原有与新增用例由本 PR 的 Go 1.20.x / stable CI 验证,包含示例客户端及 monitor 编译。
设计边界与行为说明见
docs/login-stream.md。