Skip to content

Commit

Permalink
Hopefully put the keyserver issues to a rest.
Browse files Browse the repository at this point in the history
Fingers crossed.
Wow, this is a mess. There is a keyserver software out there thinks it's a good idea to wrap the output in some blinky HTML, even if providing the mr (machine readable!) option. Instead it has some arbitrary criteria to decide whether to add this wrapper or not. The only criteria we have any influence on is a missing User-Agent header which is supposed to indicate an API client instead of a browser. All of this you can only find out if you have access to the server software. At least it's open source.
Next comes the various HTTP backends that you need to convince to not send a User-Agent header. CURL allows to set it to NULL, PECL_HTTP, which uses CURL, not. Instead you have to use an empty string, which again you only can find out when ... yada, yada. I didn't even bother to test the fopen backend.
  • Loading branch information
yunosh committed Mar 22, 2016
1 parent 471b54b commit 794c3d9
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 17 deletions.
19 changes: 14 additions & 5 deletions framework/Crypt/lib/Horde/Crypt/Pgp/Keyserver.php
Expand Up @@ -61,9 +61,18 @@ class Horde_Crypt_Pgp_Keyserver
public function __construct($pgp, array $params = array())
{
$this->_pgp = $pgp;
$this->_http = (isset($params['http']) && ($params['http'] instanceof Horde_Http_Client))
? $params['http']
: new Horde_Http_Client();
if (isset($params['http'])) {
if (!($params['http'] instanceof Horde_Http_Client)) {
throw new InvalidArgumentException('Argument is not a Horde_Http_Client instance');
}
$this->_http = $params['http'];
} else {
$this->_http = new Horde_Http_Client();
}
/* There is a broken key server software that returns HTML content
* instead of plain text on arbitrary criteria. A User-Agent header is
* one of those. */
$this->_http->{'request.userAgent'} = '';
$this->_keyserver = isset($params['keyserver'])
? $params['keyserver']
: 'http://pool.sks-keyservers.net';
Expand Down Expand Up @@ -98,7 +107,7 @@ public function get($keyid)
return substr($start, 0, $length);
}

throw new Horde_Crypt_Exception(Horde_Crypt_Translation::t("Could not obtain public key from the keyserver.") . ' Response: ' . $output);
throw new Horde_Crypt_Exception(Horde_Crypt_Translation::t("Could not obtain public key from the keyserver."));
}

/**
Expand Down Expand Up @@ -174,7 +183,7 @@ public function getKeyId($address)

if (!$output) {
throw new Horde_Crypt_Exception(
Horde_Crypt_Translation::t("Could not obtain public key from the keyserver.") . ' Response: ' . var_export($response, true)
Horde_Crypt_Translation::t("Could not obtain public key from the keyserver.")
);
}

Expand Down
33 changes: 27 additions & 6 deletions framework/Crypt/test/Horde/Crypt/PgpKeyserverTest.php
Expand Up @@ -12,26 +12,26 @@
class Horde_Crypt_PgpKeyserverTest extends Horde_Test_Case
{
protected $_ks;
protected $_gnupg;

protected function setUp()
{
$c = self::getConfig('CRYPTPGP_TEST_CONFIG', __DIR__);
$gnupg = isset($c['gnupg'])
$this->_gnupg = isset($c['gnupg'])
? $c['gnupg']
: '/usr/bin/gpg';

if (!is_executable($gnupg)) {
if (!is_executable($this->_gnupg)) {
$this->markTestSkipped(sprintf(
'GPG binary not found at %s.',
$gnupg
$this->_gnupg
));
}

$this->_ks = new Horde_Crypt_Pgp_Keyserver(
Horde_Crypt::factory('Pgp', array(
'program' => $gnupg
)),
array('keyserver' => 'http://ha.pool.sks-keyservers.net')
'program' => $this->_gnupg
))
);
}

Expand Down Expand Up @@ -64,4 +64,25 @@ public function testKeyserverRetrieveByEmail()
}
}

public function testBrokenKeyserver()
{
$ks = new Horde_Crypt_Pgp_Keyserver(
Horde_Crypt::factory('Pgp', array(
'program' => $this->_gnupg
)),
array('keyserver' => 'http://pgp.key-server.io')
);
try {
$this->assertEquals(
'4DE5B969',
$ks->getKeyID('jan@horde.org')
);
} catch (Horde_Crypt_Exception $e) {
if (strpos($e->getMessage(), 'Operation timed out') === 0) {
$this->markTestSkipped($e->getMessage());
} else {
throw $e;
}
}
}
}
14 changes: 11 additions & 3 deletions framework/Pgp/lib/Horde/Pgp/Keyserver.php
Expand Up @@ -53,9 +53,17 @@ class Horde_Pgp_Keyserver
*/
public function __construct(array $params = array())
{
$this->_http = (isset($params['http']) && ($params['http'] instanceof Horde_Http_Client))
? $params['http']
: new Horde_Http_Client();
if (isset($params['http'])) {
if (!($params['http'] instanceof Horde_Http_Client)) {
throw new InvalidArgumentException('Argument is not a Horde_Http_Client instance');
}
$this->_http = $params['http'];
} else {
$this->_http = new Horde_Http_Client();
}
/* There is a broken key server software that returns HTML content
* instead of plain text on arbitrary criteria. A User-Agent header is
* one of those. */
$this->_http->{'request.userAgent'} = '';
$this->_keyserver = isset($params['keyserver'])
? $params['keyserver']
Expand Down
23 changes: 20 additions & 3 deletions framework/Pgp/test/Horde/Pgp/KeyserverTest.php
Expand Up @@ -29,9 +29,7 @@ class Horde_Pgp_KeyserverTest extends Horde_Test_Case

protected function setUp()
{
$this->_ks = new Horde_Pgp_Keyserver(array(
'keyserver' => 'http://ha.pool.sks-keyservers.net:11371'
));
$this->_ks = new Horde_Pgp_Keyserver();
}

/**
Expand Down Expand Up @@ -73,6 +71,25 @@ public function testKeyserverRetrieveByEmail($email, $id)
}
}

/**
* @dataProvider keyserverRetrieveByEmailProvider
*/
public function testBrokenKeyserver($email, $id)
{
$ks = new Horde_Pgp_Keyserver(array(
'keyserver' => 'http://pgp.key-server.io:11371'
));
try {
$this->_checkKey($ks->getKeyByEmail($email), $id);
} catch (Horde_Pgp_Exception $e) {
if ($e->getPrevious() instanceof Horde_Http_Exception) {
$this->markTestSkipped($e->getPrevious()->getMessage());
} else {
throw $e;
}
}
}

public function keyserverRetrieveByEmailProvider()
{
return array(
Expand Down

0 comments on commit 794c3d9

Please sign in to comment.