MagicEnvelope object orientation (no passing arrays)

MagicEnvelope now uses object properties instead of passing arrays
around everywhere.

plugins/OStatus/classes/Magicsig.php

@@ -279,6 +279,9 @@ class Magicsig extends Managed_DataObject
     public function sign($bytes)
     {
         $sig = $this->privateKey->sign($bytes);
+        if ($sig === false) {
+            throw new ServerException('Could not sign data');
+        }
         return Magicsig::base64_url_encode($sig);
     }
 

plugins/OStatus/lib/magicenvelope.php

@@ -32,6 +32,33 @@ class MagicEnvelope
 
     const NS = 'http://salmon-protocol.org/ns/magic-env';
 
+    protected $data      = null;    // When stored here it is _always_ base64url encoded
+    protected $data_type = null;
+    protected $encoding  = null;
+    protected $alg       = null;
+    protected $sig       = null;
+
+    /**
+     * Extract envelope data from an XML document containing an <me:env> or <me:provenance> element.
+     *
+     * @param string XML source
+     * @return mixed associative array of envelope data, or false on unrecognized input
+     *
+     * @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 __construct($xml=null) {
+        if (!empty($xml)) {
+            $dom = DOMDocument::loadXML($xml);
+            if (!$dom instanceof DOMDocument) {
+                throw new ServerException('Tried to load malformed XML as DOM');
+            } elseif (!$this->fromDom($dom)) {
+                throw new ServerException('Could not load MagicEnvelope from DOM');
+            }
+        }
+    }
+
     /**
      * Get the Salmon keypair from a URI, uses XRD Discovery etc.
      *
@@ -84,60 +111,54 @@ class MagicEnvelope
      * 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) {
+    public function signingText() {
-        return implode('.', array($env['data'], // this field is pre-base64'd
+        return implode('.', array($this->data, // this field is pre-base64'd
-                            Magicsig::base64_url_encode($env['data_type']),
+                            Magicsig::base64_url_encode($this->data_type),
-                            Magicsig::base64_url_encode($env['encoding']),
+                            Magicsig::base64_url_encode($this->encoding),
-                            Magicsig::base64_url_encode($env['alg'])));
+                            Magicsig::base64_url_encode($this->alg)));
     }
 
     /**
      *
      * @param <type> $text
      * @param <type> $mimetype
-     * @param <type> $keypair
+     * @param Magicsig $magicsig    Magicsig with private key available.
-     * @return array: associative array of envelope properties
+     * @return MagicEnvelope object with all properties set
-     * @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)
+    public static function signMessage($text, $mimetype, Magicsig $magicsig)
     {
-        $signature_alg = Magicsig::fromString($keypair);
+        $magic_env = new MagicEnvelope();
-        $armored_text = Magicsig::base64_url_encode($text);
-        $env = array(
-            'data' => $armored_text,
-            'encoding' => self::ENCODING,
-            'data_type' => $mimetype,
-            'sig' => '',
-            'alg' => $signature_alg->getName()
-        );
 
-        $env['sig'] = $signature_alg->sign($this->signingText($env));
+        // Prepare text and metadata for signing
+        $magic_env->data      = Magicsig::base64_url_encode($text);
+        $magic_env->data_type = $mimetype;
+        $magic_env->encoding  = self::ENCODING;
+        $magic_env->alg       = $magicsig->getName();
+        // Get the actual signature
+        $magic_env->sig = $magicsig->sign($magic_env->signingText());
 
-        return $env;
+        return $magic_env;
     }
 
     /**
      * Create an <me:env> 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) {
+    public function toXML() {
         $xs = new XMLStringer();
         $xs->startXML();
         $xs->elementStart('me:env', array('xmlns:me' => self::NS));
-        $xs->element('me:data', array('type' => $env['data_type']), $env['data']);
+        $xs->element('me:data', array('type' => $this->data_type), $this->data);
-        $xs->element('me:encoding', null, $env['encoding']);
+        $xs->element('me:encoding', null, $this->encoding);
-        $xs->element('me:alg', null, $env['alg']);
+        $xs->element('me:alg', null, $this->alg);
-        $xs->element('me:sig', null, $env['sig']);
+        $xs->element('me:sig', null, $this->sig);
         $xs->elementEnd('me:env');
 
         $string =  $xs->getString();
-        common_debug($string);
+        common_debug('MagicEnvelope XML: ' . $string);
         return $string;
     }
 
@@ -145,16 +166,14 @@ class MagicEnvelope
      * Extract the contained XML payload, and insert a copy of the envelope
      * signature data as an <me:provenance> 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)
+    public function unfold()
     {
         $dom = new DOMDocument();
-        $dom->loadXML(Magicsig::base64_url_decode($env['data']));
+        $dom->loadXML(Magicsig::base64_url_decode($this->data));
 
         if ($dom->documentElement->tagName != 'entry') {
             return false;
@@ -162,14 +181,14 @@ class MagicEnvelope
 
         $prov = $dom->createElementNS(self::NS, 'me:provenance');
         $prov->setAttribute('xmlns:me', self::NS);
-        $data = $dom->createElementNS(self::NS, 'me:data', $env['data']);
+        $data = $dom->createElementNS(self::NS, 'me:data', $this->data);
-        $data->setAttribute('type', $env['data_type']);
+        $data->setAttribute('type', $this->data_type);
         $prov->appendChild($data);
-        $enc = $dom->createElementNS(self::NS, 'me:encoding', $env['encoding']);
+        $enc = $dom->createElementNS(self::NS, 'me:encoding', $this->encoding);
         $prov->appendChild($enc);
-        $alg = $dom->createElementNS(self::NS, 'me:alg', $env['alg']);
+        $alg = $dom->createElementNS(self::NS, 'me:alg', $this->alg);
         $prov->appendChild($alg);
-        $sig = $dom->createElementNS(self::NS, 'me:sig', $env['sig']);
+        $sig = $dom->createElementNS(self::NS, 'me:sig', $this->sig);
         $prov->appendChild($sig);
 
         $dom->documentElement->appendChild($prov);
@@ -208,24 +227,22 @@ class MagicEnvelope
      *
      * 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)
+    public function verify()
     {
-        if ($env['alg'] != 'RSA-SHA256') {
+        if ($this->alg != 'RSA-SHA256') {
             common_log(LOG_DEBUG, "Salmon error: bad algorithm");
             return false;
         }
 
-        if ($env['encoding'] != self::ENCODING) {
+        if ($this->encoding != self::ENCODING) {
             common_log(LOG_DEBUG, "Salmon error: bad encoding");
             return false;
         }
 
-        $text = Magicsig::base64_url_decode($env['data']);
+        // $this->data is base64_url_encoded and should contain XML which is passed to getAuthorUri
+        $text = Magicsig::base64_url_decode($this->data);
         $signer_uri = $this->getAuthorUri($text);
 
         try {
@@ -235,24 +252,7 @@ class MagicEnvelope
             return false;
         }
 
-        return $magicsig->verify($this->signingText($env), $env['sig']);
+        return $magicsig->verify($this->signingText(), $this->sig);
-    }
-
-    /**
-     * Extract envelope data from an XML document containing an <me:env> or <me:provenance> 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);
     }
 
     /**
@@ -261,10 +261,9 @@ class MagicEnvelope
      * @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(DOMDocument $dom)
+    protected function fromDom(DOMDocument $dom)
     {
         $env_element = $dom->getElementsByTagNameNS(self::NS, 'env')->item(0);
         if (!$env_element) {
@@ -277,12 +276,51 @@ class MagicEnvelope
 
         $data_element = $env_element->getElementsByTagNameNS(self::NS, 'data')->item(0);
         $sig_element = $env_element->getElementsByTagNameNS(self::NS, 'sig')->item(0);
-        return array(
+
-            'data' => preg_replace('/\s/', '', $data_element->nodeValue),
+        $this->data      = preg_replace('/\s/', '', $data_element->nodeValue);
-            'data_type' => $data_element->getAttribute('type'),
+        $this->data_type = $data_element->getAttribute('type');
-            'encoding' => $env_element->getElementsByTagNameNS(self::NS, 'encoding')->item(0)->nodeValue,
+        $this->encoding  = $env_element->getElementsByTagNameNS(self::NS, 'encoding')->item(0)->nodeValue;
-            'alg' => $env_element->getElementsByTagNameNS(self::NS, 'alg')->item(0)->nodeValue,
+        $this->alg       = $env_element->getElementsByTagNameNS(self::NS, 'alg')->item(0)->nodeValue;
-            'sig' => preg_replace('/\s/', '', $sig_element->nodeValue),
+        $this->sig       = preg_replace('/\s/', '', $sig_element->nodeValue);
-        );
+        return true;
+    }
+
+    /**
+     * Encode the given string as a signed MagicEnvelope XML document,
+     * using the keypair for the given local user profile. We can of
+     * course not sign a remote profile's slap, since we don't have the
+     * private key.
+     *
+     * 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 static function signForProfile($text, Profile $actor)
+    {
+        // We only generate keys for our local users of course, so let
+        // getUser throw an exception if the profile is not local.
+        $user = $actor->getUser();
+
+        // Find already stored key
+        $magicsig = Magicsig::getKV('user_id', $user->id);
+        if (!$magicsig instanceof Magicsig) {
+            // No keypair yet, let's generate one.
+            $magicsig = new Magicsig();
+            $magicsig->generate($user->id);
+        }
+
+        $magic_env = self::signMessage($text, 'application/atom+xml', $magicsig);
+
+        assert($magicenv instanceof MagicEnvelope);
+
+        return $magic_env;
     }
 }

plugins/OStatus/lib/salmon.php

@@ -54,7 +54,8 @@ class Salmon
         }
 
         try {
-            $envelope = $this->createMagicEnv($xml, $actor);
+            $magic_env = MagicEnvelope::signForProfile($xml, $actor);
+            $envxml = $magic_env->toXML();
         } catch (Exception $e) {
             common_log(LOG_ERR, "Salmon unable to sign: " . $e->getMessage());
             return false;
@@ -79,67 +80,4 @@ class Salmon
         // Success!
         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();
-
-        // We only generate keys for our local users of course, so let
-        // getUser throw an exception if the profile is not local.
-        $user = $actor->getUser();
-
-        // Find already stored key
-        $magicsig = Magicsig::getKV('user_id', $user->id);
-        if (!$magicsig instanceof Magicsig) {
-            // No keypair yet, let's generate one.
-            $magicsig = new Magicsig();
-            $magicsig->generate($user->id);
-        }
-
-        try {
-            $env = $magic_env->signMessage($text, 'application/atom+xml', $magicsig->toString());
-        } catch (Exception $e) {
-            return $text;
-        }
-        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 if not
-     * already stored locally.
-     *
-     * 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();
-
-        $env = $magic_env->parse($text);
-
-        return $magic_env->verify($env);
-    }
 }

plugins/OStatus/lib/salmonaction.php

@@ -46,20 +46,15 @@ class SalmonAction extends Action
             $this->clientError(_m('Salmon requires "application/magic-envelope+xml".'));
         }
 
-        $xml = file_get_contents('php://input');
+        $envxml = file_get_contents('php://input');
-
+        $magic_env = new MagicEnvelope($envxml);   // parse incoming XML as a MagicEnvelope
-        // Check the signature
+        if (!$magic_env->verify()) {
-        $salmon = new Salmon;
-        if (!$salmon->verifyMagicEnv($xml)) {
             common_log(LOG_DEBUG, "Salmon signature verification failed.");
             // TRANS: Client error.
             $this->clientError(_m('Salmon signature verification failed.'));
-        } else {
-            $magic_env = new MagicEnvelope();
-            $env = $magic_env->parse($xml);
-            $xml = $magic_env->unfold($env);
         }
 
+        $xml = $magic_env->unfold();    // return the enveloped XML (the actual data)
         $dom = DOMDocument::loadXML($xml);
         if ($dom->documentElement->namespaceURI != Activity::ATOM ||
             $dom->documentElement->localName != 'entry') {

plugins/OStatus/tests/MagicEnvelopeTest.php

@@ -17,26 +17,26 @@ class MagicEnvelopeTest extends PHPUnit_Framework_TestCase
      * Test that MagicEnvelope builds the correct plaintext for signing.
      * @dataProvider provider
      */
