launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-bug-971691 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #971691 in MAAS: "Incorrect padding exception raised adding an invalid ssh key"
  https://bugs.launchpad.net/maas/+bug/971691

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-bug-971691/+merge/100488

twisted.conch.ssh.keys.Key.fromString's doc says only BadKeyError can be raises but it's not true :(.  binascii.Error can also be raised.
-- 
https://code.launchpad.net/~rvb/maas/maas-bug-971691/+merge/100488
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-bug-971691 into lp:maas.

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-04-02 16:38:56 +0000
+++ src/maasserver/models.py	2012-04-02 18:05:24 +0000
@@ -25,6 +25,7 @@
     "UserProfile",
     ]
 
+import binascii
 from cgi import escape
 from collections import (
     defaultdict,
@@ -835,7 +836,7 @@
         if not key.isPublic():
             raise ValidationError(
                 "Invalid SSH public key (this key is a private key).")
-    except BadKeyError:
+    except (BadKeyError, binascii.Error):
         raise ValidationError("Invalid SSH public key.")
 
 

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-04-02 16:38:56 +0000
+++ src/maasserver/tests/test_models.py	2012-04-02 18:05:24 +0000
@@ -716,6 +716,13 @@
         self.assertRaises(
             ValidationError, validate_ssh_public_key, key_string)
 
+    def test_does_not_validate_wrongly_padded_data(self):
+        key_string = 'ssh-dss %s %s@%s' % (
+            factory.getRandomString(), factory.getRandomString(),
+            factory.getRandomString())
+        self.assertRaises(
+            ValidationError, validate_ssh_public_key, key_string)
+
     def test_does_not_validate_rsa_private_key(self):
         key_string = get_data('data/test_rsa')
         self.assertRaises(