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

Implement Curve25519 (x25519) key exchange #1384

Closed
wants to merge 11 commits into from
Closed

Conversation

@alex
Copy link
Member

@alex alex commented Feb 9, 2019

This very very badly needs tests. But I don't know anything about paramiko! Can you suggest where the right place to add them is? (Ideally interop testing specifically, because this is the preferred kex, all the code is actually excersised quite a bit)

@alex
Copy link
Member Author

@alex alex commented Feb 9, 2019

Fixes #532

@alex
Copy link
Member Author

@alex alex commented Feb 9, 2019

It seems I've duplicated most of #1258. I think this code is slightly cleaner [citation needed], but that one has a test. I defer to the maintainers on how they'd like to handle this.

@michel-grundmann
Copy link

@michel-grundmann michel-grundmann commented Mar 15, 2019

Hi Alex, I am using paramiko but have security issues due to "no acceptable kex algorithm". Do you have an idea when Curve25519 support will be available in a paramiko release? Thanks, Michel

@bitprophet bitprophet added this to the p0 milestone May 31, 2019
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 7, 2019

Attempting to marry this to the tests from #1258 as I do find the code here a bit cleaner and with less loose ends (like the latter's strange while loop around keygen and some dangling vars).

Got that working, as in the tests now pass vs this patch & I can break them by breaking the code; however a real world integration test is failing and I'm digging into why. Putting the notes here because why not; it's probably still my fault, somehow, but so far I haven't proven that 😁

  • Test server is docker's python 3.7-stretch image with openssh-server installed, which ends up being OpenSSH 7.9 - plenty new enough as this kex was added in 7.4? 7.2? somewhere in there. (sob)
  • Ensured that eg ssh -o KexAlgorithms=curve25519-sha256 ... against the docker container functions correctly (including tweaking both ends to make absolute sure it is definitely using that and only that kex, because paranoid)
  • Got auth errors with my integration branch via Fabric 2 HEAD, though Fabric doesn't do much Paramiko level stuff which would impact this.
  • Went back to Paramiko master, and it works correctly in that scenario, so it's not some Fab 2 issue or recently-committed-to-Paramiko issue

Log from the server end makes it look like something low level is a bit off, but I haven't dug yet:

debug1: Forked child 35.                                                                                   
debug1: Set /proc/self/oom_score_adj to 0                                                                  
debug1: rexec start in 5 out 5 newsock 5 pipe 7 sock 8                                                     
rexec line 13: Deprecated option KeyRegenerationInterval                                                   
rexec line 25: Deprecated option RSAAuthentication                                                         
rexec line 26: Deprecated option RhostsRSAAuthentication                                                   
rexec line 27: Deprecated option ServerKeyBits                                                             
Could not load host key: /etc/ssh/ssh_host_dsa_key                                                         
debug1: inetd sockets after dupping: 3, 3                                                                  
Connection from 172.17.0.1 port 34694 on 172.17.0.2 port 22                                                
debug1: Client protocol version 2.0; client software version paramiko_2.4.2                                
debug1: no match: paramiko_2.4.2                                                                           
debug1: Local version string SSH-2.0-OpenSSH_7.4p1 Debian-10+deb9u6                                        
debug1: Enabling compatibility mode for protocol 2.0                                                       
debug1: permanently_set_uid: 102/65534 [preauth]                                                           
debug1: list_hostkey_types: ecdsa-sha2-nistp256,ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256 [preauth]    
debug1: SSH2_MSG_KEXINIT sent [preauth]                                                                    
debug1: SSH2_MSG_KEXINIT received [preauth]                                                                
debug1: kex: algorithm: curve25519-sha256@libssh.org [preauth]                                             
debug1: kex: host key algorithm: ssh-ed25519 [preauth]                                                     
debug1: kex: client->server cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]              
debug1: kex: server->client cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]              
debug1: expecting SSH2_MSG_KEX_ECDH_INIT [preauth]                                                         
debug1: rekey after 4294967296 blocks [preauth]                                                            
debug1: SSH2_MSG_NEWKEYS sent [preauth]                                                                    
debug1: expecting SSH2_MSG_NEWKEYS [preauth]                                                               
debug1: SSH2_MSG_NEWKEYS received [preauth]                                                                
debug1: rekey after 4294967296 blocks [preauth]                                                            
debug1: KEX done [preauth]                                                                                 
Bad packet length 435147897. [preauth]                                                                     
ssh_dispatch_run_fatal: Connection from 172.17.0.1 port 34694: message authentication code incorrect [preauth]
debug1: do_cleanup [preauth]                                                                               
debug1: monitor_read_log: child log fd closed                                                              
debug1: do_cleanup                                                                                         
debug1: Killing privsep child 36                                                                           
debug1: audit_event: unhandled event 12                                

