From 7f32a5679d01fb89b5bbdb0f3a75ff6cc23f0421 Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Tue, 24 Aug 2010 23:30:12 +0200 Subject: [PATCH] * removed `type` field in IDF_Key on request of Loic and automatically detect and validate raw key data * reworked the parseMonotoneKeyData() function to parse ssh and monotone keys * tweak help texts and exception strings --- src/IDF/Form/Admin/UserCreate.php | 23 ++----- src/IDF/Form/UserAccount.php | 69 +++++++------------ src/IDF/Key.php | 62 +++++++---------- src/IDF/Migrations/16KeyType.php | 55 --------------- src/IDF/Plugin/SyncGit/Cron.php | 3 +- .../templates/idf/gadmin/users/create.html | 6 +- src/IDF/templates/idf/user/myaccount.html | 6 +- 7 files changed, 60 insertions(+), 164 deletions(-) delete mode 100644 src/IDF/Migrations/16KeyType.php diff --git a/src/IDF/Form/Admin/UserCreate.php b/src/IDF/Form/Admin/UserCreate.php index 1e63073..e39ceb7 100644 --- a/src/IDF/Form/Admin/UserCreate.php +++ b/src/IDF/Form/Admin/UserCreate.php @@ -89,16 +89,8 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form 'widget_attrs' => array('rows' => 3, 'cols' => 40), 'widget' => 'Pluf_Form_Widget_TextareaInput', - 'help_text' => __('Be careful to provide the public key and not the private key!') + 'help_text' => __('Paste a SSH or monotone public key. Be careful to not provide your private key here!') )); - $this->fields['public_key_type'] = new Pluf_Form_Field_Varchar( - array('required' => true, - 'label' => __('Key type'), - 'initial' => 'ssh', - 'widget_attrs' => array('choices' => IDF_Key::getAvailableKeyTypes()), - 'widget' => 'Pluf_Form_Widget_SelectInput', - )); - } /** @@ -143,12 +135,11 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form $params = array('user' => $user); Pluf_Signal::send('Pluf_User::passwordUpdated', 'IDF_Form_Admin_UserCreate', $params); - // Create the ssh key as needed + // Create the public key as needed if ('' !== $this->cleaned_data['public_key']) { $key = new IDF_Key(); $key->user = $user; $key->content = $this->cleaned_data['public_key']; - $key->type = $this->cleaned_data['public_key_type']; $key->create(); } // Send an email to the user with the password @@ -214,15 +205,11 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form return $this->cleaned_data['login']; } - /** - * Checks whether any given public key is valid - */ - public function clean() + public function clean_public_key() { $this->cleaned_data['public_key'] = - IDF_Form_UserAccount::checkPublicKey($this->cleaned_data['public_key'], - $this->cleaned_data['public_key_type']); + IDF_Form_UserAccount::checkPublicKey($this->cleaned_data['public_key']); - return $this->cleaned_data; + return $this->cleaned_data['public_key']; } } diff --git a/src/IDF/Form/UserAccount.php b/src/IDF/Form/UserAccount.php index 176e74e..2c26cca 100644 --- a/src/IDF/Form/UserAccount.php +++ b/src/IDF/Form/UserAccount.php @@ -99,16 +99,8 @@ class IDF_Form_UserAccount extends Pluf_Form 'widget_attrs' => array('rows' => 3, 'cols' => 40), 'widget' => 'Pluf_Form_Widget_TextareaInput', - 'help_text' => __('Be careful to provide your public key and not your private key!') + 'help_text' => __('Paste a SSH or monotone public key. Be careful to not provide your private key here!') )); - $this->fields['public_key_type'] = new Pluf_Form_Field_Varchar( - array('required' => true, - 'label' => __('Key type'), - 'initial' => 'ssh', - 'widget_attrs' => array('choices' => IDF_Key::getAvailableKeyTypes()), - 'widget' => 'Pluf_Form_Widget_SelectInput', - )); - } /** @@ -161,7 +153,6 @@ class IDF_Form_UserAccount extends Pluf_Form $key = new IDF_Key(); $key->user = $this->user; $key->content = $this->cleaned_data['public_key']; - $key->type = $this->cleaned_data['public_key_type']; if ($commit) { $key->create(); } @@ -203,50 +194,39 @@ class IDF_Form_UserAccount extends Pluf_Form * validate the key. * * @param $key string The key - * @param $key string The type ('ssh' or 'mtn') * @param $user int The user id of the user of the key (0) * @return string The clean key */ - public static function checkPublicKey($key, $type, $user=0) + public static function checkPublicKey($key, $user=0) { $key = trim($key); if (strlen($key) == 0) { return ''; } - if ($type == 'ssh') { + if (preg_match('#^ssh\-[a-z]{3}\s\S+\s\S+$#', $key)) { $key = str_replace(array("\n", "\r"), '', $key); - if (!preg_match('#^ssh\-[a-z]{3}\s\S+\s\S+$#', $key)) { - throw new Pluf_Form_Invalid( - __('The format of the key is not valid. It must start '. - 'with ssh-dss or ssh-rsa, a long string on a single '. - 'line and at the end a comment.') - ); - } + if (Pluf::f('idf_strong_key_check', false)) { + $tmpfile = Pluf::f('tmp_folder', '/tmp').'/'.$user.'-key'; file_put_contents($tmpfile, $key, LOCK_EX); $cmd = Pluf::f('idf_exec_cmd_prefix', ''). 'ssh-keygen -l -f '.escapeshellarg($tmpfile).' > /dev/null 2>&1'; exec($cmd, $out, $return); unlink($tmpfile); + if ($return != 0) { throw new Pluf_Form_Invalid( - __('Please check the key as it does not appears '. - 'to be a valid key.') + __('Please check the key as it does not appear '. + 'to be a valid SSH public key.') ); } } } - else if ($type == 'mtn') { - if (!preg_match('#^\[pubkey [^\]]+\]\s*\S+\s*\[end\]$#', $key)) { - throw new Pluf_Form_Invalid( - __('The format of the key is not valid. It must start '. - 'with [pubkey KEYNAME], contain a long string on a single '. - 'line and end with [end] in the final third line.') - ); - } + else if (preg_match('#^\[pubkey [^\]]+\]\s*\S+\s*\[end\]$#', $key)) { if (Pluf::f('idf_strong_key_check', false)) { + // if monotone can read it, it should be valid $mtn_opts = implode(' ', Pluf::f('mtn_opts', array())); $cmd = Pluf::f('idf_exec_cmd_prefix', ''). @@ -258,22 +238,23 @@ class IDF_Form_UserAccount extends Pluf_Form if ($return != 0) { throw new Pluf_Form_Invalid( - __('Please check the key as it does not appears '. - 'to be a valid key.') + __('Please check the key as it does not appear '. + 'to be a valid monotone public key.') ); } } } - else - { - throw new Pluf_Form_Invalid(__('Unknown key type')); + else { + throw new Pluf_Form_Invalid( + __('Public key looks neither like a SSH '. + 'nor monotone public key.')); } // If $user, then check if not the same key stored if ($user) { $ruser = Pluf::factory('Pluf_User', $user); if ($ruser->id > 0) { - $sql = new Pluf_SQL('content=%s AND type=%s', array($key, $type)); + $sql = new Pluf_SQL('content=%s', array($key)); $keys = Pluf::factory('IDF_Key')->getList(array('filter' => $sql->gen())); if (count($keys) > 0) { throw new Pluf_Form_Invalid( @@ -317,9 +298,16 @@ class IDF_Form_UserAccount extends Pluf_Form return $this->cleaned_data['email']; } + function clean_public_key() + { + $this->cleaned_data['public_key'] = + self::checkPublicKey($this->cleaned_data['public_key'], + $this->user->id); + return $this->cleaned_data['public_key']; + } + /** - * Check to see if the 2 passwords are the same and if any - * given public key is valid + * Check to see if the 2 passwords are the same */ public function clean() { @@ -332,11 +320,6 @@ class IDF_Form_UserAccount extends Pluf_Form } } - $this->cleaned_data['public_key'] = - self::checkPublicKey($this->cleaned_data['public_key'], - $this->cleaned_data['public_key_type'], - $this->user->id); - return $this->cleaned_data; } } diff --git a/src/IDF/Key.php b/src/IDF/Key.php index 1e84ced..0d31ba3 100644 --- a/src/IDF/Key.php +++ b/src/IDF/Key.php @@ -54,13 +54,6 @@ class IDF_Key extends Pluf_Model 'blank' => false, 'verbose' => __('public key'), ), - 'type' => - array( - 'type' => 'Pluf_DB_Field_Varchar', - 'size' => 3, - 'blank' => false, - 'verbose' => __('key type'), - ), ); // WARNING: Not using getSqlTable on the Pluf_User object to // avoid recursion. @@ -82,27 +75,37 @@ class IDF_Key extends Pluf_Model return Pluf_Template::markSafe(Pluf_esc(substr($this->content, 0, 25)).' [...] '.Pluf_esc(substr($this->content, -55))); } - private function parseMonotoneKeyData() + private function parseContent() { - if ($this->type != "mtn") - throw new IDF_Exception("key is not a monotone key type"); + 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)) { + return array('ssh', $m[2], $m[1]); + } - preg_match("#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#", $this->content, $m); - if (count($m) != 3) - throw new IDF_Exception("invalid key data detected"); - - return array($m[1], $m[2]); + throw new IDF_Exception('invalid or unknown key data detected'); } /** - * Returns the key name of the key, i.e. most of the time the email - * address, which not neccessarily has to be unique across a project. + * Returns the type of the public key + * + * @return string 'ssh' or 'mtn' + */ + function getType() + { + list($type, , ) = $this->parseContent(); + return $type; + } + + /** + * Returns the key name of the key * * @return string */ - function getMonotoneKeyName() + function getName() { - list($keyName, ) = $this->parseMonotoneKeyData(); + list(, $keyName, ) = $this->parseContent(); return $keyName; } @@ -116,9 +119,11 @@ class IDF_Key extends Pluf_Model * * @return string */ - function getMonotoneKeyId() + function getMtnId() { - list($keyName, $keyData) = $this->parseMonotoneKeyData(); + list($type, $keyName, $keyData) = $this->parseContent(); + if ($type != 'mtn') + throw new IDF_Exception('key is not a monotone public key'); return sha1($keyName.":".$keyData); } @@ -174,19 +179,4 @@ class IDF_Key extends Pluf_Model Pluf_Signal::send('IDF_Key::preDelete', 'IDF_Key', $params); } - - /** - * Returns an associative array with available key types for this - * idf installation, ready for consumption for a