From 40abddd0ec6f99b67cb3a363677bb99a4f9f0d99 Mon Sep 17 00:00:00 2001 From: yugosasaki Date: Fri, 18 Sep 2026 01:34:25 +0900 Subject: [PATCH 1/3] =?UTF-8?q?fix(hostkey):=20known=5Fhosts=20=E3=81=AE?= =?UTF-8?q?=E3=83=AF=E3=82=A4=E3=83=AB=E3=83=89=E3=82=AB=E3=83=BC=E3=83=89?= =?UTF-8?q?=E8=A1=8C=E3=81=A7=20MITM=20=E8=AA=A4=E8=AD=A6=E5=91=8A?= =?UTF-8?q?=E3=81=8C=E5=87=BA=E3=82=8B=E3=81=AE=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CHANGELOG.md | 5 ++ LESSONS.md | 41 ++++++++++++ deploy.go | 43 +++++++++---- deploy_test.go | 5 ++ hostkey_verification_test.go | 121 ++++++++++++++++++++++++++++++++++- ssh.go | 89 +++++++++++++++++++++++--- ssh_test.go | 120 ++++++++++++++++++++++++++++++++++ 7 files changed, 403 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff45be1..4b83b9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- Host key verification no longer prints a false `REMOTE HOST IDENTIFICATION HAS CHANGED!` warning when `~/.ssh/known_hosts` matches the target through a wildcard pattern (`192.168.1.*`, `*.example.com`, …). The host key algorithms offered during negotiation are taken from `known_hosts`, but that lookup compared host fields for exact equality only, so a pattern entry was treated as "host unknown" and no restriction was applied. Against a server offering several host key types — which Windows OpenSSH does (rsa, ecdsa, ed25519) — the client could then negotiate a type absent from `known_hosts`, and the verification callback, which *does* expand patterns, reported it as a changed host key. Host matching for this lookup now follows OpenSSH: `*` and `?` wildcards, comma-separated lists, and `!` negation. `@cert-authority` and `@revoked` lines are skipped, because the algorithm on a `@cert-authority` line belongs to the CA key rather than the host key. Removal of stale entries deliberately keeps matching on exact equality, so accepting a changed host key never deletes a pattern line covering other hosts. +- The "remote host is not Windows" check no longer aborts on unrelated output. Unix shell prefixes (`bash:`, `sh:`, …) were matched as substrings anywhere in the remote output, so text such as `ssh: …` or `Publish: …` — both of which contain `sh:` — made the tool exit with "remote host does not appear to be Windows". Those prefixes are now anchored to the start of a line, matching the convention already used for the result markers. + ### Changed - Updated dependencies: `golang.org/x/crypto` v0.55.0 → v0.56.0. Versions up to v0.55.0 are affected by GO-2026-6354 and GO-2026-6355, two denial-of-service issues in `golang.org/x/crypto/ssh` channel handling that `govulncheck` reports as reachable from this tool's `ssh.Dial` call. After the update `govulncheck ./...` reports no reachable vulnerabilities. diff --git a/LESSONS.md b/LESSONS.md index 63af3de..562dc59 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -1,5 +1,46 @@ # LESSONS +## known_hosts のワイルドカード照合と、読み取り経路 / 削除経路の非対称性 (2026-09-18) + +### ホストパターンの展開は「読み取り専用の照合」にだけ入れる +- `hostKeyAlgorithmsFromKnownHosts`（ネゴシエーション制限用の読み取り）と + `replaceHostKeyInKnownHosts`（鍵変更を承認したときの行削除）が同じ照合関数を共有していた。 + 照合が完全一致だったためパターン行が「未登録」扱いになり、制限なしでネゴシエートした結果、 + known_hosts に無い種別の鍵が選ばれて正常なホストに MITM 警告が出ていた +- **却下した案**: 共有している照合関数自体をワイルドカード対応にする — 1 行で直るが、削除経路が + 同じ関数を使っているため `192.168.1.*` のような 1 行が「マッチした」と判定されて丸ごと消え、 + 同じパターンに覆われていた他ホストの鍵まで巻き添えになる。読み取りの誤警告を直すために + 破壊的な削除を招き入れる交換になる +- **決め手**: 照合を用途で 2 つに割った。読み取りは OpenSSH 準拠（`*` / `?` / カンマ列 / `!` 否定）、 + 削除は完全一致のまま据え置き。修正前のコードで回帰テストが + `REMOTE HOST IDENTIFICATION HAS CHANGED` を出して落ち、修正後に通ることを確認した + （サーバに rsa と ed25519 を両方持たせ、known_hosts にはワイルドカードで ed25519 だけを登録する + 構成で再現。単一鍵のテストサーバでは種別のズレが起きないため再現しない） +- `@cert-authority` 行が持つのは CA 鍵の種別でホスト鍵の種別ではない。パターン対応を入れると + この行が拾われて制限リストに混ざり、証明書ベースのホストへの接続を壊す。マーカー行は明示的に + スキップし、該当ホストが他に無ければ nil（＝制限なし）へ落とす +- **副作用の確認**: 照合を広げると、これまで「未登録＝制限なし」で通っていた構成に制限が付く。 + パターン行が実サーバと別種別の鍵を指していると `no common algorithm for host key` で + ハンドシェイクが落ちるため、回復は `shouldRetryWithoutHostKeyAlgorithms` の再試行頼みになる。 + この判定は sentinel error が無く文字列一致なので、実ハンドシェイク由来のエラーで再試行が + 発火することをテストで固定した（判定を潰すと実際に落ちることも確認）。予測文字列だけの + ユニットテストでは、依存を上げたときに文言が変わっても気付けない +- **覆す条件**: 削除経路をパターン対応にする必要が出たら、行を消すのではなく「マッチしたホスト名 + だけをホストフィールドから除く」形にできるか先に検討する（パターン行では表現できないので、 + OpenSSH 自身も `ssh-keygen -R` でパターン行を消さない） + +### 部分一致でシェル名を探すと無関係な出力に当たる +- 非 Windows ホスト判定が `strings.Contains(output, "sh:")` を使っており、`ssh: …` や `Publish: …` + のような文字列でも真になった。判定が真のときは graceful degradation ではなく即中断するため、 + 誤検知のコストが大きい +- **決め手**: シェル名は**行頭一致**に限定し、`command not found` のような十分に特徴的な + メッセージ断片だけを部分一致で残した。マーカー解析（`markerValue` / `hasMarkerLine`）で + 既に同じ結論に達していたので、そのルールに揃えただけとも言える +- 行頭一致へ移す際に `env:` を候補に入れかけたが外した。PowerShell の `$env:VAR` 名前空間と + 衝突し、このツール自身が実行する `Write-Output $env:SSH_CONNECTION` の出力を非 Windows と + 誤判定し得る。緩い matcher を 1 つ外して別の緩い matcher を足しては意味がない +- **覆す条件**: なし（部分一致に戻す理由が見当たらない） + ## Scoop の manifest は bucket リポジトリ直下ではなく `bucket/` に置く (2026-08-16) ### 直下レイアウトは scoop から「見えていない」 diff --git a/deploy.go b/deploy.go index 390d111..07d726d 100644 --- a/deploy.go +++ b/deploy.go @@ -186,24 +186,45 @@ func effectiveAdminKeysFromSshdT(output string) (isAdmin bool, ok bool) { return false, false } +// nonWindowsMessageSignatures は「非 Windows ホスト」を示すメッセージ断片。 +// いずれも十分に特徴的なので出力のどこに出ても判定に使える。 +var nonWindowsMessageSignatures = []string{ + "command not found", + "not recognized as", + "unknown command", + "no such file or directory", + "not supported on this platform", + "platformnotsupported", +} + +// nonWindowsShellPrefixes は Unix シェルがコマンド不在を報告するときの行頭。 +// これらは**行頭一致**でしか見ない。部分一致にすると "ssh:" や "…finish:" のような +// 無関係な文字列が "sh:" を含むだけで非 Windows と誤判定され、graceful degradation では +// なく「Windows ではない」と即座に中断してしまう（markerValue / hasMarkerLine と同じ方針）。 +// `env:` は入れない。PowerShell 自身の `$env:VAR` 名前空間と衝突し、 +// `Write-Output $env:SSH_CONNECTION` を含むこのツールの出力を非 Windows と誤判定し得る。 +// `env: 'powershell': No such file or directory` は message 側の断片で拾える。 +var nonWindowsShellPrefixes = []string{ + "bash:", "sh:", "zsh:", "ksh:", "csh:", "tcsh:", "dash:", "ash:", "fish:", + "powershell:", +} + // looksLikeNonWindows は PowerShell 実行エラー出力が Linux/非 Windows ホストを示すか判定する。 func looksLikeNonWindows(output string) bool { lower := strings.ToLower(output) - for _, sig := range []string{ - "command not found", - "not recognized as", - "powershell: not found", - "bash:", - "sh:", - "unknown command", - "no such file or directory", - "not supported on this platform", - "platformnotsupported", - } { + for _, sig := range nonWindowsMessageSignatures { if strings.Contains(lower, sig) { return true } } + for _, line := range strings.Split(lower, "\n") { + trimmed := strings.TrimSpace(line) + for _, prefix := range nonWindowsShellPrefixes { + if strings.HasPrefix(trimmed, prefix) { + return true + } + } + } return false } diff --git a/deploy_test.go b/deploy_test.go index 703357e..34988ba 100644 --- a/deploy_test.go +++ b/deploy_test.go @@ -494,6 +494,11 @@ func TestLooksLikeNonWindows(t *testing.T) { {"normal windows output — True", "True", false}, {"empty output", "", false}, {"windows error message", "The system cannot find the file specified.", false}, + // シェル名の判定は行頭一致。"ssh:" や "…finish:" は "sh:" を含むが非 Windows ではない。 + {"ssh error is not a shell prefix", "ssh: handshake failed: host key verification failed", false}, + {"word ending in sh followed by colon", "Publish: failed to upload the artifact", false}, + {"shell prefix mid-line is ignored", "Wrote log to C:\\tmp\\bash: notes.txt", false}, + {"shell prefix on a later line", "#< CLIXML\nksh: powershell: cannot execute", true}, } for _, c := range cases { diff --git a/hostkey_verification_test.go b/hostkey_verification_test.go index 8f9e909..e284703 100644 --- a/hostkey_verification_test.go +++ b/hostkey_verification_test.go @@ -3,6 +3,7 @@ package main import ( "crypto/ed25519" "crypto/rand" + "crypto/rsa" "encoding/base64" "fmt" "net" @@ -46,10 +47,27 @@ func generateHostKey(t *testing.T) ssh.Signer { return signer } +// generateRSAHostKey はテスト用の RSA ホスト鍵を生成する。 +// 実機の Windows OpenSSH は rsa / ecdsa / ed25519 を同時に提供するため、 +// 「サーバが複数種別を出す」状況を再現するのに使う。 +func generateRSAHostKey(t *testing.T) ssh.Signer { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate rsa key: %v", err) + } + signer, err := ssh.NewSignerFromKey(key) + if err != nil { + t.Fatalf("new signer: %v", err) + } + return signer +} + // startTestSSHServer は 127.0.0.1 上に password 認証のみの SSH サーバを起動し、ポートを返す。 // dialSSH はハンドシェイク＋認証完了で *ssh.Client を返す（チャネルは開かない）ため、 // チャネル要求は拒否で十分。リスナーは t.Cleanup でクローズする。 -func startTestSSHServer(t *testing.T, hostKey ssh.Signer) int { +// ホスト鍵は可変長で受け取り、複数渡すと実機同様に複数種別を提供するサーバになる。 +func startTestSSHServer(t *testing.T, hostKeys ...ssh.Signer) int { t.Helper() config := &ssh.ServerConfig{ @@ -60,7 +78,9 @@ func startTestSSHServer(t *testing.T, hostKey ssh.Signer) int { return nil, fmt.Errorf("authentication failed") }, } - config.AddHostKey(hostKey) + for _, hostKey := range hostKeys { + config.AddHostKey(hostKey) + } ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -358,3 +378,100 @@ func TestHostKeyVerification_HashKnownHosts_AppendsHashed(t *testing.T) { t.Errorf("new entry must be hashed when known_hosts already has hashed entries, got: %q", appended) } } + +// TestHostKeyVerification_WildcardKnownHosts_NoFalseMitm は、known_hosts のホスト行が +// ワイルドカードパターン（例: `192.168.1.*`）の場合でもホスト鍵アルゴリズムの制限が効き、 +// 「変わっていないホスト」に対して REMOTE HOST IDENTIFICATION HAS CHANGED の +// 誤警告が出ないことを検証する。 +// +// 回帰の中身: hostKeyAlgorithmsFromKnownHosts がホスト行を完全一致でしか見ていなかった頃、 +// パターン行は「未登録」と判定されて制限が nil になっていた。一方 knownhosts コールバックは +// パターンを解釈するため、サーバが複数種別のホスト鍵を持つと Go が known_hosts に無い種別 +// （ここでは rsa）をネゴシエートし、コールバックが「登録済みの鍵と違う」＝鍵変更として +// KeyError.Want 付きで弾く。結果、正常なホストで MITM 警告とプロンプトが出ていた。 +func TestHostKeyVerification_WildcardKnownHosts_NoFalseMitm(t *testing.T) { + edKey := generateHostKey(t) + rsaKey := generateRSAHostKey(t) + // 実機同様、サーバは rsa と ed25519 の両方を提供する + port := startTestSSHServer(t, rsaKey, edKey) + + knownHostsPath := tempHome(t) + if err := os.MkdirAll(filepath.Dir(knownHostsPath), 0700); err != nil { + t.Fatalf("mkdir .ssh: %v", err) + } + // known_hosts にはワイルドカードで ed25519 鍵だけが登録されている + pattern := fmt.Sprintf("[127.0.0.*]:%d", port) + line := knownhosts.Line([]string{pattern}, edKey.PublicKey()) + "\n" + if err := os.WriteFile(knownHostsPath, []byte(line), 0600); err != nil { + t.Fatalf("seed known_hosts: %v", err) + } + + failIfPrompted(t) + + client, err := dialSSH(testSSHUser, "127.0.0.1", port, testSSHPassword, false) + if err != nil { + t.Fatalf("dialSSH with wildcard known_hosts entry failed: %v", err) + } + defer client.Close() + + // known_hosts は書き換えられていないこと（誤検知で行が消えたり増えたりしない） + after, err := os.ReadFile(knownHostsPath) + if err != nil { + t.Fatalf("read known_hosts: %v", err) + } + if string(after) != line { + t.Errorf("known_hosts was modified\n got: %q\nwant: %q", string(after), line) + } +} + +// TestHostKeyVerification_WildcardAlgorithmMismatch_RetriesAndPrompts は、ワイルドカード行から +// 取り出したアルゴリズムがサーバの実際のホスト鍵種別と合わない場合に、dialSSH が制限を外して +// 再試行し、対話プロンプトまで到達することを検証する。 +// +// 照合をワイルドカード対応にしたことで、以前は nil（＝制限なし）だったケースに制限が付く。 +// known_hosts のパターン行が実サーバと別種別の鍵を指していると +// "no common algorithm for host key" でハンドシェイクが落ちるため、 +// shouldRetryWithoutHostKeyAlgorithms による再試行が効かないと、それまで +// プロンプトで回復できていた構成がハードエラーに変わってしまう。 +// 併せて shouldRetryWithoutHostKeyAlgorithms の文字列判定が、現行の x/crypto が返す +// 実エラーに対して機能していることも確認する（予測文字列ではなく実ハンドシェイク由来）。 +func TestHostKeyVerification_WildcardAlgorithmMismatch_RetriesAndPrompts(t *testing.T) { + rsaKey := generateRSAHostKey(t) + // サーバは rsa のみを提供する + port := startTestSSHServer(t, rsaKey) + + knownHostsPath := tempHome(t) + if err := os.MkdirAll(filepath.Dir(knownHostsPath), 0700); err != nil { + t.Fatalf("mkdir .ssh: %v", err) + } + // known_hosts のワイルドカード行は ed25519 鍵を指しており、サーバとは種別が食い違う + pattern := fmt.Sprintf("[127.0.0.*]:%d", port) + wildcardLine := knownhosts.Line([]string{pattern}, generateHostKey(t).PublicKey()) + "\n" + if err := os.WriteFile(knownHostsPath, []byte(wildcardLine), 0600); err != nil { + t.Fatalf("seed known_hosts: %v", err) + } + + promptCount := overridePrompt(t, "yes") + + client, err := dialSSH(testSSHUser, "127.0.0.1", port, testSSHPassword, false) + if err != nil { + t.Fatalf("dialSSH should have retried without the host key algorithm restriction: %v", err) + } + defer client.Close() + + if *promptCount != 1 { + t.Errorf("prompt count = %d, want 1 (retry must reach the interactive callback)", *promptCount) + } + + // 承認後: ワイルドカード行は残ったまま（削除経路は完全一致のみ）、実鍵が追記されている。 + after, err := os.ReadFile(knownHostsPath) + if err != nil { + t.Fatalf("read known_hosts: %v", err) + } + if !strings.Contains(string(after), strings.TrimSpace(wildcardLine)) { + t.Errorf("wildcard line must not be deleted (it may cover other hosts)\ngot: %q", string(after)) + } + if !strings.Contains(string(after), keyB64(rsaKey.PublicKey())) { + t.Errorf("accepted host key was not appended\ngot: %q", string(after)) + } +} diff --git a/ssh.go b/ssh.go index e49f52b..ad82b97 100644 --- a/ssh.go +++ b/ssh.go @@ -216,7 +216,13 @@ func matchHashedHost(pattern, addr string) bool { } // hostMatchesAddr はknown_hostsのホストフィールド（plain-textまたはハッシュ形式）が -// 指定アドレスにマッチするかを判定する。 +// 指定アドレスに**完全一致**するかを判定する。 +// +// ワイルドカードは意図的に解釈しない。この関数の利用者は +// replaceHostKeyInKnownHosts（＝マッチした行を known_hosts から削除する経路）であり、 +// ここでパターンを展開すると `192.168.1.*` のような 1 行が他ホストの鍵も巻き添えに消える。 +// 読み取り専用の照合（hostKeyAlgorithmsFromKnownHosts）は +// knownHostsLineMatchesAddr を使い、OpenSSH と同じパターン解釈を行う。 func hostMatchesAddr(host, addr string) bool { if strings.HasPrefix(host, "|") { return matchHashedHost(host, addr) @@ -224,8 +230,79 @@ func hostMatchesAddr(host, addr string) bool { return host == addr } +// hostPatternMatch は OpenSSH の known_hosts ホストパターン（`*` = 0 文字以上、 +// `?` = 任意の 1 文字）が addr にマッチするかを判定する。 +// known_hosts のホストフィールドは ASCII（ホスト名 / IP / `[addr]:port`）なので +// バイト単位で比較する。バックトラックは `*` の位置を 1 つ覚えるだけの線形スキャンで足りる。 +func hostPatternMatch(pattern, addr string) bool { + pi, ai := 0, 0 + star, starMatch := -1, 0 + for ai < len(addr) { + switch { + case pi < len(pattern) && (pattern[pi] == '?' || pattern[pi] == addr[ai]): + pi++ + ai++ + case pi < len(pattern) && pattern[pi] == '*': + star, starMatch = pi, ai + pi++ + case star >= 0: + // 直前の `*` に 1 文字余計に食わせてやり直す + starMatch++ + pi, ai = star+1, starMatch + default: + return false + } + } + for pi < len(pattern) && pattern[pi] == '*' { + pi++ + } + return pi == len(pattern) +} + +// knownHostsLineMatchesAddr は known_hosts 行のホストフィールド（カンマ区切り）が +// addr に適用されるかを OpenSSH と同じ規則で判定する。 +// - ハッシュ化エントリ（|1|salt|hash）は HMAC で照合する（ワイルドカードは持てない） +// - plain-text エントリは `*` / `?` のワイルドカードを解釈する +// - `!pattern` の否定が 1 つでもマッチしたら、その行は addr に適用されない +// （順序に関係なく否定が勝つため、肯定一致で早期 return してはならない） +func knownHostsLineMatchesAddr(hostField, addr string) bool { + matched := false + for _, h := range strings.Split(hostField, ",") { + h = strings.TrimSpace(h) + if h == "" { + continue + } + if negated, ok := strings.CutPrefix(h, "!"); ok { + if hostPatternMatch(negated, addr) { + return false + } + continue + } + if strings.HasPrefix(h, "|") { + if matchHashedHost(h, addr) { + matched = true + } + continue + } + if hostPatternMatch(h, addr) { + matched = true + } + } + return matched +} + // hostKeyAlgorithmsFromKnownHosts はknown_hostsファイルから対象ホストの鍵アルゴリズム一覧を返す。 // ホストが未登録の場合はnilを返し、SSHクライアントのデフォルト動作に委ねる。 +// +// ホスト照合は knownHostsLineMatchesAddr（OpenSSH 準拠のワイルドカード＋否定）で行う。 +// 完全一致だけで見ていると `192.168.1.*` のようなパターン行が「未登録」と判定され、 +// 制限なしでネゴシエーションした結果 known_hosts に載っていない種別の鍵が選ばれ、 +// 実際には何も変わっていないホストに対して「HOST IDENTIFICATION HAS CHANGED」を +// 誤表示する（knownhosts コールバック側はパターンを解釈するため食い違う）。 +// +// `@cert-authority` / `@revoked` のマーカー行は対象外にする。前者が持つのは CA 鍵の種別で +// あってホスト鍵の種別ではなく、これを制限リストに混ぜると証明書ホストへの接続を壊す。 +// マーカー行しか無いホストでは nil（＝制限なし）を返すのが安全側。 func hostKeyAlgorithmsFromKnownHosts(knownHostsPath string, addr string) []string { data, err := os.ReadFile(knownHostsPath) if err != nil { @@ -235,19 +312,15 @@ func hostKeyAlgorithmsFromKnownHosts(knownHostsPath string, addr string) []strin var algorithms []string for _, line := range strings.Split(string(data), "\n") { trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { + if trimmed == "" || strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "@") { continue } fields := strings.Fields(trimmed) if len(fields) < 3 { continue } - hosts := strings.Split(fields[0], ",") - for _, h := range hosts { - if hostMatchesAddr(h, addr) { - algorithms = append(algorithms, fields[1]) - break - } + if knownHostsLineMatchesAddr(fields[0], addr) { + algorithms = append(algorithms, fields[1]) } } return algorithms diff --git a/ssh_test.go b/ssh_test.go index 28bcc96..b9a6caa 100644 --- a/ssh_test.go +++ b/ssh_test.go @@ -199,3 +199,123 @@ func TestAtomicWriteFile_CreatesAndOverwrites(t *testing.T) { t.Errorf("expected only known_hosts to remain, got %v", names) } } + +func TestHostPatternMatch(t *testing.T) { + tests := []struct { + pattern string + addr string + want bool + }{ + {"example.com", "example.com", true}, + {"example.com", "other.com", false}, + {"*", "anything", true}, + {"*.example.com", "win.example.com", true}, + {"*.example.com", "example.com", false}, + {"192.168.1.*", "192.168.1.10", true}, + {"192.168.1.*", "192.168.2.10", false}, + {"192.168.?.10", "192.168.1.10", true}, + {"192.168.?.10", "192.168.10.10", false}, + {"[192.168.1.*]:2222", "[192.168.1.10]:2222", true}, + {"[192.168.1.*]:2222", "[192.168.1.10]:22", false}, + {"a*b*c", "axxbyyc", true}, + {"a*b*c", "axxbyy", false}, + {"", "", true}, + {"", "x", false}, + {"*", "", true}, + } + for _, tt := range tests { + if got := hostPatternMatch(tt.pattern, tt.addr); got != tt.want { + t.Errorf("hostPatternMatch(%q, %q) = %v, want %v", tt.pattern, tt.addr, got, tt.want) + } + } +} + +func TestKnownHostsLineMatchesAddr(t *testing.T) { + hashed := knownhosts.HashHostname("secret.example.com") + tests := []struct { + name string + hostField string + addr string + want bool + }{ + {"exact", "example.com", "example.com", true}, + {"wildcard", "192.168.1.*", "192.168.1.10", true}, + {"wildcard no match", "192.168.1.*", "10.0.0.1", false}, + {"comma list", "a.example.com,192.168.1.*", "192.168.1.10", true}, + {"negation vetoes wildcard", "192.168.1.*,!192.168.1.10", "192.168.1.10", false}, + {"negation before positive still vetoes", "!192.168.1.10,192.168.1.*", "192.168.1.10", false}, + {"negation of other host", "192.168.1.*,!192.168.1.11", "192.168.1.10", true}, + {"hashed", hashed, "secret.example.com", true}, + {"hashed other addr", hashed, "other.example.com", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := knownHostsLineMatchesAddr(tt.hostField, tt.addr); got != tt.want { + t.Errorf("knownHostsLineMatchesAddr(%q, %q) = %v, want %v", tt.hostField, tt.addr, got, tt.want) + } + }) + } +} + +// TestHostMatchesAddr_IgnoresWildcards は削除経路（replaceHostKeyInKnownHosts）が使う +// hostMatchesAddr がワイルドカードを展開しないことを固定する。展開してしまうと +// `192.168.1.*` の 1 行を消したときに他ホストの鍵も巻き添えで消える。 +func TestHostMatchesAddr_IgnoresWildcards(t *testing.T) { + if hostMatchesAddr("192.168.1.*", "192.168.1.10") { + t.Error("hostMatchesAddr must not expand wildcards (deletion path would remove other hosts)") + } +} + +func TestHostKeyAlgorithmsFromKnownHosts(t *testing.T) { + const ( + rsaKey = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDdummy" + edKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIdummy" + ) + write := func(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "known_hosts") + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatalf("write known_hosts: %v", err) + } + return path + } + + t.Run("wildcard entry is honored", func(t *testing.T) { + path := write(t, "192.168.1.* "+edKey+"\n") + got := hostKeyAlgorithmsFromKnownHosts(path, "192.168.1.10") + if len(got) != 1 || got[0] != "ssh-ed25519" { + t.Errorf("got %v, want [ssh-ed25519]", got) + } + }) + + t.Run("wildcard and exact entries are combined", func(t *testing.T) { + path := write(t, "192.168.1.* "+edKey+"\n192.168.1.10 "+rsaKey+"\n") + got := hostKeyAlgorithmsFromKnownHosts(path, "192.168.1.10") + if len(got) != 2 || got[0] != "ssh-ed25519" || got[1] != "ssh-rsa" { + t.Errorf("got %v, want [ssh-ed25519 ssh-rsa]", got) + } + }) + + t.Run("negated host is excluded", func(t *testing.T) { + path := write(t, "192.168.1.*,!192.168.1.10 "+edKey+"\n") + if got := hostKeyAlgorithmsFromKnownHosts(path, "192.168.1.10"); got != nil { + t.Errorf("got %v, want nil", got) + } + }) + + t.Run("marker lines are skipped", func(t *testing.T) { + // @cert-authority が持つのは CA 鍵の種別。制限リストに混ぜると証明書ホストを壊すため、 + // マーカー行しか無いホストは nil（＝制限なし）にフォールバックする。 + path := write(t, "@cert-authority *.example.com "+rsaKey+"\n@revoked win.example.com "+edKey+"\n") + if got := hostKeyAlgorithmsFromKnownHosts(path, "win.example.com"); got != nil { + t.Errorf("got %v, want nil", got) + } + }) + + t.Run("unknown host yields nil", func(t *testing.T) { + path := write(t, "10.0.0.1 "+edKey+"\n") + if got := hostKeyAlgorithmsFromKnownHosts(path, "192.168.1.10"); got != nil { + t.Errorf("got %v, want nil", got) + } + }) +} From 19b813dbb81688431dd0c15f354699993abecb48 Mon Sep 17 00:00:00 2001 From: yugosasaki Date: Fri, 18 Sep 2026 18:15:00 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E3=82=B3=E3=83=A1=E3=83=B3=E3=83=88=E3=81=AB=E5=AF=BE=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01SDRW423QvvZ1t3ESWPezBn --- deploy.go | 37 +++++++++++++++++++++++++++++++------ deploy_test.go | 6 ++++++ ssh.go | 23 ++++++++++++++++++++++- ssh_test.go | 12 ++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/deploy.go b/deploy.go index 07d726d..6c1b4b2 100644 --- a/deploy.go +++ b/deploy.go @@ -3,6 +3,7 @@ package main import ( "fmt" "net" + "path" "strconv" "strings" @@ -198,7 +199,8 @@ var nonWindowsMessageSignatures = []string{ } // nonWindowsShellPrefixes は Unix シェルがコマンド不在を報告するときの行頭。 -// これらは**行頭一致**でしか見ない。部分一致にすると "ssh:" や "…finish:" のような +// これらは**行頭一致**、または行頭が `/` の場合にかぎり**絶対パスの basename 一致** +// でしか見ない。無条件の部分一致にすると "ssh:" や "…finish:" のような // 無関係な文字列が "sh:" を含むだけで非 Windows と誤判定され、graceful degradation では // なく「Windows ではない」と即座に中断してしまう（markerValue / hasMarkerLine と同じ方針）。 // `env:` は入れない。PowerShell 自身の `$env:VAR` 名前空間と衝突し、 @@ -209,6 +211,32 @@ var nonWindowsShellPrefixes = []string{ "powershell:", } +// hasNonWindowsShellPrefix は小文字化済みの 1 行が Unix シェルの自己申告で始まるか判定する。 +// シェルは自分の名前（`sh:`）でも絶対パス（`/bin/sh: 1: powershell: not found`）でも +// 名乗るため、行頭が `/` のときにかぎり最初の `:` までを basename に落として照合する。 +// `/` 始まりに限定しているのは "ssh: handshake failed" のような行を巻き込まないため。 +func hasNonWindowsShellPrefix(line string) bool { + for _, prefix := range nonWindowsShellPrefixes { + if strings.HasPrefix(line, prefix) { + return true + } + } + if !strings.HasPrefix(line, "/") { + return false + } + colon := strings.Index(line, ":") + if colon <= 0 { + return false + } + base := path.Base(line[:colon]) + ":" + for _, prefix := range nonWindowsShellPrefixes { + if base == prefix { + return true + } + } + return false +} + // looksLikeNonWindows は PowerShell 実行エラー出力が Linux/非 Windows ホストを示すか判定する。 func looksLikeNonWindows(output string) bool { lower := strings.ToLower(output) @@ -218,11 +246,8 @@ func looksLikeNonWindows(output string) bool { } } for _, line := range strings.Split(lower, "\n") { - trimmed := strings.TrimSpace(line) - for _, prefix := range nonWindowsShellPrefixes { - if strings.HasPrefix(trimmed, prefix) { - return true - } + if hasNonWindowsShellPrefix(strings.TrimSpace(line)) { + return true } } return false diff --git a/deploy_test.go b/deploy_test.go index 34988ba..777ac17 100644 --- a/deploy_test.go +++ b/deploy_test.go @@ -499,6 +499,12 @@ func TestLooksLikeNonWindows(t *testing.T) { {"word ending in sh followed by colon", "Publish: failed to upload the artifact", false}, {"shell prefix mid-line is ignored", "Wrote log to C:\\tmp\\bash: notes.txt", false}, {"shell prefix on a later line", "#< CLIXML\nksh: powershell: cannot execute", true}, + // 絶対パスで名乗るシェル。dash の "not found" は message 断片に一致しないため、 + // basename 一致が無いと Windows 扱いのまま配置経路へ進んでしまう。 + {"absolute path dash", "/bin/sh: 1: powershell: not found", true}, + {"absolute path bash", "/usr/bin/bash: powershell: No such file", true}, + {"absolute path non-shell binary", "/usr/bin/git: 'foo' is not a git command", false}, + {"absolute path without colon", "/bin/sh is a shell", false}, } for _, c := range cases { diff --git a/ssh.go b/ssh.go index ad82b97..8c50833 100644 --- a/ssh.go +++ b/ssh.go @@ -223,6 +223,11 @@ func matchHashedHost(pattern, addr string) bool { // ここでパターンを展開すると `192.168.1.*` のような 1 行が他ホストの鍵も巻き添えに消える。 // 読み取り専用の照合（hostKeyAlgorithmsFromKnownHosts）は // knownHostsLineMatchesAddr を使い、OpenSSH と同じパターン解釈を行う。 +// +// 大文字小文字も畳まない（hostPatternMatch とは非対称）。この関数が走るのは +// x/crypto/ssh/knownhosts のコールバックが「鍵が変わった」と判定した後だけで、 +// そのコールバック自身がバイト完全一致（同パッケージに大小文字を畳む処理は無い）。 +// ここだけ広げると、コールバックが一致させていない行まで削除対象になる。 func hostMatchesAddr(host, addr string) bool { if strings.HasPrefix(host, "|") { return matchHashedHost(host, addr) @@ -230,16 +235,32 @@ func hostMatchesAddr(host, addr string) bool { return host == addr } +// lowerASCII は ASCII 大文字 1 バイトを小文字に畳む。 +// known_hosts のホストフィールドは ASCII（ホスト名 / IP / `[addr]:port`）なので +// Unicode 対応（strings.EqualFold）は不要で、バイト単位で足りる。 +func lowerASCII(b byte) byte { + if 'A' <= b && b <= 'Z' { + return b + ('a' - 'A') + } + return b +} + // hostPatternMatch は OpenSSH の known_hosts ホストパターン（`*` = 0 文字以上、 // `?` = 任意の 1 文字）が addr にマッチするかを判定する。 // known_hosts のホストフィールドは ASCII（ホスト名 / IP / `[addr]:port`）なので // バイト単位で比較する。バックトラックは `*` の位置を 1 つ覚えるだけの線形スキャンで足りる。 +// +// 比較は**大文字小文字を区別しない**。OpenSSH の match_hostname() はホスト名と +// パターンの両方を lowercase してから照合するため、区別するとパターン行が +// 取りこぼされる（`*.Example.COM` が win.example.com に当たらない）か、 +// 否定が効かなくなる（`!WIN.example.com` が win.example.com を除外できない）。 +// どちらもこの関数が解決しようとしているワイルドカード誤警告をそのまま残してしまう。 func hostPatternMatch(pattern, addr string) bool { pi, ai := 0, 0 star, starMatch := -1, 0 for ai < len(addr) { switch { - case pi < len(pattern) && (pattern[pi] == '?' || pattern[pi] == addr[ai]): + case pi < len(pattern) && (pattern[pi] == '?' || lowerASCII(pattern[pi]) == lowerASCII(addr[ai])): pi++ ai++ case pi < len(pattern) && pattern[pi] == '*': diff --git a/ssh_test.go b/ssh_test.go index b9a6caa..ae99f20 100644 --- a/ssh_test.go +++ b/ssh_test.go @@ -222,6 +222,12 @@ func TestHostPatternMatch(t *testing.T) { {"", "", true}, {"", "x", false}, {"*", "", true}, + // OpenSSH の match_hostname() はホスト名とパターンの両方を lowercase してから + // 照合する。大小を区別すると `*.Example.COM` のようなパターン行を取りこぼす。 + {"*.Example.COM", "win.example.com", true}, + {"*.example.com", "WIN.EXAMPLE.COM", true}, + {"EXAMPLE.com", "example.COM", true}, + {"WIN.example.com", "lose.example.com", false}, } for _, tt := range tests { if got := hostPatternMatch(tt.pattern, tt.addr); got != tt.want { @@ -247,6 +253,12 @@ func TestKnownHostsLineMatchesAddr(t *testing.T) { {"negation of other host", "192.168.1.*,!192.168.1.11", "192.168.1.10", true}, {"hashed", hashed, "secret.example.com", true}, {"hashed other addr", hashed, "other.example.com", false}, + // 否定も大小を区別しない。区別すると `!WIN.example.com` が + // win.example.com を除外できず、パターン行が誤って適用される。 + {"negation folds case", "*.example.com,!WIN.example.com", "win.example.com", false}, + {"negation folds case on addr", "*.example.com,!win.example.com", "WIN.example.com", false}, + // ハッシュ化エントリは HMAC がバイト厳密なので畳まない（畳むと照合が壊れる）。 + {"hashed is not case folded", hashed, "SECRET.example.com", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From ec70d0faf2c218dda346669b4bb151cf0418d8de Mon Sep 17 00:00:00 2001 From: yugosasaki Date: Fri, 18 Sep 2026 18:17:47 +0900 Subject: [PATCH 3/3] =?UTF-8?q?docs(lessons):=20=E5=A4=A7=E5=B0=8F?= =?UTF-8?q?=E6=96=87=E5=AD=97=E7=95=B3=E3=81=BF=E8=BE=BC=E3=81=BF=E3=81=AE?= =?UTF-8?q?=E9=81=A9=E7=94=A8=E7=AF=84=E5=9B=B2=E3=81=A8=E3=82=B7=E3=82=A7?= =?UTF-8?q?=E3=83=AB=E8=87=AA=E5=B7=B1=E7=94=B3=E5=91=8A=E3=81=AE=20basena?= =?UTF-8?q?me=20=E5=88=A4=E5=AE=9A=E3=82=92=E8=A8=98=E9=8C=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SDRW423QvvZ1t3ESWPezBn --- LESSONS.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/LESSONS.md b/LESSONS.md index 562dc59..ab8378d 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -520,3 +520,17 @@ - 却下した案: (A) remote 構成をそのままにし、GitLab を指す `origin` に対して依存更新のブランチを push する。(B) GitHub remote を追加するだけに留め、master の fast-forward と GitLab への push は保留する。 - 決め手: `git remote -v` の出力が `origin git@gitlab.com:kwrkb/ssh-pushkey.git` の 1 件のみで、GitHub remote が存在しなかった。`gh api` で取得した GitHub の master は `6a567fb`、ローカルと `gitlab/master` は `8b91f5f`（乖離ではなく fast-forward 可能）。先行分 3 件には `574daf5 fix: write the Scoop manifest into bucket/` が含まれ、これは Scoop manifest の出力先修正。PLAN.md は GitHub Releases を配布の正本、Scoop manifest はそこから GoReleaser が生成すると定めているため、(A) のまま `v*` タグを push すると GitLab の release ジョブだけが走り、正本のリリースと bucket 更新が沈黙して欠落する。(B) では GitLab が 3 コミット遅れたままになり、次回 master push で非 fast-forward reject を招く。よって `origin` を `gitlab` にリネーム → `origin`=GitHub を追加 → master を `6a567fb` へ fast-forward → GitLab にも push、で 3 者を揃えた。CI・GoReleaser・Makefile は `origin` を参照していないことを grep で確認済み（参照は `.claude/CLAUDE.md` の記述のみ）。 - 覆す条件: 配布の正本が GitHub Releases でなくなる、または Scoop bucket の manifest 生成が GitHub リリースに依存しなくなった場合。その時は remote の主従を再定義してよい。 + +--- + +## 2026-09-18: known_hosts のパターン照合を大文字小文字非依存にする際、畳む場所を 1 関数に閉じた + +- 却下した案: (A) パターン照合を呼ぶ側（カンマ区切りを分解する関数）で `addr` をまとめて lowercase してから各パターンに渡す。(B) 接続先ホスト名を CLI/config 解決の時点で lowercase し、以降すべて正規化済みとして扱う（OpenSSH クライアント本体と同じ方針）。(C) 照合を広げるついでに、鍵変更時の**削除**経路の完全一致比較も同様に畳んで左右対称にする。 +- 決め手: (A) は同じ `addr` がハッシュ化エントリの HMAC 照合にも渡っており、HMAC は入力バイトに厳密なので、呼び出し側で正規化すると混合ケースのホストでハッシュ行が一切引けなくなる（実際、畳まないことを固定する回帰テストを追加した）。(B) と (C) を退けた根拠は同じで、下回りのホスト鍵検証ライブラリのソースに大文字小文字を畳む処理が 1 箇所も無く、アドレス正規化関数も lowercase しないことを確認したこと。削除経路が走るのはそのライブラリのコールバックが「鍵が変わった」と判定した後だけなので、こちら側だけ照合を広げると、ライブラリが一致させていない行まで削除対象に入る。読み取り専用の照合を上流仕様（パターンもホスト名も lowercase してから比較）に合わせるのは安全だが、破壊的操作の範囲は下回りが一致させた範囲を超えてはいけない。 +- 覆す条件: 下回りのライブラリ自身が大文字小文字を畳むようになった場合、または接続先ホスト名を入口で正規化する方針に切り替えた場合。その時は削除経路とハッシュ照合の前提を洗い直す。 + +## 2026-09-18: シェルの自己申告は「行頭一致」だけでは足りず、絶対パスの basename も見る必要があった + +- 却下した案: (A) 以前に削除した「コマンド名 + not found」という固定文字列を部分一致の判定断片として復活させる。(B) シェル名の部分一致を出力全体に戻す。 +- 決め手: 一部の軽量シェルは絶対パスで名乗り（`<絶対パス>: <行番号>: <コマンド>: not found`）、この形は「コマンドが無い」ことを示す既存のメッセージ断片のどれにも一致せず、行頭もシェル名では始まらないため素通りしていた。素通りすると「対象 OS が違う」と報告せずに配置処理へ進む。(B) は直前のコミットで直したばかりの誤検知（無関係な語が `sh:` を部分文字列として含むだけで発火する）をそのまま戻すので不可。(A) は 1 つのシェルにしか効かず、他の絶対パス形を次に踏む。行頭が `/` のときにかぎり最初のコロンまでを basename に落として既存リストと突き合わせる形にすると、誤検知耐性（`/` 始まりでない行は従来どおり行頭一致のみ）を保ったまま全シェルに効いた。 +- 覆す条件: 判定対象の出力に `/` で始まりコロンを含む正常行が現れるようになった場合。その時は basename 照合の発火条件を狭める。