Merge lp:~cjwatson/lazr.sshserver/more-key-types into lp:lazr.sshserver

Proposed by Colin Watson on 2018-06-27
Status: Merged
Merged at revision: 78
Proposed branch: lp:~cjwatson/lazr.sshserver/more-key-types
Merge into: lp:lazr.sshserver
Diff against target: 187 lines (+61/-17)
3 files modified
src/lazr/sshserver/NEWS.txt (+8/-0)
src/lazr/sshserver/auth.py (+4/-0)
src/lazr/sshserver/tests/test_auth.py (+49/-17)
To merge this branch: bzr merge lp:~cjwatson/lazr.sshserver/more-key-types
Reviewer Review Type Date Requested Status
William Grant code 2018-06-27 Approve on 2018-07-02
Review via email: mp+348608@code.launchpad.net

Commit message

Add support-in-principle for authenticating using ECDSA or Ed25519 keys.

Description of the change

This also requires a sufficient version of Twisted (which at the time of writing supports ECDSA but not Ed25519) and support in the Launchpad authserver.

I've confirmed this to work locally with a modified version of Launchpad. Before pushing the Launchpad changes, we need to land these lazr.sshserver changes and upgrade its various users, since otherwise it would be rather confusing that you can add ECDSA keys but not authenticate using them.

