Skip to content

Use hex-encode DEK for keychain safety, prefer Wayland when available on Linux#22

Open
Mooling0602 wants to merge 4 commits into
QingJ01:masterfrom
Mooling0602:fix/dek-hex-encoding
Open

Use hex-encode DEK for keychain safety, prefer Wayland when available on Linux#22
Mooling0602 wants to merge 4 commits into
QingJ01:masterfrom
Mooling0602:fix/dek-hex-encoding

Conversation

@Mooling0602
Copy link
Copy Markdown

@Mooling0602 Mooling0602 commented May 5, 2026

Summary 摘要

Implement #18

  • Hex-encode DEK before storing to OS keychain — 解决字符串型 keychain backend(如 Linux kernel-keyring)无法安全传输原始二进制密钥的问题。同时支持旧二进制格式的透明迁移。
  • Prefer Wayland over hardcoded X11 on Linux — 运行时检测到 WAYLAND_DISPLAY 时设置 GDK_BACKEND=wayland,覆盖 AppImage GTK 插件硬编码的 GDK_BACKEND=x11。不影响纯 X11 环境,同一个二进制包在两种显示协议下均可正常运行。
  • Skip stripping in Linux AppImage build — 添加 NO_STRIP=1 避免构建阶段 strip 失败。

Changelog 变更详情

File Description / 说明
crates/pebble-crypto/src/keystore.rs Hex-encode DEK before storing; auto-detect and migrate legacy binary format on read; add decode_hex() helper; constant 32DEK_LEN. DEK 存储前 hex 编码;读取时自动检测并迁移旧二进制格式;新增 decode_hex() 辅助函数;常量 32DEK_LEN
src-tauri/src/lib.rs Conditionally set GDK_BACKEND=wayland at startup when a Wayland compositor is detected; does not affect pure X11 environments. 启动时检测到 Wayland compositor 时设置 GDK_BACKEND=wayland,不影响纯 X11 环境
package.json Add NO_STRIP=1 to build:linux script. build:linux 脚本添加 NO_STRIP=1
Cargo.toml / crates/pebble-crypto/Cargo.toml Add hex = "0.4" workspace dependency. 新增 hex = "0.4" workspace 依赖
Cargo.lock Lock hex crate. hex crate 锁定

Test plan 测试计划

  • cargo build 通过
  • cargo test 通过
  • npm test 通过
  • Linux 下 AppImage 启动后检测 GDK_BACKEND=wayland 设置生效

🤖 Generated with Claude Code

@Mooling0602
Copy link
Copy Markdown
Author

在某些发行版(例如Arch Linux上)编译项目时会出现该错误:

failed to bundle project `failed to run linuxdeploy`
       Error failed to bundle project `failed to run linuxdeploy`
 ELIFECYCLE  Command failed with exit code 1.

因此在 97b8074#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519 添加了 NO_STRIP=1 环境变量。

@QingJ01
Copy link
Copy Markdown
Owner

QingJ01 commented May 5, 2026

我这边核实了一下,现在不适合合并。主要有两个阻断问题:

  1. 旧 DEK 的迁移逻辑实际不会迁移 raw 32-byte 旧值。当前 get_secret() 成功后只走 decode_hex(),而 decode_hex() 会先按 UTF-8 + hex 解码;旧版本存的是 32-byte raw DEK,解码失败后会进入 delete_credential() 并重新生成 DEK。这会导致已有用户的加密数据无法再用旧 DEK 解密。

    建议处理方式:如果 secret.len() == 32,直接把它复制成 DEK,然后 set_secret(hex::encode(&*key).as_bytes()) 完成迁移;其他长度/格式才视为无效。

  2. cargo fmt --check 当前失败,位置在 crates/pebble-crypto/src/keystore.rsdecode_hex() 换行处,跑一下 cargo fmt 即可。

