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)
This commit is contained in:
Thomas Keller 2011-05-25 02:13:50 +02:00
parent 8502a36481
commit 5b82efa0be
3 changed files with 15 additions and 5 deletions

View File

@ -7,6 +7,7 @@
- monotone zip archive entries now all carry the revision date as mtime (issue 645) - 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) - 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 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 ## Documentation

View File

@ -317,8 +317,15 @@ class IDF_Form_UserAccount extends Pluf_Form
return ''; return '';
} }
if (preg_match('#^ssh\-[a-z]{3}\s\S+(\s\S+)?$#', $key)) { $keysearch = '';
$key = str_replace(array("\n", "\r"), '', $key); 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)) { 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 (Pluf::f('idf_strong_key_check', false)) {
// if monotone can read it, it should be valid // if monotone can read it, it should be valid
@ -367,7 +376,7 @@ class IDF_Form_UserAccount extends Pluf_Form
if ($user) { if ($user) {
$ruser = Pluf::factory('Pluf_User', $user); $ruser = Pluf::factory('Pluf_User', $user);
if ($ruser->id > 0) { 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())); $keys = Pluf::factory('IDF_Key')->getList(array('filter' => $sql->gen()));
if (count($keys) > 0) { if (count($keys) > 0) {
throw new Pluf_Form_Invalid( throw new Pluf_Form_Invalid(

View File

@ -80,7 +80,7 @@ class IDF_Key extends Pluf_Model
if (preg_match('#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#', $this->content, $m)) { if (preg_match('#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#', $this->content, $m)) {
return array('mtn', $m[1], $m[2]); 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])) { if (!isset($m[2])) {
$m[2] = ""; $m[2] = "";
} }