From 09eb52810bbae39ba84b869d753ea2a062423b3a Mon Sep 17 00:00:00 2001 From: Zach Prezkuta Date: Sat, 9 Jun 2012 13:54:21 -0600 Subject: [PATCH 1/5] update signature checking for relayables --- include/diaspora.php | 201 +++++++++++++++++++++++++++---------------- 1 file changed, 127 insertions(+), 74 deletions(-) diff --git a/include/diaspora.php b/include/diaspora.php index 1e6662f04..e91ebf442 100755 --- a/include/diaspora.php +++ b/include/diaspora.php @@ -1060,39 +1060,59 @@ function diaspora_comment($importer,$xml,$msg) { } $parent_item = $r[0]; - $author_signed_data = $guid . ';' . $parent_guid . ';' . $text . ';' . $diaspora_handle; - $author_signature = base64_decode($author_signature); + /* How Diaspora performs comment signature checking: - if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); + - If an item has been sent by the comment author to the top-level post owner to relay on + to the rest of the contacts on the top-level post, the top-level post owner should check + the author_signature, then create a parent_author_signature before relaying the comment on + - If an item has been relayed on by the top-level post owner, the contacts who receive it + check only the parent_author_signature. Basically, they trust that the top-level post + owner has already verified the authenticity of anything he/she sends out + - In either case, the signature that get checked is the signature created by the person + who sent the salmon + */ - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_comment: unable to find author details'); - return; - } - } - - if(! rsa_verify($author_signed_data,$author_signature,$key,'sha256')) { - logger('diaspora_comment: verification failed.'); - return; - } + $signed_data = $guid . ';' . $parent_guid . ';' . $text . ';' . $diaspora_handle; + $key = $msg['key']; if($parent_author_signature) { - $owner_signed_data = $guid . ';' . $parent_guid . ';' . $text . ';' . $diaspora_handle; + // If a parent_author_signature exists, then we've received the comment + // relayed from the top-level post owner. There's no need to check the + // author_signature if the parent_author_signature is valid $parent_author_signature = base64_decode($parent_author_signature); - $key = $msg['key']; + if(! rsa_verify($signed_data,$parent_author_signature,$key,'sha256')) { + logger('diaspora_comment: top-level owner verification failed.'); + return; + } + } + else { + // If there's no parent_author_signature, then we've received the comment + // from the comment creator. In that case, the person is commenting on + // our post, so he/she must be a contact of ours and his/her public key + // should be in $msg['key'] - if(! rsa_verify($owner_signed_data,$parent_author_signature,$key,'sha256')) { - logger('diaspora_comment: owner verification failed.'); +/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { + $person = $contact; + $key = $msg['key']; + } + else { + $person = find_diaspora_person_by_handle($diaspora_handle); + + if(is_array($person) && x($person,'pubkey')) + $key = $person['pubkey']; + else { + logger('diaspora_comment: unable to find author details'); + return; + } + }*/ + + $author_signature = base64_decode($author_signature); + + if(! rsa_verify($signed_data,$author_signature,$key,'sha256')) { + logger('diaspora_comment: comment author verification failed.'); return; } } @@ -1613,7 +1633,7 @@ function diaspora_like($importer,$xml,$msg) { intval($r[0]['id']), intval($importer['uid']) );*/ - // FIXME + // FIXME--actually don't unless it turns out that Diaspora does indeed send out "false" likes // send notification via proc_run() return; } @@ -1626,39 +1646,58 @@ function diaspora_like($importer,$xml,$msg) { return; } + + /* How Diaspora performs "like" signature checking: + + - If an item has been sent by the like author to the top-level post owner to relay on + to the rest of the contacts on the top-level post, the top-level post owner should check + the author_signature, then create a parent_author_signature before relaying the like on + - If an item has been relayed on by the top-level post owner, the contacts who receive it + check only the parent_author_signature. Basically, they trust that the top-level post + owner has already verified the authenticity of anything he/she sends out + - In either case, the signature that get checked is the signature created by the person + who sent the salmon + */ + $signed_data = $guid . ';' . $target_type . ';' . $parent_guid . ';' . $positive . ';' . $diaspora_handle; - - $author_signature = base64_decode($author_signature); - - if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_like: unable to find author details'); - return; - } - } - - if(! rsa_verify($signed_data,$author_signature,$key,'sha256')) { - logger('diaspora_like: verification failed.'); - return; - } + $key = $msg['key']; if($parent_author_signature) { - - //$owner_signed_data = $guid . ';' . $target_type . ';' . $parent_guid . ';' . $positive . ';' . $diaspora_handle; + // If a parent_author_signature exists, then we've received the like + // relayed from the top-level post owner. There's no need to check the + // author_signature if the parent_author_signature is valid $parent_author_signature = base64_decode($parent_author_signature); - $key = $msg['key']; - if(! rsa_verify($signed_data,$parent_author_signature,$key,'sha256')) { - logger('diaspora_like: owner verification failed.'); + logger('diaspora_like: top-level owner verification failed.'); + return; + } + } + else { + // If there's no parent_author_signature, then we've received the like + // from the like creator. In that case, the person is "like"ing + // our post, so he/she must be a contact of ours and his/her public key + // should be in $msg['key'] + +/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { + $person = $contact; + $key = $msg['key']; + } + else { + $person = find_diaspora_person_by_handle($diaspora_handle); + if(is_array($person) && x($person,'pubkey')) + $key = $person['pubkey']; + else { + logger('diaspora_like: unable to find author details'); + return; + } + }*/ + + $author_signature = base64_decode($author_signature); + + if(! rsa_verify($signed_data,$author_signature,$key,'sha256')) { + logger('diaspora_like: like creator verification failed.'); return; } } @@ -1802,40 +1841,54 @@ function diaspora_signed_retraction($importer,$xml,$msg) { $signed_data = $guid . ';' . $type ; + $key = $msg['key']; - $sig_decode = base64_decode($sig); + /* How Diaspora performs relayable_retraction signature checking: - if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); - - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_signed_retraction: unable to find author details'); - return; - } - } - - if(! rsa_verify($signed_data,$sig_decode,$key,'sha256')) { - logger('diaspora_signed_retraction: retraction-owner verification failed.' . print_r($msg,true)); - return; - } + - If an item has been sent by the item author to the top-level post owner to relay on + to the rest of the contacts on the top-level post, the top-level post owner checks + the author_signature, then creates a parent_author_signature before relaying the item on + - If an item has been relayed on by the top-level post owner, the contacts who receive it + check only the parent_author_signature. Basically, they trust that the top-level post + owner has already verified the authenticity of anything he/she sends out + - In either case, the signature that get checked is the signature created by the person + who sent the salmon + */ if($parent_author_signature) { + $parent_author_signature = base64_decode($parent_author_signature); - $key = $msg['key']; - if(! rsa_verify($signed_data,$parent_author_signature,$key,'sha256')) { - logger('diaspora_signed_retraction: failed to verify person relaying the retraction (e.g. owner of a post relaying a retracted comment'); + logger('diaspora_signed_retraction: top-level post owner verification failed'); return; } } + else { + +/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { + $person = $contact; + $key = $msg['key']; + } + else { + $person = find_diaspora_person_by_handle($diaspora_handle); + + if(is_array($person) && x($person,'pubkey')) + $key = $person['pubkey']; + else { + logger('diaspora_signed_retraction: unable to find author details'); + return; + } + }*/ + + $sig_decode = base64_decode($sig); + + if(! rsa_verify($signed_data,$sig_decode,$key,'sha256')) { + logger('diaspora_signed_retraction: retraction owner verification failed.' . print_r($msg,true)); + return; + } + } if($type === 'StatusMessage' || $type === 'Comment' || $type === 'Like') { $r = q("select * from item where guid = '%s' and uid = %d and not file like '%%[%%' limit 1", From 0946c4a375838072d8c54b348191004e1002416e Mon Sep 17 00:00:00 2001 From: Zach Prezkuta Date: Sun, 10 Jun 2012 08:41:23 -0600 Subject: [PATCH 2/5] still need the original author information for comments and likes --- include/diaspora.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/include/diaspora.php b/include/diaspora.php index e91ebf442..ecf1fd7a9 100755 --- a/include/diaspora.php +++ b/include/diaspora.php @@ -1119,6 +1119,18 @@ function diaspora_comment($importer,$xml,$msg) { // Phew! Everything checks out. Now create an item. + // Find the original comment author information + if(strcasecmp($diaspora_handle,$msg['author']) == 0) + $person = $contact; + else { + $person = find_diaspora_person_by_handle($diaspora_handle); + + if(! is_array($person)) { + logger('diaspora_comment: unable to find author details'); + return; + } + } + $body = diaspora2bb($text); $message_id = $diaspora_handle . ':' . $guid; @@ -1704,6 +1716,18 @@ function diaspora_like($importer,$xml,$msg) { // Phew! Everything checks out. Now create an item. + // Find the original like author information + if(strcasecmp($diaspora_handle,$msg['author']) == 0) + $person = $contact; + else { + $person = find_diaspora_person_by_handle($diaspora_handle); + + if(! is_array($person)) { + logger('diaspora_like: unable to find author details'); + return; + } + } + $uri = $diaspora_handle . ':' . $guid; $activity = ACTIVITY_LIKE; From 0ff86e28bb04c8c1cf08b2e22ae60971f0e90f1c Mon Sep 17 00:00:00 2001 From: Zach Prezkuta Date: Mon, 11 Jun 2012 20:21:35 -0600 Subject: [PATCH 3/5] fix small typo for logging command in api.php --- include/api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/api.php b/include/api.php index 9f2a5fb3b..855e4efa3 100644 --- a/include/api.php +++ b/include/api.php @@ -245,7 +245,7 @@ } - logger('api_user: ' . $extra_query . ' ' , $user); + logger('api_user: ' . $extra_query . ', user: ' . $user); // user info $uinfo = q("SELECT *, `contact`.`id` as `cid` FROM `contact` WHERE 1 From 577cfdd314259ea490c1bef89ff847bbc8e168c2 Mon Sep 17 00:00:00 2001 From: Zach Prezkuta Date: Sat, 16 Jun 2012 10:41:25 -0600 Subject: [PATCH 4/5] some cleanup of obsolete code --- include/diaspora.php | 37 ++++++------------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/include/diaspora.php b/include/diaspora.php index ecf1fd7a9..5be09a7ac 100755 --- a/include/diaspora.php +++ b/include/diaspora.php @@ -1094,21 +1094,6 @@ function diaspora_comment($importer,$xml,$msg) { // our post, so he/she must be a contact of ours and his/her public key // should be in $msg['key'] -/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); - - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_comment: unable to find author details'); - return; - } - }*/ - $author_signature = base64_decode($author_signature); if(! rsa_verify($signed_data,$author_signature,$key,'sha256')) { @@ -1119,7 +1104,9 @@ function diaspora_comment($importer,$xml,$msg) { // Phew! Everything checks out. Now create an item. - // Find the original comment author information + // Find the original comment author information. + // We need this to make sure we display the comment author + // information (name and avatar) correctly. if(strcasecmp($diaspora_handle,$msg['author']) == 0) $person = $contact; else { @@ -1692,20 +1679,6 @@ function diaspora_like($importer,$xml,$msg) { // our post, so he/she must be a contact of ours and his/her public key // should be in $msg['key'] -/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_like: unable to find author details'); - return; - } - }*/ - $author_signature = base64_decode($author_signature); if(! rsa_verify($signed_data,$author_signature,$key,'sha256')) { @@ -1716,7 +1689,9 @@ function diaspora_like($importer,$xml,$msg) { // Phew! Everything checks out. Now create an item. - // Find the original like author information + // Find the original comment author information. + // We need this to make sure we display the comment author + // information (name and avatar) correctly. if(strcasecmp($diaspora_handle,$msg['author']) == 0) $person = $contact; else { From 3132ed297991433dd387f7fa826a27b6d67675c5 Mon Sep 17 00:00:00 2001 From: Zach Prezkuta Date: Sat, 16 Jun 2012 10:45:26 -0600 Subject: [PATCH 5/5] more cleanup --- include/diaspora.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/include/diaspora.php b/include/diaspora.php index 5be09a7ac..2be266e8a 100755 --- a/include/diaspora.php +++ b/include/diaspora.php @@ -1866,21 +1866,6 @@ function diaspora_signed_retraction($importer,$xml,$msg) { } else { -/* if(strcasecmp($diaspora_handle,$msg['author']) == 0) { - $person = $contact; - $key = $msg['key']; - } - else { - $person = find_diaspora_person_by_handle($diaspora_handle); - - if(is_array($person) && x($person,'pubkey')) - $key = $person['pubkey']; - else { - logger('diaspora_signed_retraction: unable to find author details'); - return; - } - }*/ - $sig_decode = base64_decode($sig); if(! rsa_verify($signed_data,$sig_decode,$key,'sha256')) {