Comparison to the successful run from a paramiko-master client:

debug1: Forked child 37.
debug1: Set /proc/self/oom_score_adj to 0
debug1: rexec start in 5 out 5 newsock 5 pipe 7 sock 8
rexec line 13: Deprecated option KeyRegenerationInterval
rexec line 25: Deprecated option RSAAuthentication
rexec line 26: Deprecated option RhostsRSAAuthentication
rexec line 27: Deprecated option ServerKeyBits
Could not load host key: /etc/ssh/ssh_host_dsa_key
debug1: inetd sockets after dupping: 3, 3
Connection from 172.17.0.1 port 34696 on 172.17.0.2 port 22
debug1: Client protocol version 2.0; client software version paramiko_2.4.2
debug1: no match: paramiko_2.4.2
debug1: Local version string SSH-2.0-OpenSSH_7.4p1 Debian-10+deb9u6
debug1: Enabling compatibility mode for protocol 2.0
debug1: permanently_set_uid: 102/65534 [preauth]
debug1: list_hostkey_types: ecdsa-sha2-nistp256,ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256 [preauth]
debug1: SSH2_MSG_KEXINIT sent [preauth]
debug1: SSH2_MSG_KEXINIT received [preauth]
debug1: kex: algorithm: ecdh-sha2-nistp256 [preauth]
debug1: kex: host key algorithm: ssh-ed25519 [preauth]
debug1: kex: client->server cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]
debug1: kex: server->client cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]
debug1: expecting SSH2_MSG_KEX_ECDH_INIT [preauth]
debug1: rekey after 4294967296 blocks [preauth]
debug1: SSH2_MSG_NEWKEYS sent [preauth]
debug1: expecting SSH2_MSG_NEWKEYS [preauth]
debug1: SSH2_MSG_NEWKEYS received [preauth]
debug1: rekey after 4294967296 blocks [preauth]
debug1: KEX done [preauth]
debug1: userauth-request for user root service ssh-connection method password [preauth]
debug1: attempt 0 failures 0 [preauth]
reprocess config line 25: Deprecated option RSAAuthentication
reprocess config line 26: Deprecated option RhostsRSAAuthentication
debug1: PAM: initializing for "root"
debug1: PAM: setting PAM_RHOST to "172.17.0.1"
debug1: PAM: setting PAM_TTY to "ssh"
debug1: PAM: password authentication accepted for root
debug1: do_pam_account: called
Accepted password for root from 172.17.0.1 port 34696 ssh2
debug1: monitor_child_preauth: root has been authenticated by privileged process
debug1: monitor_read_log: child log fd closed
debug1: PAM: establishing credentials
debug1: rekey after 4294967296 blocks
debug1: rekey after 4294967296 blocks
debug1: ssh_packet_set_postauth: called
debug1: Entering interactive session for SSH2.
debug1: server_init_dispatch
debug1: server_input_channel_open: ctype session rchan 0 win 2097152 max 32768
debug1: input_session_request
debug1: channel 0: new [server-session]
debug1: session_new: session 0
debug1: session_open: channel 0
debug1: session_open: session 0: link with channel 0
debug1: server_input_channel_open: confirm session
debug1: server_input_channel_req: channel 0 request exec reply 1
debug1: session_by_channel: session 0 channel 0
debug1: session_input_channel_req: session 0 req exec
Starting session: command for root from 172.17.0.1 port 34696 id 0
debug1: Received SIGCHLD.
debug1: session_by_pid: pid 43
debug1: session_exit_message: session 0 channel 0 pid 43
debug1: session_exit_message: release channel 0
debug1: session_by_channel: session 0 channel 0
debug1: session_close_by_channel: channel 0 child 0
Close session: user root from 172.17.0.1 port 34696 id 0
debug1: channel 0: free: server-session, nchannels 1
Connection closed by 172.17.0.1 port 34696
debug1: do_cleanup
debug1: PAM: cleanup
debug1: PAM: closing session
debug1: PAM: deleting credentials
Transferred: sent 2248, received 992 bytes
Closing connection to 172.17.0.1 port 34696

