From 5b82efa0be040a462a2be70ad790cc53c3b4a408 Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Wed, 25 May 2011 02:13:50 +0200 Subject: [PATCH] Fix a couple of issues with our key parsing / validating code. - be explicit and expect only ssh-dss or ssh-rsa keys - allow any character (even line breaks and whitespace) in the optional comment, but shrink all of them to simple spaces (fixes issue 679) - test the newly uploaded key against existing keys only by the base key data, not the fully uploaded string (that might contain a changed comment line or the like) to avoid duplicates; also only check the keys of the user for duplicates, not all existing keys in the forge (if for whatever reason two user accounts share a key) --- NEWS.mdtext | 1 + src/IDF/Form/UserAccount.php | 17 +++++++++++++---- src/IDF/Key.php | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/NEWS.mdtext b/NEWS.mdtext index 1acf206..21bf05b 100644 --- a/NEWS.mdtext +++ b/NEWS.mdtext @@ -7,6 +7,7 @@ - monotone zip archive entries now all carry the revision date as mtime (issue 645) - Timeline only displays filter options for items a user has actually access to (issue 655) - Fix the self-link of the RSS feed (issue 666) +- Fix SSH public key parsing issues and improve the check for existing, uploaded keys (issue 679) ## Documentation diff --git a/src/IDF/Form/UserAccount.php b/src/IDF/Form/UserAccount.php index 15d845e..7d9f716 100644 --- a/src/IDF/Form/UserAccount.php +++ b/src/IDF/Form/UserAccount.php @@ -317,8 +317,15 @@ class IDF_Form_UserAccount extends Pluf_Form return ''; } - if (preg_match('#^ssh\-[a-z]{3}\s\S+(\s\S+)?$#', $key)) { - $key = str_replace(array("\n", "\r"), '', $key); + $keysearch = ''; + if (preg_match('#^(ssh\-(?:dss|rsa)\s+\S+)(.*)#', $key, $m)) { + $basekey = preg_replace('/\s+/', ' ', $m[1]); + $comment = trim(preg_replace('/[\r\n]/', ' ', $m[2])); + + $keysearch = $basekey.'%'; + $key = $basekey; + if (!empty($comment)) + $key .= ' '.$comment; if (Pluf::f('idf_strong_key_check', false)) { @@ -337,7 +344,9 @@ class IDF_Form_UserAccount extends Pluf_Form } } } - else if (preg_match('#^\[pubkey [^\]]+\]\s*\S+\s*\[end\]$#', $key)) { + else if (preg_match('#^\[pubkey [^\]]+\]\s*(\S+)\s*\[end\]$#', $key, $m)) { + $keysearch = '%'.$m[1].'%'; + if (Pluf::f('idf_strong_key_check', false)) { // if monotone can read it, it should be valid @@ -367,7 +376,7 @@ class IDF_Form_UserAccount extends Pluf_Form if ($user) { $ruser = Pluf::factory('Pluf_User', $user); if ($ruser->id > 0) { - $sql = new Pluf_SQL('content=%s', array($key)); + $sql = new Pluf_SQL('content LIKE %s AND user=%s', array($keysearch, $ruser->id)); $keys = Pluf::factory('IDF_Key')->getList(array('filter' => $sql->gen())); if (count($keys) > 0) { throw new Pluf_Form_Invalid( diff --git a/src/IDF/Key.php b/src/IDF/Key.php index e933d52..85c9469 100644 --- a/src/IDF/Key.php +++ b/src/IDF/Key.php @@ -80,7 +80,7 @@ class IDF_Key extends Pluf_Model if (preg_match('#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#', $this->content, $m)) { return array('mtn', $m[1], $m[2]); } - else if (preg_match('#^ssh\-[a-z]{3}\s(\S+)(?:\s(\S*))?$#', $this->content, $m)) { + else if (preg_match('#^ssh\-(?:dss|rsa)\s(\S+)(?:\s(.*))?$#', $this->content, $m)) { if (!isset($m[2])) { $m[2] = ""; }