Adding Ed25519 here is anticipatory, but it's harmless and will save us a step once Twisted gains Ed25519 support.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)
Colin Watson (cjwatson) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/sshserver/NEWS.txt'
2--- src/lazr/sshserver/NEWS.txt 2018-02-26 11:57:53 +0000
3+++ src/lazr/sshserver/NEWS.txt 2018-06-27 13:56:12 +0000
4@@ -2,6 +2,14 @@
5 NEWS for lazr.sshserver
6 =======================
7
8+0.1.8
9+=====
10+
11+- Add support-in-principle for authenticating using ECDSA or Ed25519 keys;
12+ this also requires a sufficient version of Twisted (which at the time of
13+ writing supports ECDSA but not Ed25519) and support in the Launchpad
14+ authserver.
15+
16 0.1.7 (2018-02-26)
17 ==================
18
19
20=== modified file 'src/lazr/sshserver/auth.py'
21--- src/lazr/sshserver/auth.py 2018-02-26 11:49:59 +0000
22+++ src/lazr/sshserver/auth.py 2018-06-27 13:56:12 +0000
23@@ -314,6 +314,10 @@
24 wantKeyType = 'DSA'
25 elif credentials.algName == 'ssh-rsa':
26 wantKeyType = 'RSA'
27+ elif credentials.algName.startswith('ecdsa-sha2-'):
28+ wantKeyType = 'ECDSA'
29+ elif credentials.algName == 'ssh-ed25519':
30+ wantKeyType = 'ED25519'
31 else:
32 # unknown key type
33 return False
34
35=== modified file 'src/lazr/sshserver/tests/test_auth.py'
36--- src/lazr/sshserver/tests/test_auth.py 2018-02-26 11:49:59 +0000
37+++ src/lazr/sshserver/tests/test_auth.py 2018-06-27 13:56:12 +0000
38@@ -416,7 +416,10 @@
39
40 valid_user = 'valid_user'
41 no_key_user = 'no_key_user'
42- valid_key = 'valid_key'
43+ valid_key_rsa = 'valid_key_rsa'
44+ valid_key_dsa = 'valid_key_dsa'
45+ valid_key_ecdsa = 'valid_key_ecdsa'
46+ valid_key_ed25519 = 'valid_key_ed25519'
47
48 def __init__(self):
49 self.calls = []
50@@ -430,7 +433,12 @@
51 if username == self.valid_user:
52 return defer.succeed({
53 'name': username,
54- 'keys': [('DSA', self.valid_key.encode('base64'))],
55+ 'keys': [
56+ ('RSA', self.valid_key_rsa.encode('base64')),
57+ ('DSA', self.valid_key_dsa.encode('base64')),
58+ ('ECDSA', self.valid_key_ecdsa.encode('base64')),
59+ ('ED25519', self.valid_key_ed25519.encode('base64')),
60+ ],
61 })
62 elif username == self.no_key_user:
63 return defer.succeed({
64@@ -443,11 +451,11 @@
65 except auth.NoSuchPersonWithName:
66 return defer.fail()
67
68- def makeCredentials(self, username, public_key, mind=None):
69+ def makeCredentials(self, username, key_type, public_key, mind=None):
70 if mind is None:
71 mind = auth.UserDetailsMind()
72 return auth.SSHPrivateKeyWithMind(
73- username, 'ssh-dss', public_key, '', None, mind)
74+ username, key_type, public_key, '', None, mind)
75
76 def makeChecker(self, do_signature_checking=False):
77 """Construct a PublicKeyFromLaunchpadChecker.
78@@ -469,21 +477,29 @@
79 super(TestPublicKeyFromLaunchpadChecker, self).setUp()
80 self.authserver = self.FakeAuthenticationEndpoint()
81
82+ @defer.inlineCallbacks
83 def test_successful(self):
84 # Attempting to log in with a username and key known to the
85 # authentication end-point succeeds.
86- creds = self.makeCredentials(
87- self.authserver.valid_user, self.authserver.valid_key)
88- checker = self.makeChecker()
89- d = checker.requestAvatarId(creds)
90- return d.addCallback(self.assertEqual, self.authserver.valid_user)
91+ for key_type, public_key in (
92+ ('ssh-rsa', self.authserver.valid_key_rsa),
93+ ('ssh-dss', self.authserver.valid_key_dsa),
94+ ('ecdsa-sha2-nistp256', self.authserver.valid_key_ecdsa),
95+ ('ssh-ed25519', self.authserver.valid_key_ed25519),
96+ ):
97+ creds = self.makeCredentials(
98+ self.authserver.valid_user, key_type, public_key)
99+ checker = self.makeChecker()
100+ username = yield checker.requestAvatarId(creds)
101+ self.assertEqual(self.authserver.valid_user, username)
102
103 @suppress_stderr
104 def test_invalid_signature(self):
105 # The checker requests attempts to authenticate if the requests have
106 # an invalid signature.
107 creds = self.makeCredentials(
108- self.authserver.valid_user, self.authserver.valid_key)
109+ self.authserver.valid_user, 'ssh-dss',
110+ self.authserver.valid_key_dsa)
111 creds.signature = 'a'
112 checker = self.makeChecker(True)
113 d = checker.requestAvatarId(creds)
114@@ -519,7 +535,7 @@
115 # Launchpad user names is public.
116 checker = self.makeChecker()
117 creds = self.makeCredentials(
118- 'no-such-user', self.authserver.valid_key)
119+ 'no-such-user', 'ssh-dss', self.authserver.valid_key_dsa)
120 return self.assertLoginError(
121 checker, creds, 'No such Launchpad account: no-such-user')
122
123@@ -528,7 +544,8 @@
124 # server informs you that the account has no keys.
125 checker = self.makeChecker()
126 creds = self.makeCredentials(
127- self.authserver.no_key_user, self.authserver.valid_key)
128+ self.authserver.no_key_user, 'ssh-dss',
129+ self.authserver.valid_key_dsa)
130 return self.assertLoginError(
131 checker, creds,
132 "Launchpad user %r doesn't have a registered SSH key"
133@@ -540,7 +557,7 @@
134 # tries several keys as part of normal operation.
135 checker = self.makeChecker()
136 creds = self.makeCredentials(
137- self.authserver.valid_user, 'invalid key')
138+ self.authserver.valid_user, 'ssh-dss', 'invalid key')
139 # We cannot use assertLoginError because we are checking that we fail
140 # with UnauthorizedLogin and not its subclass
141 # UserDisplayedUnauthorizedLogin.
142@@ -555,6 +572,20 @@
143 return d
144
145 @defer.inlineCallbacks
146+ def test_unknownKeyType(self):
147+ # Authenticating using a key with an unknown type fails.
148+ creds = self.makeCredentials(
149+ self.authserver.valid_user, 'nonsense',
150+ self.authserver.valid_key_rsa)
151+ checker = self.makeChecker()
152+ exception = yield assert_fails_with(
153+ checker.requestAvatarId(creds),
154+ UnauthorizedLogin)
155+ self.assertFalse(
156+ isinstance(exception, auth.UserDisplayedUnauthorizedLogin),
157+ "Should not be a UserDisplayedUnauthorizedLogin")
158+
159+ @defer.inlineCallbacks
160 def test_successful_with_second_key_calls_authserver_once(self):
161 # It is normal in SSH authentication to be presented with a number of
162 # keys. When the valid key is presented after some invalid ones (a)
163@@ -563,9 +594,10 @@
164 checker = self.makeChecker()
165 mind = auth.UserDetailsMind()
166 wrong_key_creds = self.makeCredentials(
167- self.authserver.valid_user, 'invalid key', mind)
168+ self.authserver.valid_user, 'ssh-dss', 'invalid key', mind)
169 right_key_creds = self.makeCredentials(
170- self.authserver.valid_user, self.authserver.valid_key, mind)
171+ self.authserver.valid_user, 'ssh-dss',
172+ self.authserver.valid_key_dsa, mind)
173 try:
174 username = yield checker.requestAvatarId(wrong_key_creds)
175 except UnauthorizedLogin:
176@@ -579,9 +611,9 @@
177 checker = self.makeChecker()
178 mind = auth.UserDetailsMind()
179 creds_1 = self.makeCredentials(
180- 'invalid-user', 'invalid key 1', mind)
181+ 'invalid-user', 'ssh-dss', 'invalid key 1', mind)
182 creds_2 = self.makeCredentials(
183- 'invalid-user', 'invalid key 2', mind)
184+ 'invalid-user', 'ssh-dss', 'invalid key 2', mind)
185 d = checker.requestAvatarId(creds_1)
186
187 def try_second_key(failure):

Subscribers

People subscribed via source and target branches

to all changes: