Ticket #5776 defect closed fixed

Opened 22 months ago

Last modified 22 months ago

twisted.conch.ssh.userauth depends on hash ordering

Reported by: antoine Owned by:
Priority: high Milestone:
Component: conch Keywords: review
Cc: z3p Branch:
Author: Launchpad Bug:

Description

Running the cftp script can fail with hash randomization:

$ python2.6 -R bin/conch/cftp -v pitrou.net
2012-07-13 18:58:43+0200 [-] Log opened.
2012-07-13 18:58:43+0200 [-] Starting factory <twisted.conch.client.direct.SSHClientFactory instance at 0x2118f80>
2012-07-13 18:58:43+0200 [SSHClientTransport,client] kex alg, key alg: diffie-hellman-group-exchange-sha1 ssh-rsa
2012-07-13 18:58:43+0200 [SSHClientTransport,client] outgoing: aes256-ctr hmac-sha1 none
2012-07-13 18:58:43+0200 [SSHClientTransport,client] incoming: aes256-ctr hmac-sha1 none
2012-07-13 18:58:43+0200 [SSHClientTransport,client] REVERSE
2012-07-13 18:58:43+0200 [SSHClientTransport,client] NEW KEYS
2012-07-13 18:58:43+0200 [SSHClientTransport,client] setting client server to <twisted.conch.client.default.SSHUserAuthClient instance at 0x2118ef0>
2012-07-13 18:58:43+0200 [SSHClientTransport,client] starting service ssh-userauth
2012-07-13 18:58:43+0200 [SSHClientTransport,client] using agent
2012-07-13 18:58:43+0200 [SSHAgentClient,client] got 3 public keys
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] can continue with: ['publickey', 'password']
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] trying to auth with publickey
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] using key of type RSA
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] can continue with: ['publickey', 'password']
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] trying to auth with publickey
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] using key of type DSA
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] can continue with: ['publickey', 'password']
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] trying to auth with publickey
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] using key of type DSA
2012-07-13 18:58:43+0200 [SSHService ssh-userauth on SSHClientTransport,client] couldn't handle 60
[snip]

At this point, the script blocks instead of connecting successfully. That's because in userauth.py, the messages table is initialized from the globals() dictionary, and there are duplicate message values. Message number 60 can be mapped to MSG_USERAUTH_PASSWD_CHANGEREQ, MSG_USERAUTH_INFO_REQUEST or MSG_USERAUTH_PK_OK depending on ordering. Without hash randomization, it is always mapped to MSG_USERAUTH_PK_OK.

Attachments

randomhash_userauth.patch Download (1.3 KB) - added by antoine 22 months ago.
randomhash_userauth2.patch Download (1.9 KB) - added by antoine 22 months ago.

Change History

1

  Changed 22 months ago by DefaultCC Plugin

  • cc z3p added

Changed 22 months ago by antoine

2

  Changed 22 months ago by antoine

  • keywords review added

Patch attached.

3

follow-up: ↓ 4   Changed 22 months ago by mwh

  • owner set to antoine
  • keywords review removed

Hi, thanks for fixing the problem and for adding the testcase. The behaviour around here is pretty horrible :/

Can I ask you to, instead of sorting the names, simply assign MSG_USERAUTH_PASSWD_CHANGEREQ and MSG_USERAUTH_INFO_REQUEST after the loop?

Also can you add a news file per  http://twistedmatrix.com/trac/wiki/ReviewProcess#Newsfiles ? I'll leave it up to you as to whether this is 'misc' or 'bugfix' :-)

4

in reply to: ↑ 3   Changed 22 months ago by antoine

Replying to mwh:

Can I ask you to, instead of sorting the names, simply assign MSG_USERAUTH_PASSWD_CHANGEREQ and MSG_USERAUTH_INFO_REQUEST after the loop?

That's a better idea indeed. I'm attaching a new patch.

Changed 22 months ago by antoine

5

  Changed 22 months ago by antoine

  • keywords review added

6

  Changed 22 months ago by antoine

  • owner antoine deleted

7

  Changed 22 months ago by mwh

  • status changed from new to closed
  • resolution set to fixed

(In [34749]) Merge randomhash_userauth2.patch: twisted.conch.ssh.userauth now works correctly with hash randomization enabled.

Author: antoine Reviewer: mwh Fixes: #5776

The userauth code depended on mapping message type 60 to MSG_USERAUTH_PK_OK but that only happened because of luck with dictionary ordering.

Note: See TracTickets for help on using tickets.