michal/ssh/add-packet-alignment-validation/OTP-20137

[Mikaka27]

Besides implementing packet alignment validation check from RFC 4253 6, so check:

Note that the length of the concatenation of 'packet_length', 'padding_length', 'payload', and 'random padding' MUST be a multiple of the cipher block size or 8, whichever is larger.

This now also implements fix for OpenSSH CVE-2008-5161, so packet discard functionality.

[github-actions]

CT Test Results

    2 files     29 suites   29m 41s ⏱️
  515 tests   508 ✅  7 💤 0 ❌
1 736 runs  1 707 ✅ 29 💤 0 ❌

Results for commit 328c280.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• No Windows Installer found

// Erlang/OTP Github Action Bot

[u3s]

let's check stuff we discussed

[Mikaka27]

I will take a look at 1 more thing. Currently this repurposed the packet_length_too_short and packet_length_too_large tests as the tests which detect incorrect padding, but previously they were followed by another packet and detected the disconnect for some other reason (need to check what was failing exactly).
I think we can restore the old tests if we make the packet longer/shorter by max(8, BlockSize) instead of 4 and add a dedicated test that will create a packet with invalid padding amount, but it's length would be real (just incorrect).

[Mikaka27]

I will take a look at 1 more thing. Currently this repurposed the packet_length_too_short and packet_length_too_large tests as the tests which detect incorrect padding, but previously they were followed by another packet and detected the disconnect for some other reason (need to check what was failing exactly). I think we can restore the old tests if we make the packet longer/shorter by max(8, BlockSize) instead of 4 and add a dedicated test that will create a packet with invalid padding amount, but it's length would be real (just incorrect).

I don't think there is a point in doing this. The old code failed with "bad_mac" error, so this is covered by new test.