Paramiko's own logs don't help much here as it's simply whether or not we see userauth ok onwards, or not, in the below (success) snippet:

paramiko.transport.transport._log: Adding ssh-ed25519 host key for [localhost]:2222: b'1f03a3e0f3a5eff2f2b9faf7c9029be1'
paramiko.transport.transport._log: userauth is OK
paramiko.transport.transport._log: Authentication (password) successful!

But that isn't too surprising since A) Paramiko's logging/excepting is still horrible and B) the issue, as per the SSHD log, seems to be an issue with post-kex packets:

debug1: KEX done [preauth]                                                                                 
Bad packet length 435147897. [preauth]                                                                     
ssh_dispatch_run_fatal: Connection from 172.17.0.1 port 34694: message authentication code incorrect [preauth]

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 7, 2019

Interestingly, using the integration branch for #1258 succeeds, so there's some actual, actionable difference between the two that I missed (or my Frankensteining fucked something up).

Reverting to exactly Alex's copy of the code (with a single merge conflict in .travis.yml aside), i.e. 59cc28c, I get the same broken behavior, so it's presumably not my local changes. Will dig.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Read both through carefully and nothing is jumping out at me as a salient difference. The biggest actual difference approach-wise is in key generation (the while loop in the other patch) but I verified (both by asking Alex and doing my own empirical check) that the method in question is indeed always yielding 32 bits, so it's not that.

