None cipher does not work in recent versions

[shyblower]

The most recent version I could use the NoneSwitch with is tagged with hpn-6_8_P1. In more recent releases the server side hangs up when trying to connect using NoneEnabled and NoneSwitch. Tested on ArchLinux/ARM where ssh -v gives me OpenSSH_6.8p1-hpn14v6. It seems to depend on the hpn patch version since on my Gentoo/Intel box I can go up to 6.9 where they use the hpn patch version hpn14v5 in their openssh ebuild (OpenSSH_6.9p1-hpn14v5). Your hpn-6_9_P1 tag uses hpn14v7 which does not work.

[rapier1]

It looks like a change (to streamline some logic) in sshd.c do_ssh2_kex introduced in version 6.9 is preventing the none cipher from being in the approved cipher list. I'm going to try reverting those changes. If that works then I'll propagate them through the various versions. Which is quite a lot of fun :\

[rapier1]

I was able to recreate and fix the problem in 7.0 and 7.1. However, I was not able to recreate the problem in hpn-6_9_P1. Could you verify that the sshd-config file has NoneEnabled=yes? If it is then could you do me a favor and send me the first 18 lines of the do_ssh2_kex function in sshd.c? It should look like the following. In any case, I was able to build a functional 6.9 server with None support off of the current git.

do_ssh2_kex(void)
{
        char *myproposal[PROPOSAL_MAX] = { KEX_SERVER };
        struct kex *kex;
        int r;

        if (options.ciphers != NULL) {
                myproposal[PROPOSAL_ENC_ALGS_CTOS] =
                myproposal[PROPOSAL_ENC_ALGS_STOC] = options.ciphers;
        } else if (options.none_enabled == 1) {
                debug ("WARNING: None cipher enabled");
                myproposal[PROPOSAL_ENC_ALGS_CTOS] =
                    myproposal[PROPOSAL_ENC_ALGS_STOC] = KEX_ENCRYPT_INCLUDE_NONE;
        }
        myproposal[PROPOSAL_ENC_ALGS_CTOS] =
            compat_cipher_proposal(myproposal[PROPOSAL_ENC_ALGS_CTOS]);
        myproposal[PROPOSAL_ENC_ALGS_STOC] =
            compat_cipher_proposal(myproposal[PROPOSAL_ENC_ALGS_STOC]);

[shyblower]

Thanks for your effort. Never mind the 6.9 issue, I probably did something wrong during my tests and it wasn't 6.9 I was connecting to. I just tried it again and saw that 6.9 from your repo is perfectly functional and also the do_ssh2_kex function snippet is identical to what you posted here.

[rapier1]

Not a problem at all. Especially because it did uncover the same problem in the 7.0 and 7.1 lines. Hurray for happy coincidences.

[bdrewery]

This method is also removing the Ciphers decisions by the user by replacing the proposal with the defaults. I think this would be better:

(Note I have my own define here which won't match what you're using)

sshd.c:

@@ -1693,6 +1709,15 @@ main(int ac, char **av)
        /* Fill in default values for those options not explicitly set. */
        fill_default_server_options(&options);
+#ifdef NONE_CIPHER_ENABLED
+       if (options.none_enabled == 1) {
+               char *old_ciphers = options.ciphers;
+
+               xasprintf(&options.ciphers, "%s,none", old_ciphers);
+               free(old_ciphers);
+       }
+#endif
+
        /* challenge-response is implemented via keyboard interactive */
        if (options.challenge_response_authentication)
                options.kbd_interactive_authentication = 1;
@@ -2539,6 +2569,11 @@ do_ssh2_kex(void)
        struct kex *kex;
        int r;
+#ifdef NONE_CIPHER_ENABLED
+        if (options.none_enabled == 1)
+                debug ("WARNING: None cipher enabled");
+#endif
+
        myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(
            options.kex_algorithms);
        myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal(

This allows not having to reindent or mess with the upstream code as well. It also allows removing the KEX_ENCRYPT_INCLUDE_NONE define from myproposal.h.

Here I show it won't have none until I've edited the config. I haven't done a remote test of it though but given it is in the dumped config it should be fine.

root@exp-10amd64-commit-test:~ # /usr/local/sbin/sshd -T|grep cipher
ciphers chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com
root@exp-10amd64-commit-test:~ # vi /usr/local/etc/ssh/sshd_config
root@exp-10amd64-commit-test:~ # /usr/local/sbin/sshd -T | grep cipher
ciphers chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com,none

[vapier]

if options.cipher is NULL, that code may crash

[bdrewery]

Good point, easy enough to fix.

[bdrewery]

Actually it appears that fill_default_server_options will always populate it with KEX_SERVER_ENCRYPT via kex_assemble_names(KEX_SERVER_ENCRYPT, &options->ciphers) which does a strdup(KEX_SERVER_ENCRYPT)

[rapier1]

They dropped the check for option.ciphers=NULL in 7.0. That is still a valid concern in 6.9. So, as for losing the rest of the ciphers when none is enabled. If I remember correctly (as this was written many years ago now) we did this in order to ensure that none was the only possible method if the user requested it. I believe we initially had some problems where if we offered other methods it wouldn't necessarily use none.

[rapier1]

I tried out your suggested changes and they seem to work as advertised. I initially had some concerns about when none would be offered by the server preauth but that seems to be the same behaviour I was offering. I might look at that again as even with the safeguards in place I'm thinking that might be a mistake on my part.