blinksh / blink

Blink Mobile Shell for iOS (Mosh based)
https://blink.sh
GNU General Public License v3.0
6.1k stars 566 forks source link

SSH may need to be updated to properly handle Terrapin attack #1918

Closed ChaiTRex closed 5 months ago

ChaiTRex commented 7 months ago

Very recently, a new attack on SSH called Terrapin was reported. ssh inside Blink Shell may need to be updated to a version that can properly handle the attack.

l2dy commented 6 months ago

https://github.com/blinksh/libssh/tree/LibSSHBlockRunLoop has not been updated for a year. It's referenced in https://github.com/blinksh/libssh-apple/blob/main/Sources/libssh-apple/main.swift along with an outdated version of OpenSSL (1.1.1k).

carloscabanero commented 6 months ago

Related to #1671. We will add this for 17.2.0

carloscabanero commented 6 months ago

https://github.com/blinksh/libssh/tree/LibSSHBlockRunLoop has not been updated for a year. It's referenced in https://github.com/blinksh/libssh-apple/blob/main/Sources/libssh-apple/main.swift along with an outdated version of OpenSSL (1.1.1k).

And...? (Sorry, message sent by mistake),

We usually update when we need to make changes from our side on the backend, or when the libraries have important updates or critical security issues to the client side (like this case). We have diverged in some ways (like for example sockets in case of lib ssh). We keep an eye so we don't fall behind, but as the library is just linked to Blink, we need to be very careful with pushing updates before we thoroughly test them (this has caused issues before).

All that being said, we definitely need to improve the cadence of these library updates.

l2dy commented 6 months ago

https://github.com/blinksh/libssh/tree/LibSSHBlockRunLoop has not been updated for a year. It's referenced in https://github.com/blinksh/libssh-apple/blob/main/Sources/libssh-apple/main.swift along with an outdated version of OpenSSL (1.1.1k).

And...?

I forgot to mention that I also checked the libssh commit history upstream and noticed that we are missing many commits referencing a CVE number including this one.

The information is provided so that if someone is super concerned about this vulnerability, they can go ahead and implement the fix themselves in a fork. libssh 0.9.8 also includes the fix for CVE-2023-48795, which should be an easier target to upgrade to than 0.10.

I mentioned OpenSSL because I think it's a good time to update it as well. It is out of context and I shouldn't have said it. Sorry about that!

We usually update when we need to make changes from our side on the backend, or when the libraries have important updates or critical security issues to the client side (like this case). We have diverged in some ways (like for example sockets in case of lib ssh). We keep an eye so we don't fall behind, but as the library is just linked to Blink, we need to be very careful with pushing updates before we thoroughly test them (this has caused issues before).

That's a sensible decision. However, it does make it more difficult for third parties to review whether any of the CVE fixes Blink missed are important or not.

All that being said, we definitely need to improve the cadence of these library updates.

That's the point. The more you fall behind, the harder it is to execute an upgrade confidently. I do agree with your choice of focusing on important fixes, but periodic sync with upstream is important as well.

carloscabanero commented 6 months ago

Apologies because my And...? Message out of context sounded super harsh. Your response is what I was looking for though, pointing out in case there was something else that we were missing. I will try to see what we need to keep up to date, or at least to block a version and apply necessary patches as we need them. Maybe only go with major versions when we move with major new versions ourselves as those are usually longer release cycles, etc...

Any ideas or help here is greatly appreciated.

carloscabanero commented 6 months ago

Updates on this as I worked on it this week. We are now back on track and this should be out on 17.2.0. Run all the battery of tests and we should be good. But, I will release on TestFlight first so all patches can be throughly tested. We will post-pone the release 17.2.0 to end of month so we can properly get feedback.

carloscabanero commented 5 months ago

Shipped with 17.2.0