Release 2.0.6.1 (beta) 2021 - SFTP failure (Synology)

warwickmm · May 11, 2021, 12:44am

I think that’s still possible, but debugging this way can in general be much more difficult. The releases are built in release mode where the compiler can optimized the code in a way where what you see in the source code doesn’t quite reflect what is being run. As such, you might find that you won’t be able to set a breakpoint on certain lines, methods can get inlined, etc.

Nice find @drwtsn32! Looks like with my setup I’m getting a ed25519 host key.

ts678 · May 11, 2021, 12:54am

Limit what algorithms or ciphers are used #730 might be a way out, if it can trim things down on mono.

warwickmm · May 11, 2021, 12:55am

Maybe it’s possible to configure the SSH.NET client to not advertise support for ecdsa host keys when running under mono?

ts678 · May 11, 2021, 12:56am

Look above your post.

drwtsn32 · May 11, 2021, 12:57am

Ah that makes sense… I wasn’t sure why I had a different result.

I don’t normally use Bitvise and after install it only had those two keys. ECDSA and the RSA one.

warwickmm · May 11, 2021, 1:04am

I didn’t see that post, but I had a tab open with that issue!

@drwtsn32, is it easy for you to try this patch (with your ecdsa key added back) to see if this approach might work?

diff --git a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs
index 61241914d..92c6508b7 100644
--- a/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs
+++ b/Duplicati/Library/Backend/SSHv2/SSHv2Backend.cs
@@ -328,6 +328,7 @@ namespace Duplicati.Library.Backend
             if (m_keepaliveinterval.Ticks != 0)
                 con.KeepAliveInterval = m_keepaliveinterval;
 
+            con.ConnectionInfo.HostKeyAlgorithms.Remove("ecdsa-sha2-nistp384");
             con.Connect();
 
             m_con = con;

drwtsn32 · May 11, 2021, 1:07am

Yes I’ll give it a try tonight and let you know!

warwickmm · May 11, 2021, 1:12am

BTW, here are where the host key strings are defined:

github.com

sshnet/SSH.NET/blob/master/src/Renci.SshNet/ConnectionInfo.cs#L382-L398


            HostKeyAlgorithms = new Dictionary<string, Func<byte[], KeyHostAlgorithm>>
                {
                    {"ssh-ed25519", data => new KeyHostAlgorithm("ssh-ed25519", new ED25519Key(), data)},
#if FEATURE_ECDSA
                    {"ecdsa-sha2-nistp256", data => new KeyHostAlgorithm("ecdsa-sha2-nistp256", new EcdsaKey(), data)},
                    {"ecdsa-sha2-nistp384", data => new KeyHostAlgorithm("ecdsa-sha2-nistp384", new EcdsaKey(), data)},
                    {"ecdsa-sha2-nistp521", data => new KeyHostAlgorithm("ecdsa-sha2-nistp521", new EcdsaKey(), data)},
#endif
                    {"ssh-rsa", data => new KeyHostAlgorithm("ssh-rsa", new RsaKey(), data)},
                    {"ssh-dss", data => new KeyHostAlgorithm("ssh-dss", new DsaKey(), data)},
                    //{"x509v3-sign-rsa", () => { ... },
                    //{"x509v3-sign-dss", () => { ... },
                    //{"spki-sign-rsa", () => { ... },
                    //{"spki-sign-dss", () => { ... },
                    //{"pgp-sign-rsa", () => { ... },
                    //{"pgp-sign-dss", () => { ... },
                };

Also, there is an issue with how SSH.NET implements this, where the order in which the keys are enumerated is not guaranteed. I bumped it in December, but it hasn’t gotten much attention.

github.com/sshnet/SSH.NET

Dictionary enumeration order is relied upon in ConnectionInfo despite being undefined behaviour

opened 10:17PM - 24 Aug 20 UTC

MattJamesChampion