Otherwise, the only thing that should presumably matter in post-kex packet construction is what happens to Transport, which is mostly in this case generation/storage of K and especially H (which is itself not used directly in the init/reply handshake but is used afterwards). But that looks the same on both sides.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Here's a sshd log from the successful run of the other patch's code (I realized the snippet above isn't as useful for diffing as it, due to being on paramiko master, falls back to a different kex altogether):

debug1: Forked child 20.
debug1: Set /proc/self/oom_score_adj to 0
debug1: rexec start in 5 out 5 newsock 5 pipe 7 sock 8
rexec line 13: Deprecated option KeyRegenerationInterval
rexec line 25: Deprecated option RSAAuthentication
rexec line 26: Deprecated option RhostsRSAAuthentication
rexec line 27: Deprecated option ServerKeyBits
Could not load host key: /etc/ssh/ssh_host_dsa_key
debug1: inetd sockets after dupping: 3, 3
Connection from 172.17.0.1 port 34716 on 172.17.0.2 port 22
debug1: Client protocol version 2.0; client software version paramiko_2.4.2
debug1: no match: paramiko_2.4.2
debug1: Local version string SSH-2.0-OpenSSH_7.4p1 Debian-10+deb9u6
debug1: Enabling compatibility mode for protocol 2.0
debug1: permanently_set_uid: 102/65534 [preauth]
debug1: list_hostkey_types: ecdsa-sha2-nistp256,ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256 [preauth]
debug1: SSH2_MSG_KEXINIT sent [preauth]
debug1: SSH2_MSG_KEXINIT received [preauth]
debug1: kex: algorithm: curve25519-sha256@libssh.org [preauth]
debug1: kex: host key algorithm: ssh-ed25519 [preauth]
debug1: kex: client->server cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]
debug1: kex: server->client cipher: aes128-ctr MAC: hmac-sha2-256 compression: none [preauth]
debug1: expecting SSH2_MSG_KEX_ECDH_INIT [preauth]
debug1: rekey after 4294967296 blocks [preauth]
debug1: SSH2_MSG_NEWKEYS sent [preauth]
debug1: expecting SSH2_MSG_NEWKEYS [preauth]
debug1: SSH2_MSG_NEWKEYS received [preauth]
debug1: rekey after 4294967296 blocks [preauth]
debug1: KEX done [preauth]
debug1: userauth-request for user root service ssh-connection method password [preauth]
debug1: attempt 0 failures 0 [preauth]
reprocess config line 25: Deprecated option RSAAuthentication
reprocess config line 26: Deprecated option RhostsRSAAuthentication
debug1: PAM: initializing for "root"
debug1: PAM: setting PAM_RHOST to "172.17.0.1"
debug1: PAM: setting PAM_TTY to "ssh"
debug1: PAM: password authentication accepted for root
debug1: do_pam_account: called
Accepted password for root from 172.17.0.1 port 34716 ssh2
debug1: monitor_child_preauth: root has been authenticated by privileged process
debug1: monitor_read_log: child log fd closed
debug1: PAM: establishing credentials
debug1: rekey after 4294967296 blocks
debug1: rekey after 4294967296 blocks
debug1: ssh_packet_set_postauth: called
debug1: Entering interactive session for SSH2.
debug1: server_init_dispatch
debug1: server_input_channel_open: ctype session rchan 0 win 2097152 max 32768
debug1: input_session_request
debug1: channel 0: new [server-session]
debug1: session_new: session 0
debug1: session_open: channel 0
debug1: session_open: session 0: link with channel 0
debug1: server_input_channel_open: confirm session
debug1: server_input_channel_req: channel 0 request exec reply 1
debug1: session_by_channel: session 0 channel 0
debug1: session_input_channel_req: session 0 req exec
Starting session: command for root from 172.17.0.1 port 34716 id 0
debug1: Received SIGCHLD.
debug1: session_by_pid: pid 26
debug1: session_exit_message: session 0 channel 0 pid 26
debug1: session_exit_message: release channel 0
debug1: session_by_channel: session 0 channel 0
debug1: session_close_by_channel: channel 0 child 0
Close session: user root from 172.17.0.1 port 34716 id 0
debug1: channel 0: free: server-session, nchannels 1
Connection closed by 172.17.0.1 port 34716
debug1: do_cleanup
debug1: PAM: cleanup
debug1: PAM: closing session
debug1: PAM: deleting credentials
Transferred: sent 1936, received 984 bytes
Closing connection to 172.17.0.1 port 34716

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Vimdiff shows that the issue pops up on the userauth request (duh, it's the next packet sent):

debug1: KEX done [preauth]                                                                |  debug1: KEX done [preauth]                                                                
debug1: userauth-request for user root service ssh-connection method password [preauth]   |  Bad packet length 435147897. [preauth]                                                    
debug1: attempt 0 failures 0 [preauth]                                                    |  ssh_dispatch_run_fatal: Connection from 172.17.0.1 port 34694: message authentication code
reprocess config line 25: Deprecated option RSAAuthentication                             |  debug1: do_cleanup [preauth]                                                              
reprocess config line 26: Deprecated option RhostsRSAAuthentication                       |  debug1: monitor_read_log: child log fd closed                                             

Still not sure what exactly's afoot but going to try silly stuff like temporarily patching the key generation to spit out the static value from the test suite, which might help me figure out whether some math is getting strange at some point. Though that's only really going to be reproducible if I mate it to a Paramiko-driven server process. Hm.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Aha, I noticed something earlier in the logs and lost the plot for a while - was in Paramiko's logs, not the sshd's. It's coming out of the Packetizer during construction of any message, as expected, and has to do with H, also as I was guessing. #1258's code yields:

paramiko.transport.transport._log: kex engine KexCurve25519 specified hash_algo <built-in function openssl_sha256>

but this patch, despite appearing to set the metadata in Transport correctly, says this:

paramiko.transport.transport._log: kex engine KexCurve25519 specified hash_algo None, falling back to sha1

Well, that'd certainly do it. So, why? It's because there's an unpublished (on account of "Kex engines" not being an actual God damned class or API. PAST MAINTAINER!!!) interface that says these classes need to expose a .hash_algo class attribute. #1258 sets it; this doesn't. Looks just like a minor, inconsequential detail until you read the Packetizer code 😩

Sure enough, that like 3 line tweak fixes things

Now to find out why the tests don't test this. But it's Paramiko so probably because testing is a bit of a PITA.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Reason the tests don't catch this is that the packetizer's totally cut out, the fake transport in this test module doesn't invoke it. Something to tackle later - I think a more useful change would be to add a kex class API so future new kexes don't have to deal with this. Will see how quick I can do that. Then it's merge tiem.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

Dug at why that fallback exists; its most recent change is in #356 but that ticket was just centralizing some earlier hardcoded "we have always been at war with Eastasia hashed with SHA1" behavior.

Anyway I've reached my limit for this so I made #1452 for the followup and gonna merge this in a sec.

bitprophet added a commit that referenced this issue Jun 8, 2019
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2019

We've diverged slightly but it's ok. I got it! it's good. we're good. good night.

@bitprophet bitprophet closed this Jun 8, 2019
@alex alex deleted the kex-curve25519 branch Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants