fix: known_hosts ワイルドカード行での MITM 誤警告を修正 + 依存の定期更新 - #25
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d498e61517
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| star, starMatch := -1, 0 | ||
| for ai < len(addr) { | ||
| switch { | ||
| case pi < len(pattern) && (pattern[pi] == '?' || pattern[pi] == addr[ai]): |
There was a problem hiding this comment.
OpenSSH matches known-host hostname patterns case-insensitively, but this byte comparison is case-sensitive. For example, *.Example.COM does not match win.example.com, and *.example.com,!WIN.example.com incorrectly applies to win.example.com. Consequently hostKeyAlgorithmsFromKnownHosts can ignore or misapply valid pattern entries, leaving the wildcard false-warning/TOFU behavior unresolved for mixed-case entries; normalize both operands to a common ASCII case before wildcard matching.
Useful? React with 👍 / 👎.
x/crypto は直前の v0.56.0（到達可能な DoS 2 件を解消済み）から、現行リリースの v0.57.0 への定期更新。v0.56.0 に該当する advisory は無く、セキュリティ修正ではない。 x/term も v0.46.0 へ揃え、間接依存の x/sys が v0.48.0 に上がる。 Go 1.27.1（ローカル）と toolchain go1.26.6（CI）の両方で govulncheck が 到達可能な脆弱性 0 件。go ディレクティブは Termux 互換のため 1.26.0 のまま。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hostKeyAlgorithmsFromKnownHosts はホスト行を完全一致でしか照合しておらず、 `192.168.1.*` のようなパターン行は「未登録」と判定されてネゴシエーション制限が nil になっていた。一方 knownhosts コールバックはパターンを解釈するため、サーバが 複数種別のホスト鍵を提供する場合（Windows OpenSSH は rsa/ecdsa/ed25519）に known_hosts に無い種別が選ばれ、何も変わっていないホストに対して REMOTE HOST IDENTIFICATION HAS CHANGED を誤表示していた。 読み取り用の照合を OpenSSH 準拠にする（`*` / `?` / カンマ列 / `!` 否定）。 `@cert-authority` / `@revoked` 行は対象外にする（前者の種別は CA 鍵のもので、 制限リストに混ぜると証明書ホストへの接続を壊す）。 削除経路（replaceHostKeyInKnownHosts）の hostMatchesAddr は完全一致のまま 据え置く。ここでパターンを展開すると、鍵変更を承認したときに 1 行が丸ごと消えて 同じパターンに覆われた他ホストの鍵まで巻き添えになる。 照合が広がる副作用として、パターン行が実サーバと別種別の鍵を指す構成では 制限付きハンドシェイクが落ち、回復が shouldRetryWithoutHostKeyAlgorithms の 再試行頼みになる。実ハンドシェイク由来のエラーで再試行が発火することをテストで 固定した。誤警告の回帰テストとあわせ、いずれも修正前のコードで落ちることを確認済み。 あわせて looksLikeNonWindows のシェル名判定を行頭一致に変更する。部分一致では `ssh: …` や `Publish: …` が "sh:" を含むだけで非 Windows と誤判定され、 graceful degradation ではなく即中断していた。`env:` は PowerShell の `$env:` 名前空間と衝突するため行頭一致の候補からも外す。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d498e61 to
40abddd
Compare
kwrkb
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40abddd0ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var nonWindowsShellPrefixes = []string{ | ||
| "bash:", "sh:", "zsh:", "ksh:", "csh:", "tcsh:", "dash:", "ash:", "fish:", | ||
| "powershell:", | ||
| } |
There was a problem hiding this comment.
When the remote Unix login shell identifies itself by an absolute path, this prefix list no longer recognizes the failure. For example, /bin/sh -c powershell emits /bin/sh: 1: powershell: not found; it contains none of the message signatures and does not start with sh:, so looksLikeNonWindows returns false. resolveKeyFileTarget then treats this as a recoverable admin-check failure and proceeds down the Windows deployment path instead of reporting that the target is unsupported. Preserve the false-positive fix while accepting basename prefixes such as /bin/sh: (or specifically retaining the removed powershell: not found signature).
Useful? React with 👍 / 👎.
Codex レビューの P2 指摘 2 件。 - hostPatternMatch のホストパターン照合を大文字小文字非依存にする。 OpenSSH の match_hostname() はホスト名とパターンの両方を lowercase して から照合するため、バイト比較のままでは `*.Example.COM` が win.example.com に当たらず、`!WIN.example.com` が win.example.com を 除外できなかった。どちらも本 PR が直そうとしているワイルドカード誤警告を そのまま残す。畳むのは hostPatternMatch の内部だけに閉じる。 knownHostsLineMatchesAddr の addr は matchHashedHost にも渡っており、 HMAC はバイト厳密なので呼び出し側で正規化すると混合ケースの ハッシュ化エントリが引けなくなる。 削除経路の hostMatchesAddr も完全一致のまま据え置く。あの関数が走るのは x/crypto/ssh/knownhosts のコールバックが「鍵が変わった」と判定した後だけで、 同パッケージには大小文字を畳む処理が無い（Normalize も lowercase しない）。 ここだけ広げるとコールバックが一致させていない行まで削除対象になる。 - looksLikeNonWindows が絶対パスで名乗るシェルを取りこぼしていた。 dash の `/bin/sh: 1: powershell: not found` は message 断片のどれにも 一致せず、行頭も `sh:` ではないため false を返し、resolveKeyFileTarget が 「回復可能な admin 判定失敗」とみなして Windows 配置経路へ進んでいた。 行頭が `/` の場合にかぎり最初の `:` までを basename に落として照合する。 `/` 始まりに限定することで、"ssh: handshake failed" のような行を 巻き込まない既存の誤検知耐性は保つ。 いずれも修正前のコードで新規テストが落ちることを確認済み。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDRW423QvvZ1t3ESWPezBn
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDRW423QvvZ1t3ESWPezBn
1 participant
定期メンテナンスと、その過程で見つかったバグ 2 件の修正。
1. 依存更新（定期）
golang.org/x/cryptov0.56.0 → v0.57.0、golang.org/x/termv0.45.0 → v0.46.0（間接依存のx/sysが v0.48.0 へ）GO-2026-6354/GO-2026-6355は master の 5e71f7d（v0.56.0）で既に解消済みで、v0.56.0 に該当する advisory は無いgoディレクティブは Termux 互換のため 1.26.0 のまま、toolchain go1.26.6も据え置きgovulncheckが到達可能な脆弱性 0 件2. known_hosts のワイルドカード行で MITM 誤警告（実バグ）
hostKeyAlgorithmsFromKnownHostsがホスト行を完全一致でしか照合しておらず、192.168.1.*や*.example.comのようなパターン行は「未登録」扱いになってネゴシエーション制限が nil になっていた。一方 knownhosts コールバックはパターンを解釈するため、複数種別のホスト鍵を提供するサーバ（Windows OpenSSH は rsa / ecdsa / ed25519）に対して known_hosts に無い種別が選ばれ、何も変わっていないホストにREMOTE HOST IDENTIFICATION HAS CHANGED!が出ていた。*/?/ カンマ区切り /!否定）@cert-authority/@revoked行はスキップする。前者が持つのは CA 鍵の種別でホスト鍵の種別ではなく、制限リストに混ぜると証明書ベースのホストへの接続を壊すreplaceHostKeyInKnownHosts）のhostMatchesAddrは完全一致のまま据え置き。 ここでパターンを展開すると、鍵変更を承認したときにパターン行が丸ごと消え、同じパターンに覆われた他ホストの鍵まで巻き添えになるshouldRetryWithoutHostKeyAlgorithmsの再試行頼みになる。この判定は sentinel error が無く文字列一致なので、実ハンドシェイク由来のエラーで再試行が発火することをテストで固定した回帰テストは 2 本とも、修正前のコードで実際に落ちることを確認済み（誤警告のほうは
REMOTE HOST IDENTIFICATION HAS CHANGEDを出して失敗する）。3.
looksLikeNonWindowsの誤検知Unix シェル名を出力全体への部分一致で探していたため、
ssh: handshake failed…やPublish: …が"sh:"を含むだけで「Windows ではない」と判定されていた。判定が真のときは graceful degradation ではなく即中断するため、誤検知のコストが大きい。markerValue/hasMarkerLineで既に採っている方針に合わせた）。加えて行頭が/の場合にかぎり絶対パスの basename でも照合する（下記 4. のレビュー対応で追加）command not foundのような十分に特徴的なメッセージ断片は部分一致のまま残したenv:は行頭一致の候補からも外した。PowerShell の$env:VAR名前空間と衝突し、このツール自身が実行するWrite-Output $env:SSH_CONNECTIONの出力を非 Windows と誤判定し得る検証
make check（build + vet + test + integration ビルド）go test -race ./...govulncheck ./...（Go 1.27.1 / go1.26.6 両方）goreleaser checkmake itest-admin/make itest-user統合テストは実機 Windows ホストと実ターミナルが必要なため未実行。今回の 2 件はどちらも SSH クライアント側のロジックで、Go のテストサーバで実コードパスを exercise できている。ただし
looksLikeNonWindowsはリモート実行経路にあるため、実際の Windows 出力が新しい行頭リストに引っかからないことは実機実行でしか確認できない。マージ前にmake itest-allを回すことを推奨。なお
.github/workflows/{ci,release}.ymlのアクション版（checkout@v7 / setup-go@v7 / goreleaser-action@v7 / load-secrets-action@v5）と.goreleaser.yamlも点検したが、更新の必要はなかった。4. レビュー対応（19b813d）
Codex レビューの P2 指摘 2 件に対応。いずれも修正前のコードで新規テストが落ちることを確認済み。
hostPatternMatchのホストパターン照合を大文字小文字非依存にした。 OpenSSH のmatch_hostname()はホスト名とパターンの両方を lowercase してから照合するため、バイト比較のままでは*.Example.COMがwin.example.comに当たらず、!WIN.example.comがwin.example.comを除外できなかった。どちらも 2. が直そうとしているワイルドカード誤警告をそのまま残すhostPatternMatchの内部だけ。knownHostsLineMatchesAddrのaddrはmatchHashedHostにも渡っており、HMAC はバイト厳密なので呼び出し側で正規化すると混合ケースのハッシュ化エントリが引けなくなるhostMatchesAddrは完全一致のまま据え置き（意図的な非対称）。 あの関数が走るのはx/crypto/ssh/knownhostsのコールバックが「鍵が変わった」と判定した後だけで、同パッケージには大小文字を畳む処理が一切無い（Normalize()も lowercase しない）。ここだけ広げると、コールバックが一致させていない行まで削除対象になるlooksLikeNonWindowsが絶対パスで名乗るシェルを取りこぼしていた。 dash の/bin/sh: 1: powershell: not foundは message 断片のどれにも一致せず（not found単体は断片に無い）、行頭もsh:ではないためfalseを返し、resolveKeyFileTargetが「回復可能な admin 判定失敗」とみなして Windows 配置経路へ進んでいた/の場合にかぎり最初の:までを basename に落として照合する。/始まりに限定することで、ssh: handshake failedのような行を巻き込まない 3. の誤検知耐性は保つpowershell: not found断片の復活ではなく basename 一致を採ったのは、/bin/dash//usr/bin/bashなど他のシェルにも同じ形で効くためmake check/go test -race ./...は 19b813d でも再実行してパス。統合テストは引き続き未実行（上表のとおり）。🤖 Generated with Claude Code