Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerability check incorrect? #35

Open
uli42 opened this issue Apr 3, 2024 · 2 comments
Open

Vulnerability check incorrect? #35

uli42 opened this issue Apr 3, 2024 · 2 comments

Comments

@uli42
Copy link

uli42 commented Apr 3, 2024

The scanner decides in these lines

return (report.SupportsChaCha20 || report.SupportsCbcEtm) && !report.SupportsStrictKex

So for a system to be reported as vulnerable it isrequired to fulfill both these conditions:

  • chacha20 support is active or cbc-etm is active
  • strict kex is not implemented.

According to https://jfrog.com/blog/ssh-protocol-flaw-terrapin-attack-cve-2023-48795-all-you-need-to-know/#cve-2023-48795-overview chacha20 is sufficient for a system to be vulnerable:

There are two vulnerable OpenSSH configurations:
- ChaCha20-Poly1305
- Any aes(128|192|256)-cbc ciphers using the default MACs (or any MAC that uses Encrypt-then-MAC, EtM, for example – [email protected]).

From my understanding If either side does not support strict kex the connection falls back to the the previous, vulnerable, behaviour. That means it is vulnerable if chacha20 is enabled. As we cannot guarantee that strict kex will be used the scanner should report vulnerability in that case.

@TrueSkrillor
Copy link
Contributor

From my understanding If either side does not support strict kex the connection falls back to the the previous, vulnerable, behaviour.

This is correct. The reason for this is that strict-kex introduces backward-incompatible changes to the SSH protocol which would cause connections to terminate if supported by one side only.

That means it is vulnerable if chacha20 is enabled. As we cannot guarantee that strict kex will be used the scanner should report vulnerability in that case.

Strictly speaking, yes. However, we decided not to because this would not be helpful at all and may confuse users that have updated to a client or server version that supports strict-kex. We also assume that a significant portion of users will have access to both client and server to ensure strict-kex support on both sides. Disabling ChaCha20-Poly1305 and Encrypt-then-MAC cipher modes when Terrapin mitigations are supported is not recommended, given that other modes (namely CBC-EaM) suffer from more significant weaknesses. Instead, we are trying to convey this limitation by the choice of words in the scanning summary:

Terrapin-Scanner/main.go

Lines 73 to 75 in e50b408

fmt.Println("The scanned peer supports Terrapin mitigations and can establish")
fmt.Println("connections that are NOT VULNERABLE to Terrapin. Glad to see this.")
fmt.Println("For strict key exchange to take effect, both peers must support it.")

Note that the Terrapin mitigation cannot be downgraded by a man-in-the-middle, so strict-kex is being enforced whenever the client and server support it.

@uli42
Copy link
Author

uli42 commented Apr 8, 2024

Thanks for the answer. However, this does not convice me...

In Enterprises you often control only a part of the network, but have to interact with clients and/or servers outof your control.

So for your own servers to be secure you will have take care of the mitigations no matter what. It would be better for the test to emphasize this more. Maybe add a switch that enforces a stricter checking result.

Also, people tend to check many servers. So having to carefully read the text is not efficient and can lead to people living with a false sense of security.

Regarding the weak remaining ciphers: this is surely a problem. So maybe it should be emphasized in the check result, too.

I think the better solution woud be openssh adding another fix that enables admins to configure a "strict kex or nothing" policy.

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

No branches or pull requests

2 participants