In [`ConnectionInfo.cs`](https://github.com/sshnet/SSH.NET/blob/develop/src/Renc…i.SshNet/ConnectionInfo.cs) there are a number of attributes of type `IDictionary` that rely on the ordering that entities were added to the dictionary, even though the documentation for `Dictionary` states that [the order in which the items are returned is undefined](https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2#remarks). As a concrete example, take [`HostKeyAlgorithms`](https://github.com/sshnet/SSH.NET/blob/cefdc203d98cd890815e029bc759bc43ec5a9643/src/Renci.SshNet/ConnectionInfo.cs#L60). Modifying `HostKeyAlgorithms` as part of `ConnectionInfo` appears to be the method used to specify which algorithms are used and in what order. As an example, [reversing the order that the collection contents are added in](https://gist.github.com/MattJamesChampion/976a093739f47116bfd533209a640243) produces different host key fingerprints, which indicates that the order matters internally. As well as that, [the `Test_KeyExchangeInitMessage_Load` test uses `SequenceEqual` assertions to check the contents of `ServerHostKeyAlgorithms`](https://github.com/sshnet/SSH.NET/blob/7691cb0b55f5e0de8dc2ad48dd824419471ab710/src/Renci.SshNet.Tests/Classes/Messages/Transport/KeyExchangeInitMessageTest.cs#L35) which imply that the order is expected to be consistent. Since the order that `Dictionary` items are enumerated is undefined according to the documentation/standard, it's possible that a valid implementation could return values in a different order and therefore cause unexpected and inconsistent results. Using the above example, if a different host key algorithm is successfully negotiated first, the returned fingerprint would be different and could lead to the connection not being trusted (but only when the algorithms are enumerated in a certain order). The [`OrderedDictionary`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.specialized.ordereddictionary) class and the [`IOrderedDictionary`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.specialized.iordereddictionary) interface are potential replacements since they guarantee a consistent order, although they aren't drop-in replacements.

ts678 · May 11, 2021, 1:26am

2020.0.0 names the new key exchange (where mine was dying) and host key algorithms, but the file you cite also shows them, and its blame view shows when they were added. Knowing more about the mono omission might be helpful. Or is anything “elliptic” broken? That seems like a major gap in the algorithms.

The SSH.NET folks know the area far better than we do. Would it be worth trying to work with them, even using the existing open unsolved mystery issue perhaps. Oh. I see we’ve already started on that path.

Can’t connect (SCP, SSH) in Xamarin Android #807

EDIT:

Edwards-Curve Digital Signature Algorithm (EdDSA)
says ed25519 uses an Edwards curve, but that seems a variety of elliptic curve, so I’m looking for where
the gap in mono ends, so as to limit the amount of cutting (and probably security loss) that must be done.

warwickmm · May 11, 2021, 1:49am

Here’s the candidate NotImplementedException, which looks specific to ecdsa:

github.com

mono/mono/blob/a2833fdfc2b113bda9692aa445dfa4789a6cdc5a/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsa.cs#L31-L37


        public static new ECDsa Create() {
#if MONO
            throw new NotImplementedException ();
#else
            return Create(typeof(ECDsaCng).FullName);
#endif
        }

EDIT: This might not be the exact line. See below for a possibly more accurate explanation.

drwtsn32 · May 11, 2021, 2:51am

Yep, this worked in my test.

I was able to reproduce the problem without Bitvise on Windows by simply reconfiguring the sshd service on my Mint machine to have RSA + ECDSA keys. Tested before your fix and had the ‘not implemented’ exception. After the fix, no exception and Duplicati showed me the RSA key fingerprint.

So yeah I think this is a good fix when Duplicati is running on mono.

warwickmm · May 11, 2021, 3:38am

Thanks @drwtsn32 for testing. We should come up with something a little more robust in the actual code to defend against future changes to mono, but I think it would be pretty simple. I think in .NET 5, the responsibility is pushed to the operating system so future compatibility may be even more platform-dependent once we make that transition.

warwickmm · May 11, 2021, 4:32pm

Looking into this some more, I believe this might explain what we’ve discovered. This pull request added support for elliptic curve algorithms in SSH.NET. For ECDSA, it relies on the System.Security.Cryptography.ECDsaCng class

github.com

sshnet/SSH.NET/blob/a5bd08d655bb6a3c762306472cec354556dca3a3/src/Renci.SshNet/Security/Cryptography/EcdsaKey.cs#L357


            {
                bw.Write((int)curve_magic);
                bw.Write(cord_size);
                bw.Write(qx); // q.x
                bw.Write(qy); // q.y
                if (privatekey != null)
                    bw.Write(privatekey); // d
            }
            key = CngKey.Import(blob, privatekey == null ? CngKeyBlobFormat.EccPublicBlob : CngKeyBlobFormat.EccPrivateBlob);


            Ecdsa = new ECDsaCng(key);
#endif
        }


        private string GetCurveOid(string curve_s)
        {
            switch (curve_s.ToLower())
            {
                case "nistp256":
                    return ECDSA_P256_OID_VALUE;
                case "nistp384":

which is not implemented in Mono

github.com

mono/mono/blob/def4f3be68c44c3fb353fe46f00d540f4ed9bd15/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs#L29-L32


[SecuritySafeCritical]
public ECDsaCng(CngKey key) {
    throw new NotImplementedException ();
}

For ED25519, SSH.NET provides its own implementation (from Choas.NaCl).

warwickmm · May 11, 2021, 5:59pm

Here’s a draft of a potential fix. Testing is needed.

github.com/duplicati/duplicati

Avoid ECDSA algorithms when using SSH with Mono

duplicati:master ← warwickmm:mono_ssh_avoid_ecdsa_algorithms

opened 05:57PM - 11 May 21 UTC

warwickmm

+36 -3

SSH.NET [relies on](https://github.com/sshnet/SSH.NET/blob/a5bd08d655bb6a3c76230…6472cec354556dca3a3/src/Renci.SshNet/Security/Cryptography/EcdsaKey.cs#L357) the `System.Security.Cryptography.ECDsaCng` class for ECDSA algorithms, which is [not implemented](https://github.com/mono/mono/blob/a01309d1041ff8ae15b1f8b717420ec139e1cdb8/mcs/class/referencesource/System.Core/System/Security/Cryptography/ECDsaCng.cs#L29-L32) 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.

drwtsn32 · May 11, 2021, 6:21pm

Nice, I’ll try to test it out today or tomorrow. Thanks!

ts678 · May 13, 2021, 1:21am

Based on good earlier testing, I did some more research, and gave SSH.NET a theory of failure here, piggybacking on the existing GitHub issue. Theory still sounds good, but their confirm would be nice.

ts678 · May 13, 2021, 2:32pm

Can you test Control Panel → Server Management → Edit advanced settings → SSH algorithms → Signature, and see if unchecking all the ecdsa ones gets your Test connection and backup going?

@Rob_Canada can test similar if storage device is configurable. Some might need to know OpenSSH configuration, where I think @drwtsn32 knows more than I do (and doublecheck Bitvise above please).

ghysler · May 15, 2021, 10:59am

Sorry for the delay in my response. I already implemented the workaround provided by @drwtsn32 earlier (removing the ECDSA host key on the Bitvise SSH Server), and I can confirm this works (now defaults to RSA host key).

Great work on tackling the root cause everyone. I assume the merge request will take some time to make it to the beta, but the workaround works fine, of course. Thanks again

ts678 · May 29, 2021, 7:51pm

Release: 2.0.6.2 (experimental) 2021-05-29 is out with what is believed to be a workaround for the mono bug that broke SSH.NET that broke Duplicati. ECDSA host keys will not be used in mono until supported.

@Rob_Canada and @ghysler (and any others who saw this) are invited to remove workarounds from servers, and test it out. It should be safe to stay on, as it has only two changes from 2.0.6.1 Beta release, however you can set the Update channel back to Beta if you prefer to avoid future Experimental releases. Experimental is currently used as last step before Beta, so this code will probably be next Beta after test.

If you generally upgrade from Duplicati autoupgrader, you can go to Settings and change Update channel from Beta to Experimental, and it should offer this upgrade. If you prefer, installing manually will also work.

For best testing, please go to the Destination screen Advanced options, delete any ssh-fingerprint option, then use the Test connection button to accept a new one. Remember to go to final screen 5, to save it.

Rob_Canada · May 29, 2021, 8:45pm

I just updated to the 2.0.6.2 experimental version for the Synology, and it now works great!

Indeed, I again would like to thank everybody for such a great effort. I will stay on the Beta path but always look forward to the latest and greatest. On the rare occasion that something breaks, it’s fantastic to see such a vibrant community.