-    public function testSignatureText($env, $expected)
+    public function testSignatureText(MagicEnvelope $env, $expected)
     {
-        $magic = new MagicEnvelope;
+        $text = $env->signingText();
-        $text = $magic->signingText($env);
 
         $this->assertEquals($expected, $text, "'$text' should be '$expected'");
     }
 
     static public function provider()
     {
+        // Sample case given in spec:
+        // http://salmon-protocol.googlecode.com/svn/trunk/draft-panzer-magicsig-00.html#signing
+        $magic_env = new MagicEnvelope();
+        $magic_env->data = 'Tm90IHJlYWxseSBBdG9t';
+        $magic_env->data_type = 'application/atom+xml';
+        $magic_env->encoding = 'base64url';
+        $magic_env->alg = 'RSA-SHA256';
+
         return array(
             array(
-                // Sample case given in spec:
+                $magic_env,
-                // 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=='
             )
         );

plugins/OStatus/tests/slap.php

@@ -51,15 +51,16 @@ echo "== Original entry ==\n\n";
 print $entry;
 print "\n\n";
 
-$salmon = new Salmon();
+$magic_env = MagicEnvelope::signForProfile($entry, $profile);
-$envelope = $salmon->createMagicEnv($entry, $profile);
+$envxml = $magic_env->toXML();
 
 echo "== Signed envelope ==\n\n";
-print $envelope;
+print $envxml;
 print "\n\n";
 
 echo "== Testing local verification ==\n\n";
-$ok = $salmon->verifyMagicEnv($envelope);
+$magic_env = new MagicEnvelope($envxml);
+$ok = $magic_env->verify();
 if ($ok) {
     print "OK\n\n";
 } else {
@@ -72,7 +73,7 @@ if (have_option('--verify')) {
     print "Sending for verification to $url ...\n";
 
     $client = new HTTPClient();
-    $response = $client->post($url, array(), array('magic_env' => $envelope));
+    $response = $client->post($url, array(), array('magic_env' => $envxml));
 
     print $response->getStatus() . "\n\n";
     print $response->getBody() . "\n\n";