From 3e2103335bb825dfa212c0a05361631eb56996e3 Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 11 May 2021 09:53:18 -0700 Subject: SSH: Avoid advertising support for ECDSA algorithms with Mono. SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). This prevents clients from connecting if one of the ECDSA algorithms is chosen as the host key algorithm. In the event that this causes a connection failure, we will prevent the client from advertising support for ECDSA algorithms and make another connection attempt. Related forum discussion: https://forum.duplicati.com/t/release-2-0-6-1-beta-2021-sftp-failure-synology/12358 --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 37 +++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 61241914d..1efb782d6 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -273,7 +273,7 @@ namespace Duplicati.Library.Backend if (m_con != null && !m_con.IsConnected) { - m_con.Connect(); + this.TryConnect(m_con); return; } @@ -328,11 +328,44 @@ namespace Duplicati.Library.Backend if (m_keepaliveinterval.Ticks != 0) con.KeepAliveInterval = m_keepaliveinterval; - con.Connect(); + this.TryConnect(con); m_con = con; } + private void TryConnect(SftpClient client) + { + try + { + client.Connect(); + } + catch (NotImplementedException) + { + // SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for + // ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). + // This prevents clients from connecting if one of the ECDSA algorithms is + // chosen as the host key algorithm. In the event that this causes a + // connection failure, we will prevent the client from advertising support + // for ECDSA algorithms and make another connection attempt. + // + // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs + if (Utility.Utility.IsMono) + { + IEnumerable ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")); + foreach (string key in ecdsaKeys) + { + client.ConnectionInfo.HostKeyAlgorithms.Remove(key); + } + + client.Connect(); + } + else + { + throw; + } + } + } + private void ChangeDirectory(string path) { if (string.IsNullOrEmpty(path)) -- cgit v1.2.3 From b80845a17652fe333145d90bc2509a732d02a517 Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 11 May 2021 15:38:17 -0700 Subject: Enumerate items to list before modifying collection. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 1efb782d6..2d0d1fa02 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -351,7 +351,7 @@ namespace Duplicati.Library.Backend // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs if (Utility.Utility.IsMono) { - IEnumerable ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")); + List ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")).ToList(); foreach (string key in ecdsaKeys) { client.ConnectionInfo.HostKeyAlgorithms.Remove(key); -- cgit v1.2.3 From 3d8817608e81bbb06e6fc58ac68323d7769e6043 Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Sun, 16 May 2021 17:29:00 -0700 Subject: Test for ECDSA support explicitly. The previous implementation could possibly fail for reasons unrelated to ECDSA support. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 26 +++++++++++-------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 2d0d1fa02..b19ac7fc4 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -28,6 +28,7 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; @@ -335,35 +336,30 @@ namespace Duplicati.Library.Backend private void TryConnect(SftpClient client) { - try - { - client.Connect(); - } - catch (NotImplementedException) + if (Utility.Utility.IsMono) { // SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for // ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). // This prevents clients from connecting if one of the ECDSA algorithms is - // chosen as the host key algorithm. In the event that this causes a - // connection failure, we will prevent the client from advertising support - // for ECDSA algorithms and make another connection attempt. + // chosen as the host key algorithm. In this case, we will prevent the + // client from advertising support for ECDSA algorithms. // // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs - if (Utility.Utility.IsMono) + try + { + ECDsa.Create(default(ECCurve)); + } + catch (NotImplementedException) { List ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")).ToList(); foreach (string key in ecdsaKeys) { client.ConnectionInfo.HostKeyAlgorithms.Remove(key); } - - client.Connect(); - } - else - { - throw; } } + + client.Connect(); } private void ChangeDirectory(string path) -- cgit v1.2.3 From 90dfce81b2371a643154f1b1f13be2d5e5cf874c Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 18 May 2021 09:00:40 -0700 Subject: Construct instance of ECDsaCng to test for ECDSA support. This more closely reflects the SSH.NET code. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index b19ac7fc4..1bbb75761 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -347,7 +347,7 @@ namespace Duplicati.Library.Backend // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs try { - ECDsa.Create(default(ECCurve)); + ECDsaCng unused = new ECDsaCng(); } catch (NotImplementedException) { -- cgit v1.2.3 From bfa240c8eca57c927c0779c2f27acf994a17714f Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 18 May 2021 09:06:00 -0700 Subject: Assume we have ECDSA support if our test throws exception. It's possible that our test will no longer be valid (e.g., due to changes in SSH.NET), so we don't want the backend to fail if our test is no longer valid. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 1bbb75761..3237d8a4e 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -357,6 +357,10 @@ namespace Duplicati.Library.Backend client.ConnectionInfo.HostKeyAlgorithms.Remove(key); } } + catch + { + // Ignore other exceptions and assume that we have ECDSA support. + } } client.Connect(); -- cgit v1.2.3 From 22564bb2a332e6455afd0ea66780044ffcbf035b Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 18 May 2021 09:17:13 -0700 Subject: Cache knowledge of ECDSA support. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 54 +++++++++++++++---------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 3237d8a4e..1fca6aad7 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -59,6 +59,8 @@ namespace Duplicati.Library.Backend private SftpClient m_con; + private readonly Lazy supportsECDSA = new Lazy(SSHv2.SupportsECDSA, true); + public SSHv2() { } @@ -334,32 +336,40 @@ namespace Duplicati.Library.Backend m_con = con; } + /// + /// SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for + /// ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). + /// This prevents clients from connecting if one of the ECDSA algorithms is + /// chosen as the host key algorithm. In this case, we will prevent the + /// client from advertising support for ECDSA algorithms. + /// + /// Mono ECDsaCng implementation. + /// Whether ECDSA algorithms are supported or not. + private static bool SupportsECDSA() + { + try + { + ECDsaCng unused = new ECDsaCng(); + return true; + } + catch (NotImplementedException) + { + return false; + } + catch + { + return true; + } + } + private void TryConnect(SftpClient client) { - if (Utility.Utility.IsMono) + if (!this.supportsECDSA.Value) { - // SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for - // ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). - // This prevents clients from connecting if one of the ECDSA algorithms is - // chosen as the host key algorithm. In this case, we will prevent the - // client from advertising support for ECDSA algorithms. - // - // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs - try - { - ECDsaCng unused = new ECDsaCng(); - } - catch (NotImplementedException) - { - List ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")).ToList(); - foreach (string key in ecdsaKeys) - { - client.ConnectionInfo.HostKeyAlgorithms.Remove(key); - } - } - catch + List ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")).ToList(); + foreach (string key in ecdsaKeys) { - // Ignore other exceptions and assume that we have ECDSA support. + client.ConnectionInfo.HostKeyAlgorithms.Remove(key); } } -- cgit v1.2.3 From c246976fbbd937164e2ef60a2c61e762b6fe075b Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 18 May 2021 10:18:14 -0700 Subject: Cache knowledge of ECDSA support using static field. This is simpler, and would potentially allow us to query this without an instance. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 54 ++++++++++++------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 1fca6aad7..2299d2cd5 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -59,7 +59,31 @@ namespace Duplicati.Library.Backend private SftpClient m_con; - private readonly Lazy supportsECDSA = new Lazy(SSHv2.SupportsECDSA, true); + private static readonly bool supportsECDSA; + + static SSHv2() + { + // SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for + // ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). + // This prevents clients from connecting if one of the ECDSA algorithms is + // chosen as the host key algorithm. In this case, we will prevent the + // client from advertising support for ECDSA algorithms. + // + // See https://github.com/mono/mono/blob/mono-6.12.0.144/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs. + try + { + ECDsaCng unused = new ECDsaCng(); + SSHv2.supportsECDSA = true; + } + catch (NotImplementedException) + { + SSHv2.supportsECDSA = false; + } + catch + { + SSHv2.supportsECDSA = true; + } + } public SSHv2() { @@ -336,35 +360,9 @@ namespace Duplicati.Library.Backend m_con = con; } - /// - /// SSH.NET relies on the System.Security.Cryptography.ECDsaCng class for - /// ECDSA algorithms, which is not implemented in Mono (as of 6.12.0.144). - /// This prevents clients from connecting if one of the ECDSA algorithms is - /// chosen as the host key algorithm. In this case, we will prevent the - /// client from advertising support for ECDSA algorithms. - /// - /// Mono ECDsaCng implementation. - /// Whether ECDSA algorithms are supported or not. - private static bool SupportsECDSA() - { - try - { - ECDsaCng unused = new ECDsaCng(); - return true; - } - catch (NotImplementedException) - { - return false; - } - catch - { - return true; - } - } - private void TryConnect(SftpClient client) { - if (!this.supportsECDSA.Value) + if (!SSHv2.supportsECDSA) { List ecdsaKeys = client.ConnectionInfo.HostKeyAlgorithms.Keys.Where(x => x.StartsWith("ecdsa")).ToList(); foreach (string key in ecdsaKeys) -- cgit v1.2.3 From a5fbee033536d54a937d7dbc7ff935efcbe10b7c Mon Sep 17 00:00:00 2001 From: Kenneth Hsu Date: Tue, 18 May 2021 12:18:23 -0700 Subject: Simplify exception handling when testing for ECDSA support. .NET 5 moves the responsibility to the operating system, and since SSH.NET currently relies on the ECDsaCng class, we will simply interpret all exceptions to mean lack of ECDSA support. --- Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs index 2299d2cd5..14bb5264a 100644 --- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs +++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs @@ -75,13 +75,9 @@ namespace Duplicati.Library.Backend ECDsaCng unused = new ECDsaCng(); SSHv2.supportsECDSA = true; } - catch (NotImplementedException) - { - SSHv2.supportsECDSA = false; - } catch { - SSHv2.supportsECDSA = true; + SSHv2.supportsECDSA = false; } } -- cgit v1.2.3