另外两个建议:

  • GDK_BACKEND 现在只要检测到 WAYLAND_DISPLAY 就无条件覆盖为 wayland,会覆盖用户显式设置的 GDK_BACKEND=x11,而且不限于 AppImage 场景。建议至少只在 GDK_BACKEND 未设置时修改,或更精确地识别 AppImage 的错误默认值。
  • hex_key 以及 hex::decode() 返回的 String/Vec<u8> 都包含 DEK 派生内容,建议用 Zeroizing<String> / Zeroizing<Vec<u8>> 包一下。

我本地验证结果:

  • cargo fmt --check:失败
  • cargo test -p pebble-crypto:通过,4 passed,2 ignored
  • 临时 legacy raw DEK 迁移测试:失败,可复现上面的迁移问题
  • cargo check -p pebble:通过
  • pnpm test:通过,78 files / 275 tests

所以这个 PR 方向是对的,但需要先修掉旧 DEK 迁移问题和 fmt。

@Mooling0602
Copy link
Copy Markdown
Author

我这边核实了一下,现在不适合合并。主要有两个阻断问题:

  1. 旧 DEK 的迁移逻辑实际不会迁移 raw 32-byte 旧值。当前 get_secret() 成功后只走 decode_hex(),而 decode_hex() 会先按 UTF-8 + hex 解码;旧版本存的是 32-byte raw DEK,解码失败后会进入 delete_credential() 并重新生成 DEK。这会导致已有用户的加密数据无法再用旧 DEK 解密。
    建议处理方式:如果 secret.len() == 32,直接把它复制成 DEK,然后 set_secret(hex::encode(&*key).as_bytes()) 完成迁移;其他长度/格式才视为无效。
  2. cargo fmt --check 当前失败,位置在 crates/pebble-crypto/src/keystore.rsdecode_hex() 换行处,跑一下 cargo fmt 即可。

另外两个建议:

  • GDK_BACKEND 现在只要检测到 WAYLAND_DISPLAY 就无条件覆盖为 wayland,会覆盖用户显式设置的 GDK_BACKEND=x11,而且不限于 AppImage 场景。建议至少只在 GDK_BACKEND 未设置时修改,或更精确地识别 AppImage 的错误默认值。
  • hex_key 以及 hex::decode() 返回的 String/Vec<u8> 都包含 DEK 派生内容,建议用 Zeroizing<String> / Zeroizing<Vec<u8>> 包一下。

我本地验证结果:

  • cargo fmt --check:失败
  • cargo test -p pebble-crypto:通过,4 passed,2 ignored
  • 临时 legacy raw DEK 迁移测试:失败,可复现上面的迁移问题
  • cargo check -p pebble:通过
  • pnpm test:通过,78 files / 275 tests

所以这个 PR 方向是对的,但需要先修掉旧 DEK 迁移问题和 fmt。

已尝试对以上问题进行修复:3a6c6f6

@QingJ01
Copy link
Copy Markdown
Owner

QingJ01 commented May 9, 2026

审核了当前 PR 头提交 3a6c6f6,建议先处理下面两个点后再合并。

  1. src-tauri/src/lib.rs 的 Wayland 逻辑没有真正覆盖 AppImage 场景。现在只有在 GDK_BACKEND 未设置时才会设置为 wayland,但注释里也说明 AppImage GTK 插件会在 AppRun hook 里预先硬编码 GDK_BACKEND=x11。这意味着实际 AppImage 启动时变量已经存在,即使 WAYLAND_DISPLAY 可用,也不会切到 Wayland,PR 原本要修的 AppImage x11 默认值问题仍然存在。建议明确识别并覆盖 AppImage 注入的默认 x11,或者把 PR 目标改成“不覆盖任何已有用户/环境设置”,但现在代码和注释表达的意图不一致。

  2. crates/pebble-crypto/src/keystore.rs 的 legacy raw DEK 分支需要继续 zeroize 读出的原始 secret。当前 entry.get_secret() 返回的 Vec<u8> 在复制到 Zeroizing<[u8; 32]> 后普通 drop,旧实现会把这份 buffer 包在 Zeroizing 里。建议在 Ok(secret) if secret.len() == DEK_LEN 分支里先 let secret = Zeroizing::new(secret);,再 key.copy_from_slice(&secret),避免迁移路径把 raw DEK 留在未清零内存中。

