Skip to content

fix(gcm): reset AEAD packet/nonce counters during key exchanges (#165) - #14

Merged
GT-610 merged 1 commit into
masterfrom
fix/gcm-nonce-reset-1193
Jun 30, 2026
Merged

fix(gcm): reset AEAD packet/nonce counters during key exchanges (#165)#14
GT-610 merged 1 commit into
masterfrom
fix/gcm-nonce-reset-1193

Conversation

@GT-610

@GT-610 GT-610 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Re-applies upstream vicajilau#166 that was lost during the local 'Fix AES-GCM padding' commit (4518047).

AES-GCM AEAD ciphers must use a dedicated invocation counter that restarts at 0 on every NEWKEYS / key exchange, rather than the transport packet sequence number (_localPacketSN/_remotePacketSN) which keeps growing across rekeys. Using the SN as the GCM nonce causes 'message authentication code incorrect' failures against OpenSSH servers configured with aes256-gcm@openssh.com / aes128-gcm@openssh.com.

Changes:

  • Add _localAeadPacketCount / _remoteAeadPacketCount fields.
  • Use them as the AEAD sequence (with post-increment) in _sendAeadPacket and _consumeAeadPacket.
  • Reset both counters to 0 in _applyLocalKeys / _applyRemoteKeys when an AEAD cipher is selected.

Refs: lollipopkit/flutter_server_box#1193

Summary by CodeRabbit

  • Bug Fixes
    • 修复了 SSH 连接在使用 AEAD 加密方式(如 GCM、ChaCha20-Poly1305)时的包序号处理问题。
    • 在密钥更新后,AEAD 方向计数会正确重置,减少因序号不同步导致的传输异常或连接失败。

…jilau#165)

Re-applies upstream vicajilau#166 that was lost during
the local 'Fix AES-GCM padding' commit (4518047).

AES-GCM AEAD ciphers must use a dedicated invocation counter that
restarts at 0 on every NEWKEYS / key exchange, rather than the
transport packet sequence number (_localPacketSN/_remotePacketSN)
which keeps growing across rekeys. Using the SN as the GCM nonce
causes 'message authentication code incorrect' failures against
OpenSSH servers configured with aes256-gcm@openssh.com /
aes128-gcm@openssh.com.

Changes:
- Add _localAeadPacketCount / _remoteAeadPacketCount fields.
- Use them as the AEAD sequence (with post-increment) in
  _sendAeadPacket and _consumeAeadPacket.
- Reset both counters to 0 in _applyLocalKeys / _applyRemoteKeys
  when an AEAD cipher is selected.

Refs: lollipopkit/flutter_server_box#1193
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

概览

SSHTransport 中为 AEAD 加密路径(GCM / ChaCha20-Poly1305)新增独立的本地/远端数据包计数器 _localAeadPacketCount_remoteAeadPacketCount,替换原先复用的通用序号,并在每次密钥轮换时将其重置为 0。

变更

AEAD 独立计数器

Layer / File(s) 摘要
字段声明与收发计数器使用
lib/src/ssh_transport.dart
新增 _localAeadPacketCount_remoteAeadPacketCount 两个字段(初始值 0);_sendAeadPacket 传入 _localAeadPacketCount++_consumeAeadPacket 传入 _remoteAeadPacketCount++,使 AEAD nonce 计数与通用 packet SN 解耦。
密钥应用时重置计数器
lib/src/ssh_transport.dart
_applyLocalKeys_applyRemoteKeys 的 AEAD 分支中分别将对应计数器归零,确保每次密钥轮换后 AEAD sequence 从 0 重新开始。

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/src/ssh_transport.dart`:
- Around line 1039-1043: The rekey reset logic in the AEAD path only clears
`_localAeadPacketCount` when that branch is reached, but the server-side
`NEWKEYS` flow does not go through `_handleMessageKexReply()`. Update the server
rekey handling so `_localAeadPacketCount` is reset whenever new keys are
activated, using the same reset point as the client path in `ssh_transport.dart`
to keep AEAD nonce sequencing aligned after rekey.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d831aa14-594f-4843-a9eb-c5f0145fe9aa

📥 Commits

Reviewing files that changed from the base of the PR and between aad680b and a5b1cda.

📒 Files selected for processing (1)
  • lib/src/ssh_transport.dart
📜 Review details
🔇 Additional comments (4)
lib/src/ssh_transport.dart (4)

213-216: LGTM!


376-383: LGTM!


747-754: LGTM!


1109-1113: LGTM!

Comment thread lib/src/ssh_transport.dart
@GT-610
GT-610 merged commit acd59da into master Jun 30, 2026
2 checks passed
@GT-610
GT-610 deleted the fix/gcm-nonce-reset-1193 branch June 30, 2026 10:35
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.

1 participant