Skip to content

fix(github): derive_keyをHKDF-SHA256に置き換える#279

Open
yupix wants to merge 1 commit into
mainfrom
fix/66-hkdf-key-derivation
Open

fix(github): derive_keyをHKDF-SHA256に置き換える#279
yupix wants to merge 1 commit into
mainfrom
fix/66-hkdf-key-derivation

Conversation

@yupix

@yupix yupix commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

概要

PR #44 で実装した github_token_crypto::derive_key が設定文字列の先頭32バイトを
そのまま鍵として使う(実質的に鍵導出をしていない)実装だったため、HKDF-SHA256
hkdf crate)による適切な鍵導出に置き換える。

Closes #66

変更内容

  • apps/backend/crates/service/src/github_token_crypto.rs
    • derive_keyderive_key_v2(HKDF-SHA256、Hkdf::<Sha256>::new(None, key_material) から
      固定 info 文字列で 32 バイトを expand)に置き換え
    • 旧方式は derive_key_v1 として残し、既存データの復号専用にした(新規暗号化には使わない)
    • encrypt_token は常に v2 で暗号化し、暗号文に "v2:" プレフィックスを付与
    • decrypt_token"v2:" プレフィックスの有無で v1/v2 を判別し、両方式を復号できる
    • 呼び出し側が lazy re-encryption を行えるように decrypt_token_for_migration /
      DecryptedToken { plaintext, needs_reencrypt } を追加(v1 データを検出した場合に
      needs_reencrypt: true を返す)
  • apps/backend/crates/service/Cargo.toml: hkdf = "0.13" を追加(既存の sha2 = "0.11" /
    digest 0.11 系と互換)
  • apps/backend/Cargo.lock: 上記に伴う更新(hkdf 0.13.0 は既存の依存グラフに解決済みで、
    service クレートへの依存エッジ追加のみ)

挙動の変化

  • 新規に暗号化される Installation Access Token は HKDF-SHA256 で導出した鍵を使う
    (フォーマットが "v2:" + base64(nonce + ciphertext) に変わる)
  • 既存の DB に保存済みの v1 暗号文(プレフィックスなし)は引き続き decrypt_token
    復号できる。フォーマット判別は暗号文のプレフィックス文字列で行っており、鍵導出方式の
    違いによる復号失敗は起きない

互換性・移行手順

  • 一括再暗号化 SQL マイグレーションは行わない。 暗号文にバージョンプレフィックス
    v2:)を付け、復号時に v1/v2 どちらの形式も読めるようにした上で、
    lazy re-encryption(読み込み時に v1 を検出したら v2 で再暗号化して保存し直す)を
    推奨方式として採用した。一括再暗号化は原鍵の取り違えや大量データに対する
    一時的なロックのリスクがあり、lazy migration の方が安全と判断
  • ただし、現時点のコードベースには github_integrations.access_token_enc
    「読み出して使う」呼び出し箇所が存在しない(Wave 0 では書き込みのみ。Issue にも
    記載の通り、実際にトークンを読み出して GitHub API を叩く処理は Wave 1 のトークン
    リフレッシュで追加予定)。そのため、本 PR では再暗号化のためのビルディングブロック
    decrypt_token_for_migration / needs_reencrypt フラグ)のみを用意した。
    実際に「読み出し→needs_reencrypt なら再暗号化して UPDATE」を DB に対して行う
    配線は、その読み出し処理自体を追加する Wave 1 の PR で行うのが適切と考え、本 PR の
    スコープ外とした
  • 既存の v1 暗号文は、鍵導出方式が変わっても復号可能なため、Wave 1 が実装されるまでの
    間もサービスに支障はない

テスト

  • cargo fmt --check: pass
  • cargo check --workspace --all-targets: pass
  • cargo clippy --workspace --all-targets: pass(warning 0件)
  • cargo test --workspace --lib: pass(63 tests, うち github_token_crypto 関連 5 tests
    — v2 ラウンドトリップ、v1 後方互換復号、decrypt_token_for_migration のフラグ、
    HKDF 鍵が単純切り取りと異なること、短い鍵材料の拒否)
  • 統合テスト(apps/backend/tests/)は実 Postgres/Redis が必要なため、並行作業中の
    他エージェントとのポート競合を避けてローカルでは実行していない。CI で実行される

Installation Access Token の鍵導出が先頭32バイト切り取りのみだった問題を
HKDF-SHA256 (hkdf crate) に置き換える。既存の暗号化済みトークンとの互換を
保つため、暗号文に v2 プレフィックスを付け、復号時は旧方式 (v1) のデータも
読める後方互換ロジックを実装した。新規暗号化は常に v2 のみを使う。
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying koyori with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5c67fb6
Status: ✅  Deploy successful!
Preview URL: https://3cfd39b6.koyori.pages.dev
Branch Preview URL: https://fix-66-hkdf-key-derivation.koyori.pages.dev

View logs

@github-actions

Copy link
Copy Markdown

コードを全体的に精査しました。実装は以下の点で適切です:

  1. v1/v2 判別: base64 アルファベットに : が含まれないため、"v2:" プレフィックスによる v1/v2 判別は曖昧さなし
  2. HKDF の salt None: RFC 5869 に従い HashLen 分のゼロバイト列として処理される。info 文字列でドメイン分離されているため問題なし
  3. check_key_material_len: str::len() は Rust ではバイト長を返すため、「32 bytes」のエラーメッセージと整合
  4. derive_key_v1bytes[..32]: check_key_material_len がバイト長 >= 32 を保証するためスライスは安全
  5. テスト: v2 ラウンドトリップ、v1 後方互換、migration フラグ、v1≠v2 キー、短い鍵材料の拒否の 5 カバー

前回のレビューコメントは存在しないため、差分全体を対象にレビューしました。問題は見つかりませんでした。

New%20session%20-%202026-07-10T00%3A59%3A22.668Z
opencode session  |  github run

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.

feat(github): derive_key を HKDF-SHA256 に置き換える (Wave 1+)

1 participant