我本地验证了:

  • git merge-tree origin/master origin/pr/22 无冲突
  • cargo fmt --check 通过
  • cargo test -p pebble-crypto 通过
  • cargo check -p pebble 通过

Mooling0602 added a commit to Mooling0602/Pebble that referenced this pull request May 11, 2026
- Detect AppImage via APPDIR env var and override its hardcoded
  GDK_BACKEND=x11 when a Wayland compositor is available, while
  still respecting explicit user-set GDK_BACKEND values.
- Wrap the raw secret Vec<u8> in Zeroizing before copy_from_slice
  in the legacy binary DEK migration path to avoid leaving the
  plaintext key in unzeroed memory.

Ref: QingJ01#22 (comment)
@Mooling0602
Copy link
Copy Markdown
Author

已针对两个问题进行了修复:

1. AppImage GDK_BACKEND 覆盖逻辑

通过 `APPDIR` 环境变量识别 AppImage 场景:当检测到 AppImage 注入的 `GDK_BACKEND=x11` 且 Wayland compositor 可用时,覆盖为 `wayland`。非 AppImage 场景下用户显式设置的 `GDK_BACKEND` 不受影响。

2. Legacy DEK 迁移路径 zeroize

在 `Ok(secret) if secret.len() == DEK_LEN` 分支中先用 `Zeroizing::new(secret)` 包裹 `get_secret()` 返回的 `Vec`,再执行 `copy_from_slice`,避免 raw DEK 经普通 drop 残留在未清零内存。

本地验证:

  • `cargo fmt --check` ✅
  • `cargo test -p pebble-crypto` ✅ 4 passed
  • `cargo check -p pebble` ✅

请再次审核,谢谢!

@QingJ01
Copy link
Copy Markdown
Owner

QingJ01 commented May 16, 2026

感谢继续修复,之前提到的 legacy raw DEK 迁移和 AppImage GDK_BACKEND=x11 覆盖问题现在看起来已经处理了。

我这边又跑了一轮验证,当前还有一个阻断点:CI 的 clippy 会失败。仓库里会执行:

cargo clippy --workspace --all-targets -- -D warnings

当前失败位置在 crates/pebble-crypto/src/keystore.rs

  • line 29: hex::encode(&*key)
  • line 53: hex::encode(&*key)

触发的是 clippy::needless-borrows-for-generic-args。建议不要直接改成 hex::encode(*key),因为那会复制一份 DEK;可以改成传 slice,例如:

hex::encode(&key[..])

另外两个非阻断建议:

  1. legacy raw DEK 迁移分支里 entry.set_secret(...) 现在用 let _ = ... 静默忽略写入失败,建议至少打 warn,或者直接返回 Auth 错误。
  2. 如果目标是“prefer Wayland”,GDK_BACKEND=wayland,x11 可能比纯 wayland 更合适,可以保留 X11 fallback。

我本地验证结果:

  • git merge-tree origin/master origin/pr/22:无冲突
  • git diff --check:通过
  • cargo fmt --check:通过
  • cargo test -p pebble-crypto:通过
  • cargo check -p pebble:通过
  • cargo clippy -p pebble-crypto -p pebble --all-targets -- -D warnings:失败,原因如上

修掉 clippy 后我认为这个 PR 就比较接近可以合并了。

Mooling0602 added a commit to Mooling0602/Pebble that referenced this pull request May 18, 2026
- Detect AppImage via APPDIR env var and override its hardcoded
  GDK_BACKEND=x11 when a Wayland compositor is available, while
  still respecting explicit user-set GDK_BACKEND values.
- Wrap the raw secret Vec<u8> in Zeroizing before copy_from_slice
  in the legacy binary DEK migration path to avoid leaving the
  plaintext key in unzeroed memory.

