From 51d1535f155c1178d8d17a40f06b6d9f7df9ff98 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 14:05:59 -0800 Subject: [PATCH 01/17] Added doc comments on Salmon magicsig-related stuff to help in figuring out what's going on --- plugins/OStatus/classes/Magicsig.php | 111 +++++++++++++++++++- plugins/OStatus/classes/Ostatus_profile.php | 1 + plugins/OStatus/lib/magicenvelope.php | 72 +++++++++++++ plugins/OStatus/lib/salmon.php | 34 +++++- 4 files changed, 215 insertions(+), 3 deletions(-) diff --git a/plugins/OStatus/classes/Magicsig.php b/plugins/OStatus/classes/Magicsig.php index e057deb144..31d061e6a0 100644 --- a/plugins/OStatus/classes/Magicsig.php +++ b/plugins/OStatus/classes/Magicsig.php @@ -39,11 +39,41 @@ class Magicsig extends Memcached_DataObject public $__table = 'magicsig'; + /** + * Key to user.id/profile.id for the local user whose key we're storing. + * + * @var int + */ public $user_id; + + /** + * Flattened string representation of the key pair; callers should + * usually use $this->publicKey and $this->privateKey directly, + * which hold live Crypt_RSA key objects. + * + * @var string + */ public $keypair; + + /** + * Crypto algorithm used for this key; currently only RSA-SHA256 is supported. + * + * @var string + */ public $alg; + /** + * Public RSA key; gets serialized in/out via $this->keypair string. + * + * @var Crypt_RSA + */ public $publicKey; + + /** + * PrivateRSA key; gets serialized in/out via $this->keypair string. + * + * @var Crypt_RSA + */ public $privateKey; public function __construct($alg = 'RSA-SHA256') @@ -51,6 +81,13 @@ class Magicsig extends Memcached_DataObject $this->alg = $alg; } + /** + * Fetch a Magicsig object from the cache or database on a field match. + * + * @param string $k + * @param mixed $v + * @return Magicsig + */ public /*static*/ function staticGet($k, $v=null) { $obj = parent::staticGet(__CLASS__, $k, $v); @@ -103,6 +140,14 @@ class Magicsig extends Memcached_DataObject return array(false, false, false); } + /** + * Save this keypair into the database. + * + * Overloads default insert behavior to encode the live key objects + * as a flat string for storage. + * + * @return mixed + */ function insert() { $this->keypair = $this->toString(); @@ -110,6 +155,14 @@ class Magicsig extends Memcached_DataObject return parent::insert(); } + /** + * Generate a new keypair for a local user and store in the database. + * + * Warning: this can be very slow on systems without the GMP module. + * Runtimes of 20-30 seconds are not unheard-of. + * + * @param int $user_id id of local user we're creating a key for + */ public function generate($user_id) { $rsa = new Crypt_RSA(); @@ -128,6 +181,12 @@ class Magicsig extends Memcached_DataObject $this->insert(); } + /** + * Encode the keypair or public key as a string. + * + * @param boolean $full_pair set to false to leave out the private key. + * @return string + */ public function toString($full_pair = true) { $mod = Magicsig::base64_url_encode($this->publicKey->modulus->toBytes()); @@ -140,6 +199,13 @@ class Magicsig extends Memcached_DataObject return 'RSA.' . $mod . '.' . $exp . $private_exp; } + /** + * Decode a string representation of an RSA public key or keypair + * as a Magicsig object which can be used to sign or verify. + * + * @param string $text + * @return Magicsig + */ public static function fromString($text) { $magic_sig = new Magicsig(); @@ -168,6 +234,14 @@ class Magicsig extends Memcached_DataObject return $magic_sig; } + /** + * Fill out $this->privateKey or $this->publicKey with a Crypt_RSA object + * representing the give key (as mod/exponent pair). + * + * @param string $mod base64-encoded + * @param string $exp base64-encoded exponent + * @param string $type one of 'public' or 'private' + */ public function loadKey($mod, $exp, $type = 'public') { common_log(LOG_DEBUG, "Adding ".$type." key: (".$mod .', '. $exp .")"); @@ -186,11 +260,22 @@ class Magicsig extends Memcached_DataObject } } + /** + * Returns the name of the crypto algorithm used for this key. + * + * @return string + */ public function getName() { return $this->alg; } + /** + * Returns the name of a hash function to use for signing with this key. + * + * @return string + * @fixme is this used? doesn't seem to be called by name. + */ public function getHash() { switch ($this->alg) { @@ -200,24 +285,48 @@ class Magicsig extends Memcached_DataObject } } + /** + * Generate base64-encoded signature for the given byte string + * using our private key. + * + * @param string $bytes as raw byte string + * @return string base64-encoded signature + */ public function sign($bytes) { $sig = $this->privateKey->sign($bytes); return Magicsig::base64_url_encode($sig); } + /** + * + * @param string $signed_bytes as raw byte string + * @param string $signature as base64 + * @return boolean + */ public function verify($signed_bytes, $signature) { $signature = Magicsig::base64_url_decode($signature); return $this->publicKey->verify($signed_bytes, $signature); } - + /** + * URL-encoding-friendly base64 variant encoding. + * + * @param string $input + * @return string + */ public static function base64_url_encode($input) { return strtr(base64_encode($input), '+/', '-_'); } + /** + * URL-encoding-friendly base64 variant decoding. + * + * @param string $input + * @return string + */ public static function base64_url_decode($input) { return base64_decode(strtr($input, '-_', '+/')); diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 9c0f014fc6..06e42187d4 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -331,6 +331,7 @@ class Ostatus_profile extends Memcached_DataObject * an acceptable response from the remote site. * * @param mixed $entry XML string, Notice, or Activity + * @param Profile $actor * @return boolean success */ public function notifyActivity($entry, $actor) diff --git a/plugins/OStatus/lib/magicenvelope.php b/plugins/OStatus/lib/magicenvelope.php index 03e6f7c665..6d3956cb9b 100644 --- a/plugins/OStatus/lib/magicenvelope.php +++ b/plugins/OStatus/lib/magicenvelope.php @@ -81,6 +81,14 @@ class MagicEnvelope } + /** + * + * @param $text + * @param $mimetype + * @param $keypair + * @return array: associative array of envelope properties + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + */ public function signMessage($text, $mimetype, $keypair) { $signature_alg = Magicsig::fromString($keypair); @@ -95,6 +103,13 @@ class MagicEnvelope ); } + /** + * Create an XML representation of the envelope. + * + * @param array $env associative array with envelope data + * @return string representation of XML document + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + */ public function toXML($env) { $xs = new XMLStringer(); $xs->startXML(); @@ -110,6 +125,16 @@ class MagicEnvelope return $string; } + /** + * Extract the contained XML payload, and insert a copy of the envelope + * signature data as an section. + * + * @param array $env associative array with envelope data + * @return string representation of modified XML document + * + * @fixme in case of XML parsing errors, this will spew to the error log or output + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + */ public function unfold($env) { $dom = new DOMDocument(); @@ -136,6 +161,14 @@ class MagicEnvelope return $dom->saveXML(); } + /** + * Find the author URI referenced in the given Atom entry. + * + * @param string $text string containing Atom entry XML + * @return mixed URI string or false if XML parsing fails, or null if no author URI can be found + * + * @fixme XML parsing failures will spew to error logs/output + */ public function getAuthor($text) { $doc = new DOMDocument(); if (!$doc->loadXML($text)) { @@ -153,11 +186,30 @@ class MagicEnvelope } } + /** + * Check if the author in the Atom entry fragment claims to match + * the given identifier URI. + * + * @param string $text string containing Atom entry XML + * @param string $signer_uri + * @return boolean + */ public function checkAuthor($text, $signer_uri) { return ($this->getAuthor($text) == $signer_uri); } + /** + * Attempt to verify cryptographic signing for parsed envelope data. + * Requires network access to retrieve public key referenced by the envelope signer. + * + * Details of failure conditions are dumped to output log and not exposed to caller. + * + * @param array $env array representation of magic envelope data, as returned from MagicEnvelope::parse() + * @return boolean + * + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + */ public function verify($env) { if ($env['alg'] != 'RSA-SHA256') { @@ -190,12 +242,32 @@ class MagicEnvelope return $verifier->verify($env['data'], $env['sig']); } + /** + * Extract envelope data from an XML document containing an or element. + * + * @param string XML source + * @return mixed associative array of envelope data, or false on unrecognized input + * + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + * @fixme will spew errors to logs or output in case of XML parse errors + * @fixme may give fatal errors if some elements are missing or invalid XML + * @fixme calling DOMDocument::loadXML statically triggers warnings in strict mode + */ public function parse($text) { $dom = DOMDocument::loadXML($text); return $this->fromDom($dom); } + /** + * Extract envelope data from an XML document containing an or element. + * + * @param DOMDocument $dom + * @return mixed associative array of envelope data, or false on unrecognized input + * + * @fixme it might be easier to work with storing envelope data these in the object instead of passing arrays around + * @fixme may give fatal errors if some elements are missing + */ public function fromDom($dom) { $env_element = $dom->getElementsByTagNameNS(MagicEnvelope::NS, 'env')->item(0); diff --git a/plugins/OStatus/lib/salmon.php b/plugins/OStatus/lib/salmon.php index 963da65084..6137f7bee5 100644 --- a/plugins/OStatus/lib/salmon.php +++ b/plugins/OStatus/lib/salmon.php @@ -38,10 +38,12 @@ class Salmon /** * Sign and post the given Atom entry as a Salmon message. * - * @fixme pass through the actor for signing? + * Side effects: may generate a keypair on-demand for the given user, + * which can be very slow on some systems. * * @param string $endpoint_uri - * @param string $xml + * @param string $xml string representation of payload + * @param Profile $actor local user profile whose keys to sign with * @return boolean success */ public function post($endpoint_uri, $xml, $actor) @@ -75,6 +77,21 @@ class Salmon return true; } + /** + * Encode the given string as a signed MagicEnvelope XML document, + * using the keypair for the given local user profile. + * + * Side effects: will create and store a keypair on-demand if one + * hasn't already been generated for this user. This can be very slow + * on some systems. + * + * @param string $text XML fragment to sign, assumed to be Atom + * @param Profile $actor Profile of a local user to use as signer + * @return string XML string representation of magic envelope + * + * @throws Exception on bad profile input or key generation problems + * @fixme if signing fails, this seems to return the original text without warning. Is there a reason for this? + */ public function createMagicEnv($text, $actor) { $magic_env = new MagicEnvelope(); @@ -101,6 +118,19 @@ class Salmon return $magic_env->toXML($env); } + /** + * Check if the given magic envelope is well-formed and correctly signed. + * Needs to have network access to fetch public keys over the web. + * + * Side effects: exceptions and caching updates may occur during network + * fetches. + * + * @param string $text XML fragment of magic envelope + * @return boolean + * + * @throws Exception on bad profile input or key generation problems + * @fixme could hit fatal errors or spew output on invalid XML + */ public function verifyMagicEnv($text) { $magic_env = new MagicEnvelope(); From e25c34a2b60c4d6050896e51c63f1ba52dfbc45f Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 14:27:53 -0800 Subject: [PATCH 02/17] Salmon slap / magicsig test script Given a notice in the local system, we package it up as an Atom entry and MagicSig it up. We run the magicenv verification on it locally to make sure our own functions can decode it. Optionally with --verify we can send to Tuomas Koski's verification test service (not sure if this is working 100%) If given --slap= with a target Salmon endpoint, we'll sent it on and see if it liked it. (Note that StatusNet will reject if there's not a relevant mention, but will report acceptance for dupes so you can use a message that's already been delivered as a test.) --- plugins/OStatus/tests/slap.php | 92 ++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 plugins/OStatus/tests/slap.php diff --git a/plugins/OStatus/tests/slap.php b/plugins/OStatus/tests/slap.php new file mode 100644 index 0000000000..b5f9d3e073 --- /dev/null +++ b/plugins/OStatus/tests/slap.php @@ -0,0 +1,92 @@ +#!/usr/bin/env php +. + */ + +define('INSTALLDIR', realpath(dirname(__FILE__) . '/../../..')); + +$longoptions = array('verify', 'slap=', 'notice='); + +$helptext = << send signed Salmon slap to the destination endpoint + + +END_OF_HELP; + +require_once INSTALLDIR.'/scripts/commandline.inc'; + +if (!have_option('--notice')) { + print "$helptext"; + exit(1); +} + +$notice_id = get_option_value('--notice'); + +$notice = Notice::staticGet('id', $notice_id); +$profile = $notice->getProfile(); +$entry = $notice->asAtomEntry(true); + +echo "== Original entry ==\n\n"; +print $entry; +print "\n\n"; + +$salmon = new Salmon(); +$envelope = $salmon->createMagicEnv($entry, $profile); + +echo "== Signed envelope ==\n\n"; +print $envelope; +print "\n\n"; + +echo "== Testing local verification ==\n\n"; +$ok = $salmon->verifyMagicEnv($envelope); +if ($ok) { + print "OK\n\n"; +} else { + print "FAIL\n\n"; +} + +if (have_option('--verify')) { + $url = 'http://www.madebymonsieur.com/ostatus_discovery/magic_env/validate/'; + echo "== Testing remote verification ==\n\n"; + print "Sending for verification to $url ...\n"; + + $client = new HTTPClient(); + $response = $client->post($url, array(), array('magic_env' => $envelope)); + + print $response->getStatus() . "\n\n"; + print $response->getBody() . "\n\n"; +} + +if (have_option('--slap')) { + $url = get_option_value('--slap'); + echo "== Remote salmon slap ==\n\n"; + print "Sending signed Salmon slap to $url ...\n"; + + $ok = $salmon->post($url, $entry, $profile); + if ($ok) { + print "OK\n\n"; + } else { + print "FAIL\n\n"; + } +} From 946a4ac17b5152119e8618d3339777159b524d53 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 23:26:09 +0000 Subject: [PATCH 03/17] Add test cases for internal change in Salmon signing; fix for the new code. Updated sig passes Tuomas's verifier, which is a good sign --- plugins/OStatus/lib/magicenvelope.php | 46 ++++++++++++++-- plugins/OStatus/tests/MagicEnvelopeTest.php | 60 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 plugins/OStatus/tests/MagicEnvelopeTest.php diff --git a/plugins/OStatus/lib/magicenvelope.php b/plugins/OStatus/lib/magicenvelope.php index 6d3956cb9b..384506280d 100644 --- a/plugins/OStatus/lib/magicenvelope.php +++ b/plugins/OStatus/lib/magicenvelope.php @@ -80,6 +80,20 @@ class MagicEnvelope throw new Exception(_m('Unable to locate signer public key.')); } + /** + * The current MagicEnvelope spec as used in StatusNet 0.9.7 and later + * includes both the original data and some signing metadata fields as + * the input plaintext for the signature hash. + * + * @param array $env + * @return string + */ + public function signingText($env) { + return implode('.', array($env['data'], // this field is pre-base64'd + Magicsig::base64_url_encode($env['data_type']), + Magicsig::base64_url_encode($env['encoding']), + Magicsig::base64_url_encode($env['alg']))); + } /** * @@ -93,14 +107,17 @@ class MagicEnvelope { $signature_alg = Magicsig::fromString($keypair); $armored_text = Magicsig::base64_url_encode($text); - - return array( + $env = array( 'data' => $armored_text, 'encoding' => MagicEnvelope::ENCODING, 'data_type' => $mimetype, - 'sig' => $signature_alg->sign($armored_text), + 'sig' => '', 'alg' => $signature_alg->getName() ); + + $env['sig'] = $signature_alg->sign($this->signingText($env)); + + return $env; } /** @@ -239,7 +256,7 @@ class MagicEnvelope return false; } - return $verifier->verify($env['data'], $env['sig']); + return $verifier->verify($this->signingText($env), $env['sig']); } /** @@ -290,3 +307,24 @@ class MagicEnvelope ); } } + +/** + * Variant of MagicEnvelope using the earlier signature form listed in the MagicEnvelope + * spec in early 2010; this was used in StatusNet up through 0.9.6, so for backwards compatiblity + * we still need to accept and sometimes send this format. + */ +class MagicEnvelopeCompat extends MagicEnvelope { + + /** + * StatusNet through 0.9.6 used an earlier version of the MagicEnvelope spec + * which used only the input data, without the additional fields, as the plaintext + * for signing. + * + * @param array $env + * @return string + */ + public function signingText($env) { + return $env['data']; + } +} + diff --git a/plugins/OStatus/tests/MagicEnvelopeTest.php b/plugins/OStatus/tests/MagicEnvelopeTest.php new file mode 100644 index 0000000000..5ee0362d28 --- /dev/null +++ b/plugins/OStatus/tests/MagicEnvelopeTest.php @@ -0,0 +1,60 @@ +signingText($env); + + $this->assertEquals($expected, $text, "'$text' should be '$expected'"); + } + + static public function provider() + { + return array( + array( + // Sample case given in spec: + // http://salmon-protocol.googlecode.com/svn/trunk/draft-panzer-magicsig-00.html#signing + array( + 'data' => 'Tm90IHJlYWxseSBBdG9t', + 'data_type' => 'application/atom+xml', + 'encoding' => 'base64url', + 'alg' => 'RSA-SHA256' + ), + 'Tm90IHJlYWxseSBBdG9t.YXBwbGljYXRpb24vYXRvbSt4bWw=.YmFzZTY0dXJs.UlNBLVNIQTI1Ng==' + ) + ); + } + + + /** + * Test that MagicEnvelope builds the correct plaintext for signing. + * @dataProvider provider + */ + public function testSignatureTextCompat($env, $expected) + { + // Our old code didn't add the extra fields, just used the armored text. + $alt = $env['data']; + + $magic = new MagicEnvelopeCompat; + $text = $magic->signingText($env); + + $this->assertEquals($alt, $text, "'$text' should be '$alt'"); + } + +} From f5650806cc0556d93ada1b43b16608ea3695c76a Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 23:27:17 +0000 Subject: [PATCH 04/17] Switch autoloader from '__autoload' magic function name to registering our function with spl_autoload_register(); fixes compat problem with PHPUnit 3.5+ which seems to break the old __autoload --- lib/common.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/common.php b/lib/common.php index 6138200e49..b072a49699 100644 --- a/lib/common.php +++ b/lib/common.php @@ -95,7 +95,11 @@ function _have_config() return StatusNet::haveConfig(); } -function __autoload($cls) +/** + * Wrapper for class autoloaders. + * This used to be the special function name __autoload(), but that causes bugs with PHPUnit 3.5+ + */ +function autoload_sn($cls) { if (file_exists(INSTALLDIR.'/classes/' . $cls . '.php')) { require_once(INSTALLDIR.'/classes/' . $cls . '.php'); @@ -111,6 +115,8 @@ function __autoload($cls) } } +spl_autoload_register('autoload_sn'); + // XXX: how many of these could be auto-loaded on use? // XXX: note that these files should not use config options // at compile time since DB config options are not yet loaded. From 437ac120b07542952c30c21ec93f8ebecda012a3 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 23:54:16 +0000 Subject: [PATCH 05/17] Outgoing Salmon slaps now use the corrected signature format; if the first hit is rejected with an HTTP error, we try again with the old format. (This is not 100% ideal; possibly should try to distinguish between server errors and rejections, etc.) --- plugins/OStatus/lib/salmon.php | 54 +++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/plugins/OStatus/lib/salmon.php b/plugins/OStatus/lib/salmon.php index 6137f7bee5..5535049203 100644 --- a/plugins/OStatus/lib/salmon.php +++ b/plugins/OStatus/lib/salmon.php @@ -52,29 +52,35 @@ class Salmon return false; } - try { - $xml = $this->createMagicEnv($xml, $actor); - } catch (Exception $e) { - common_log(LOG_ERR, "Salmon unable to sign: " . $e->getMessage()); - return false; - } + $classes = array('MagicEnvelope', 'MagicEnvelopeCompat'); + foreach ($classes as $class) { + try { + $envelope = $this->createMagicEnv($xml, $actor, $class); + } catch (Exception $e) { + common_log(LOG_ERR, "Salmon unable to sign: " . $e->getMessage()); + return false; + } + + $headers = array('Content-Type: application/magic-envelope+xml'); + + try { + $client = new HTTPClient(); + $client->setBody($envelope); + $response = $client->post($endpoint_uri, $headers); + } catch (HTTP_Request2_Exception $e) { + common_log(LOG_ERR, "Salmon ($class) post to $endpoint_uri failed: " . $e->getMessage()); + continue; + } + if ($response->getStatus() != 200) { + common_log(LOG_ERR, "Salmon ($class) at $endpoint_uri returned status " . + $response->getStatus() . ': ' . $response->getBody()); + continue; + } - $headers = array('Content-Type: application/magic-envelope+xml'); - - try { - $client = new HTTPClient(); - $client->setBody($xml); - $response = $client->post($endpoint_uri, $headers); - } catch (HTTP_Request2_Exception $e) { - common_log(LOG_ERR, "Salmon post to $endpoint_uri failed: " . $e->getMessage()); - return false; + // Success! + return true; } - if ($response->getStatus() != 200) { - common_log(LOG_ERR, "Salmon at $endpoint_uri returned status " . - $response->getStatus() . ': ' . $response->getBody()); - return false; - } - return true; + return false; } /** @@ -87,14 +93,16 @@ class Salmon * * @param string $text XML fragment to sign, assumed to be Atom * @param Profile $actor Profile of a local user to use as signer + * @param string $class to override the magic envelope signature version, pass a MagicEnvelope subclass here + * * @return string XML string representation of magic envelope * * @throws Exception on bad profile input or key generation problems * @fixme if signing fails, this seems to return the original text without warning. Is there a reason for this? */ - public function createMagicEnv($text, $actor) + public function createMagicEnv($text, $actor, $class='MagicEnvelope') { - $magic_env = new MagicEnvelope(); + $magic_env = new $class(); $user = User::staticGet('id', $actor->id); if ($user->id) { From 1d15145993a00d1db1057dacf71f3783cd16c119 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 00:01:42 +0000 Subject: [PATCH 06/17] Salmon signature checks on incoming slaps now check both old and new signature formats. --- plugins/OStatus/lib/salmon.php | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/plugins/OStatus/lib/salmon.php b/plugins/OStatus/lib/salmon.php index 5535049203..2f5772a844 100644 --- a/plugins/OStatus/lib/salmon.php +++ b/plugins/OStatus/lib/salmon.php @@ -52,8 +52,7 @@ class Salmon return false; } - $classes = array('MagicEnvelope', 'MagicEnvelopeCompat'); - foreach ($classes as $class) { + foreach ($this->formatClasses() as $class) { try { $envelope = $this->createMagicEnv($xml, $actor, $class); } catch (Exception $e) { @@ -83,6 +82,15 @@ class Salmon return false; } + /** + * List the magic envelope signature class variants in the order we try them. + * Multiples are needed for backwards-compat with StatusNet prior to 0.9.7, + * which used a draft version of the magic envelope spec. + */ + protected function formatClasses() { + return array('MagicEnvelope', 'MagicEnvelopeCompat'); + } + /** * Encode the given string as a signed MagicEnvelope XML document, * using the keypair for the given local user profile. @@ -129,6 +137,7 @@ class Salmon /** * Check if the given magic envelope is well-formed and correctly signed. * Needs to have network access to fetch public keys over the web. + * Both current and back-compat signature formats will be checked. * * Side effects: exceptions and caching updates may occur during network * fetches. @@ -141,10 +150,16 @@ class Salmon */ public function verifyMagicEnv($text) { - $magic_env = new MagicEnvelope(); + foreach ($this->formatClasses() as $class) { + $magic_env = new $class(); - $env = $magic_env->parse($text); + $env = $magic_env->parse($text); - return $magic_env->verify($env); + if ($magic_env->verify($env)) { + return true; + } + } + + return false; } } From 281076d5f68e07d67709a5e97740a4b625f4ad68 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 13:22:08 -0800 Subject: [PATCH 07/17] Fix for PHP notice spew in group creation via API: set default 'mainpage' in User_group::register() rather than forcing all callers to do it manually. --- actions/newgroup.php | 3 --- classes/User_group.php | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/actions/newgroup.php b/actions/newgroup.php index 42d488e54e..fb7f6347d4 100644 --- a/actions/newgroup.php +++ b/actions/newgroup.php @@ -200,8 +200,6 @@ class NewgroupAction extends Action } } - $mainpage = common_local_url('showgroup', array('nickname' => $nickname)); - $cur = common_current_user(); // Checked in prepare() above @@ -215,7 +213,6 @@ class NewgroupAction extends Action 'location' => $location, 'aliases' => $aliases, 'userid' => $cur->id, - 'mainpage' => $mainpage, 'local' => true)); common_redirect($group->homeUrl(), 303); diff --git a/classes/User_group.php b/classes/User_group.php index 68f61cb7f4..d402ed4773 100644 --- a/classes/User_group.php +++ b/classes/User_group.php @@ -487,6 +487,7 @@ class User_group extends Memcached_DataObject } // MAGICALLY put fields into current scope + // @fixme kill extract(); it makes debugging absurdly hard extract($fields); @@ -498,6 +499,9 @@ class User_group extends Memcached_DataObject // fill in later... $uri = null; } + if (empty($mainpage)) { + $mainpage = common_local_url('showgroup', array('nickname' => $nickname)); + } $group->nickname = $nickname; $group->fullname = $fullname; From edf8101b29010e9e8f9f27b628db10ef7a40400f Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 13:46:39 -0800 Subject: [PATCH 08/17] allow group join/leave commands in api posting, at least for the moment (no other way to do remote subscribe without a preexisting local id number via api) --- actions/apistatusesupdate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/apistatusesupdate.php b/actions/apistatusesupdate.php index a8ec7f8bb9..5773bdc2e8 100644 --- a/actions/apistatusesupdate.php +++ b/actions/apistatusesupdate.php @@ -377,7 +377,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction function supported($cmd) { static $cmdlist = array('MessageCommand', 'SubCommand', 'UnsubCommand', - 'FavCommand', 'OnCommand', 'OffCommand'); + 'FavCommand', 'OnCommand', 'OffCommand', 'JoinCommand', 'LeaveCommand'); if (in_array(get_class($cmd), $cmdlist)) { return true; From f2a43769e7a77a626df6111c2059870e548bc47a Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 15:05:29 -0800 Subject: [PATCH 09/17] Fix for atom/activity streams parsing: feed's was being taken at a higher priority than entry's , which broke OStatus group posting since we retired . Added test case to ActivityParseTests. --- lib/activity.php | 18 ++++-- tests/ActivityParseTests.php | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 5 deletions(-) diff --git a/lib/activity.php b/lib/activity.php index 8d7ae1540b..7a33c23b14 100644 --- a/lib/activity.php +++ b/lib/activity.php @@ -182,6 +182,9 @@ class Activity $actorEl = $this->_child($entry, self::ACTOR); if (!empty($actorEl)) { + // Standalone elements are a holdover from older + // versions of ActivityStreams. Newer feeds should have this data + // integrated straight into . $this->actor = new ActivityObject($actorEl); @@ -196,19 +199,24 @@ class Activity $this->actor->id = $authorObj->id; } } - } else if (!empty($feed) && - $subjectEl = $this->_child($feed, self::SUBJECT)) { - - $this->actor = new ActivityObject($subjectEl); - } else if ($authorEl = $this->_child($entry, self::AUTHOR, self::ATOM)) { + // An in the entry overrides any author info on + // the surrounding feed. $this->actor = new ActivityObject($authorEl); } else if (!empty($feed) && $authorEl = $this->_child($feed, self::AUTHOR, self::ATOM)) { + // If there's no on the entry, it's safe to assume + // the containing feed's authorship info applies. $this->actor = new ActivityObject($authorEl); + } else if (!empty($feed) && + $subjectEl = $this->_child($feed, self::SUBJECT)) { + + // Feed subject is used for things like groups. + // Should actually possibly not be interpreted as an actor...? + $this->actor = new ActivityObject($subjectEl); } $contextEl = $this->_child($entry, self::CONTEXT); diff --git a/tests/ActivityParseTests.php b/tests/ActivityParseTests.php index 378478d741..c2817a4602 100644 --- a/tests/ActivityParseTests.php +++ b/tests/ActivityParseTests.php @@ -382,6 +382,29 @@ class ActivityParseTests extends PHPUnit_Framework_TestCase } } + public function testExample10() + { + global $_example10; + $dom = new DOMDocument(); + $dom->loadXML($_example10); + + // example 10 is a PuSH item of a post on a group feed, as generated + // by 0.9.7 code after migration away from to + $feed = $dom->documentElement; + $entry = $dom->getElementsByTagName('entry')->item(0); + $expected = 'http://lazarus.local/mublog/user/557'; + + // Reading just the entry alone should pick up its own + // as the actor. + $act = new Activity($entry); + $this->assertEquals($act->actor->id, $expected); + + // Reading the entry in feed context used to be buggy, picking up + // the feed's which referred to the group. + // It should now be returning the expected author entry... + $act = new Activity($entry, $feed); + $this->assertEquals($act->actor->id, $expected); + } } $_example1 = << EXAMPLE9; + +// Sample PuSH entry from a group feed in 0.9.7 +// Old has been removed from entries in this version. +// A bug in the order of input processing meant that we were incorrectly +// reading the feed's instead of the entry's , +// causing the entry to get rejected as malformed (groups can't post on +// their own; we want to see the actual author's info here). +$_example10 = << + + StatusNet + http://lazarus.local/mublog/api/statusnet/groups/timeline/22.atom + grouptest316173 timeline + Updates from grouptest316173 on Blaguette! + http://lazarus.local/mublog/theme/default/default-avatar-profile.png + 2011-01-06T22:44:18+00:00 + + http://activitystrea.ms/schema/1.0/group + http://lazarus.local/mublog/group/22/id + grouptest316173 + + + + + grouptest316173 + grouptest316173 + + + http://activitystrea.ms/schema/1.0/group + http://lazarus.local/mublog/group/22/id + grouptest316173 + + + + + grouptest316173 + grouptest316173 + + + + + + + + + + http://activitystrea.ms/schema/1.0/note + http://lazarus.local/mublog/notice/1243 + Group post from local to !grouptest316173, should go out over push. + Group post from local to !<span class="vcard"><a href="http://lazarus.local/mublog/group/22/id" class="url"><span class="fn nickname">grouptest316173</span></a></span>, should go out over push. + + http://activitystrea.ms/schema/1.0/post + 2011-01-06T22:44:18+00:00 + 2011-01-06T22:44:18+00:00 + + http://activitystrea.ms/schema/1.0/person + http://lazarus.local/mublog/user/557 + Pubtest316173 Smith + + + + + pubtest316173 + Pubtest316173 Smith + Stub account for OStatus tests. + + homepage + http://example.org/pubtest316173 + true + + + + + + + + http://lazarus.local/mublog/api/statuses/user_timeline/557.atom + Pubtest316173 Smith + + + + http://lazarus.local/mublog/theme/default/default-avatar-profile.png + 2011-01-06T22:44:18+00:00 + + + + + + +EXAMPLE10; From 7ec456198a4041974189462cd0f7b763eff972e6 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 13:52:12 -0800 Subject: [PATCH 10/17] Ticket #2732: add some regression tests for groups to OStatus remote-tests.php Note that these tests won't pass on master branch yet as the join/leave don't work, and there's a bug in Activity parsing which prevents interop between new feeds and old remote subscribers (both fixed in this branch). --- plugins/OStatus/tests/remote-tests.php | 92 ++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/plugins/OStatus/tests/remote-tests.php b/plugins/OStatus/tests/remote-tests.php index 7888ec7c5d..1dce588dfe 100644 --- a/plugins/OStatus/tests/remote-tests.php +++ b/plugins/OStatus/tests/remote-tests.php @@ -72,6 +72,8 @@ class OStatusTester extends TestBase $base = 'test' . mt_rand(1, 1000000); $this->pub = new SNTestClient($this->a, 'pub' . $base, 'pw-' . mt_rand(1, 1000000), $timeout); $this->sub = new SNTestClient($this->b, 'sub' . $base, 'pw-' . mt_rand(1, 1000000), $timeout); + + $this->group = 'group' . $base; } function run() @@ -163,6 +165,39 @@ class OStatusTester extends TestBase $this->assertFalse($this->pub->hasSubscriber($this->sub->getProfileUri())); } + function testCreateGroup() + { + $this->groupUrl = $this->pub->createGroup($this->group); + $this->assertTrue(!empty($this->groupUrl)); + } + + function testJoinGroup() + { + #$this->assertFalse($this->sub->inGroup($this->groupUrl)); + $this->sub->joinGroup($this->groupUrl); + #$this->assertTrue($this->sub->inGroup($this->groupUrl)); + } + + function testLocalGroupPost() + { + $post = $this->pub->post("Group post from local to !{$this->group}, should go out over push."); + $this->assertNotEqual('', $post); + $this->sub->assertReceived($post); + } + + function testRemoteGroupPost() + { + $post = $this->sub->post("Group post from remote to !{$this->group}, should come in over salmon."); + $this->assertNotEqual('', $post); + $this->pub->assertReceived($post); + } + + function testLeaveGroup() + { + #$this->assertTrue($this->sub->inGroup($this->groupUrl)); + $this->sub->leaveGroup($this->groupUrl); + #$this->assertFalse($this->sub->inGroup($this->groupUrl)); + } } class SNTestClient extends TestBase @@ -534,6 +569,63 @@ class SNTestClient extends TestBase return false; } + /** + * Create a group on this site. + * + * @param string $nickname + * @param array $options + * @return string: profile URL for the group + */ + function createGroup($nickname, $options=array()) { + $this->log("Creating group as %s on %s: %s", + $this->username, + $this->basepath, + $nickname); + + $data = $this->api('statusnet/groups/create', 'json', + array_merge(array('nickname' => $nickname), $options)); + $url = $data['url']; + + if ($url) { + $this->log(' created as %s', $url); + } else { + $this->log(' failed? %s', var_export($data, true)); + } + return $url; + } + + function groupInfo($nickname) { + $data = $this->api('statusnet/groups/show', 'json', array( + 'id' => $nickname + )); + } + + /** + * Join a group. + * + * @param string $group nickname or URL + */ + function joinGroup($group) { + $this->post('join ' . $group); + } + + /** + * Leave a group. + * + * @param string $group nickname or URL + */ + function leaveGroup($group) { + $this->post('drop ' . $group); + } + + /** + * + * @param string $nickname + * @return + */ + function inGroup($nickname) { + // @todo + } } // @fixme switch to commandline.inc? From f97380fdb5c03557fc01ee8301f4c558e916d374 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 16:36:57 -0800 Subject: [PATCH 11/17] Fix regression in last year's update of InfiniteScroll -- having debug off caused breakage due to bad code interpreting every variable as a selector, and jQuery then failing when passed 'false'. Note that the current version of the infinitescroll jquery plugin fixes this, but I'm not updating to it because the code's been altered from the upstream version, apparently to stop it from actually working as infinite scroll. WTF? :) --- plugins/InfiniteScroll/jquery.infinitescroll.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/InfiniteScroll/jquery.infinitescroll.js b/plugins/InfiniteScroll/jquery.infinitescroll.js index ec31bb0863..dd61f8b18a 100644 --- a/plugins/InfiniteScroll/jquery.infinitescroll.js +++ b/plugins/InfiniteScroll/jquery.infinitescroll.js @@ -21,7 +21,7 @@ // grab each selector option and see if any fail. function areSelectorsValid(opts){ for (var key in opts){ - if (key.indexOf && key.indexOf('Selector') && $(opts[key]).length === 0){ + if (key.indexOf && (key.indexOf('Selector') != -1) && $(opts[key]).length === 0){ debug('Your ' + key + ' found no elements.'); return false; } From 36711f305a93c21e485da328eca4b4e35e2510db Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 16:53:39 -0800 Subject: [PATCH 12/17] Ticket #1968: fix favoriting, reply when using InfiniteScroll --- plugins/InfiniteScroll/infinitescroll.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/InfiniteScroll/infinitescroll.js b/plugins/InfiniteScroll/infinitescroll.js index 0c8edce2b0..961c320916 100644 --- a/plugins/InfiniteScroll/infinitescroll.js +++ b/plugins/InfiniteScroll/infinitescroll.js @@ -16,6 +16,12 @@ jQuery(document).ready(function($){ contentSelector : "#notices_primary ol.notices", itemSelector : "#notices_primary ol.notices li" },function(){ - SN.Init.Notices(); + // Reply button and attachment magic need to be set up + // for each new notice. + // DO NOT run SN.Init.Notices() which will duplicate stuff. + $(this).find('.notice').each(function() { + SN.U.NoticeReplyTo($(this)); + SN.U.NoticeWithAttachment($(this)); + }); }); }); From 35507cd0394fd058d69e65e7d18fcf880588bf2b Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 6 Jan 2011 17:43:00 -0800 Subject: [PATCH 13/17] Fix ticket #2392: sending invitation email fails when site name contains double quotes Gotta escape quotes! --- lib/mail.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mail.php b/lib/mail.php index dd6a1a366e..9b6b7d6988 100644 --- a/lib/mail.php +++ b/lib/mail.php @@ -121,7 +121,7 @@ function mail_notify_from() $domain = mail_domain(); - $notifyfrom = '"'.common_config('site', 'name') .'" '; + $notifyfrom = '"'. str_replace('"', '\\"', common_config('site', 'name')) .'" '; } return $notifyfrom; From 5616bfb5ffa3af4c6a375ff9c8c8560d86208898 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 7 Jan 2011 15:29:30 -0800 Subject: [PATCH 14/17] Fix warning in subscribers/subscriptions list pages where we attempted to call free() an ArrayWrapper after it was used up, thus trying to forward the call to a nonexistent object. Removed the free calls (unneeded since destructors now work), and added an error check w/ logging & an exception for future attempts to forward calls to nonexistent object. --- actions/subscribers.php | 2 -- actions/subscriptions.php | 2 -- lib/arraywrapper.php | 4 ++++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/actions/subscribers.php b/actions/subscribers.php index 2862f35c6d..ad522a4bae 100644 --- a/actions/subscribers.php +++ b/actions/subscribers.php @@ -100,8 +100,6 @@ class SubscribersAction extends GalleryAction } } - $subscribers->free(); - $this->pagination($this->page > 1, $cnt > PROFILES_PER_PAGE, $this->page, 'subscribers', array('nickname' => $this->user->nickname)); diff --git a/actions/subscriptions.php b/actions/subscriptions.php index a814a4f354..ddcf237e62 100644 --- a/actions/subscriptions.php +++ b/actions/subscriptions.php @@ -106,8 +106,6 @@ class SubscriptionsAction extends GalleryAction } } - $subscriptions->free(); - $this->pagination($this->page > 1, $cnt > PROFILES_PER_PAGE, $this->page, 'subscriptions', array('nickname' => $this->user->nickname)); diff --git a/lib/arraywrapper.php b/lib/arraywrapper.php index 8a1cdd96e1..f9d3c3cf94 100644 --- a/lib/arraywrapper.php +++ b/lib/arraywrapper.php @@ -76,6 +76,10 @@ class ArrayWrapper function __call($name, $args) { $item =& $this->_items[$this->_i]; + if (!is_object($item)) { + common_log(LOG_ERR, "Invalid entry " . var_export($item, true) . " at index $this->_i of $this->N; calling $name()"); + throw new ServerException("Internal error: bad entry in array wrapper list."); + } return call_user_func_array(array($item, $name), $args); } } From cb56f445b89ab0c60236bdeb3335305d825576cf Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 7 Jan 2011 16:23:54 -0800 Subject: [PATCH 15/17] Ticket #2166: accept aliases for local group names in API Also simplifies the code by using User_group::getForNickname instead of duplicating half of it :D --- lib/apiaction.php | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/lib/apiaction.php b/lib/apiaction.php index 2de513cbb1..dcce18ef27 100644 --- a/lib/apiaction.php +++ b/lib/apiaction.php @@ -1437,41 +1437,23 @@ class ApiAction extends Action { if (empty($id)) { if (self::is_decimal($this->arg('id'))) { - return User_group::staticGet($this->arg('id')); + return User_group::staticGet('id', $this->arg('id')); } else if ($this->arg('id')) { - $nickname = common_canonical_nickname($this->arg('id')); - $local = Local_group::staticGet('nickname', $nickname); - if (empty($local)) { - return null; - } else { - return User_group::staticGet('id', $local->id); - } + return User_group::getForNickname($this->arg('id')); } else if ($this->arg('group_id')) { - // This is to ensure that a non-numeric user_id still - // overrides screen_name even if it doesn't get used + // This is to ensure that a non-numeric group_id still + // overrides group_name even if it doesn't get used if (self::is_decimal($this->arg('group_id'))) { return User_group::staticGet('id', $this->arg('group_id')); } } else if ($this->arg('group_name')) { - $nickname = common_canonical_nickname($this->arg('group_name')); - $local = Local_group::staticGet('nickname', $nickname); - if (empty($local)) { - return null; - } else { - return User_group::staticGet('id', $local->group_id); - } + return User_group::getForNickname($this->arg('group_name')); } } else if (self::is_decimal($id)) { - return User_group::staticGet($id); + return User_group::staticGet('id', $id); } else { - $nickname = common_canonical_nickname($id); - $local = Local_group::staticGet('nickname', $nickname); - if (empty($local)) { - return null; - } else { - return User_group::staticGet('id', $local->group_id); - } + return User_group::getForNickname($id); } } From 74a1c9def08a052180d50854ac6f0e28795f7bfa Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 7 Jan 2011 16:25:58 -0800 Subject: [PATCH 16/17] Fix error handling for missing group in apigroupmembership -- was trying to call methods on the variable before we checked it, which triggers PHP fatal error --- actions/apigroupmembership.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/actions/apigroupmembership.php b/actions/apigroupmembership.php index 99ac965fa1..939d22d757 100644 --- a/actions/apigroupmembership.php +++ b/actions/apigroupmembership.php @@ -66,6 +66,12 @@ class ApiGroupMembershipAction extends ApiPrivateAuthAction parent::prepare($args); $this->group = $this->getTargetGroup($this->arg('id')); + if (empty($this->group)) { + // TRANS: Client error displayed trying to show group membership on a non-existing group. + $this->clientError(_('Group not found.'), 404, $this->format); + return false; + } + $this->profiles = $this->getProfiles(); return true; @@ -84,12 +90,6 @@ class ApiGroupMembershipAction extends ApiPrivateAuthAction { parent::handle($args); - if (empty($this->group)) { - // TRANS: Client error displayed trying to show group membership on a non-existing group. - $this->clientError(_('Group not found.'), 404, $this->format); - return false; - } - // XXX: RSS and Atom switch($this->format) { From a3c08faddd8bce8887bbcf263cec8bdc2a6c0f11 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Mon, 17 Jan 2011 17:32:44 -0500 Subject: [PATCH 17/17] Erroneous code ensuring Webfinger accounts Ostatus_profile::ensureProfileURI() was accidentally falling through to the default switch case, and was also calling common_log() incorrectly. --- plugins/OStatus/classes/Ostatus_profile.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 06e42187d4..303e177a57 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -1779,8 +1779,10 @@ class Ostatus_profile extends Memcached_DataObject case 'mailto': $rest = $match[2]; $oprofile = Ostatus_profile::ensureWebfinger($rest); + break; default: - common_log("Unrecognized URI protocol for profile: $protocol ($uri)"); + common_log(LOG_WARNING, + "Unrecognized URI protocol for profile: $protocol ($uri)"); break; } }