Ref: QingJ01#22 (comment)
@Mooling0602 Mooling0602 force-pushed the fix/dek-hex-encoding branch from 9485fce to 0122229 Compare May 18, 2026 01:27
Mooling0602 and others added 4 commits May 18, 2026 09:29
- Hex-encode encryption key before persisting to OS keychain for
  better compatibility with string-based keychain backends
- Auto-detect and migrate legacy binary-format entries on read
- Conditionally set GDK_BACKEND=wayland when compositor is active,
  overriding AppImage GTK plugin's hardcoded x11 default
- Disable binary stripping in Linux AppImage build

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Migrate legacy raw 32-byte DEK to hex encoding on read instead of
  discarding and regenerating, preserving existing encrypted data
- Wrap hex-encoded DEK strings and decode buffers in Zeroizing
- Only set GDK_BACKEND=wayland when no backend is explicitly configured,
  respecting user preference over the AppImage hardcoded default

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Detect AppImage via APPDIR env var and override its hardcoded
  GDK_BACKEND=x11 when a Wayland compositor is available, while
  still respecting explicit user-set GDK_BACKEND values.
- Wrap the raw secret Vec<u8> in Zeroizing before copy_from_slice
  in the legacy binary DEK migration path to avoid leaving the
  plaintext key in unzeroed memory.

Ref: QingJ01#22 (comment)
- Replace hex::encode(&*key) with hex::encode(&key[..]) to silence
  clippy::needless-borrows-for-generic-args on Zeroizing<[u8; 32]>.
- Warn instead of silently ignoring set_secret failure in the legacy
  raw DEK migration path so broken writes are visible in logs.
- Use GDK_BACKEND=wayland,x11 instead of pure wayland to preserve
  the X11 fallback when preferring Wayland on Linux.
- Fix pre-existing clippy dead_code and needless_return warnings in
  the pebble crate that would fail CI.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Mooling0602 Mooling0602 force-pushed the fix/dek-hex-encoding branch from 0122229 to 1b5e9a8 Compare May 18, 2026 01:30
@Mooling0602
Copy link
Copy Markdown
Author

有一个模块在修复时和主线发生了冲突,经过判断使用主线版本更为合适,故进行了调整并在rebase后重新强制推送(只影响修复分支)。

@Mooling0602
Copy link
Copy Markdown
Author

审核了当前 PR 头提交 3a6c6f6,建议先处理下面两个点后再合并。

  1. src-tauri/src/lib.rs 的 Wayland 逻辑没有真正覆盖 AppImage 场景。现在只有在 GDK_BACKEND 未设置时才会设置为 wayland,但注释里也说明 AppImage GTK 插件会在 AppRun hook 里预先硬编码 GDK_BACKEND=x11。这意味着实际 AppImage 启动时变量已经存在,即使 WAYLAND_DISPLAY 可用,也不会切到 Wayland,PR 原本要修的 AppImage x11 默认值问题仍然存在。建议明确识别并覆盖 AppImage 注入的默认 x11,或者把 PR 目标改成“不覆盖任何已有用户/环境设置”,但现在代码和注释表达的意图不一致。
  2. crates/pebble-crypto/src/keystore.rs 的 legacy raw DEK 分支需要继续 zeroize 读出的原始 secret。当前 entry.get_secret() 返回的 Vec<u8> 在复制到 Zeroizing<[u8; 32]> 后普通 drop,旧实现会把这份 buffer 包在 Zeroizing 里。建议在 Ok(secret) if secret.len() == DEK_LEN 分支里先 let secret = Zeroizing::new(secret);,再 key.copy_from_slice(&secret),避免迁移路径把 raw DEK 留在未清零内存中。

我本地验证了:

  • git merge-tree origin/master origin/pr/22 无冲突
  • cargo fmt --check 通过
  • cargo test -p pebble-crypto 通过
  • cargo check -p pebble 通过

修复已基本完成,可以继续审查。

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.

2 participants