Page MenuHomePhorge
Create Task
Maniphest T1784

kolab_2fa: yubico doesn't use config in /etc/roundcubemail/kolab_2fa.inc.php
Closed, ResolvedPublic
Actions

Description

I'm running my own validation server (privacyidea) and tried to use it with

[root@kolab-35 ~]# rpm -q roundcubemail-plugins-kolab
roundcubemail-plugins-kolab-3.3-40.2.el7.kolab_wf.noarch

I couldn't save the yubico token, because the verification always failed.
Looking into the logs of my validation server showed that it wasn't even
connected. So I started to trace through kolab_2fa's code.

I have the following configuration in /etc/roundcubemail/kolab_2fa.inc.php:

$config['kolab_2fa_drivers'] = array('totp','yubikey');
$config['kolab_2fa_yubikey'] = array(
    'clientid' => '23453',
    'apikey' => '[redacted]=',
    'hosts'  => array('athene.jochen.org'),
);

Bute kolab_2fa always used what was defined in Yubico.php:

class Yubikey extends Base
{
    public $method = 'yubikey';

    protected $config = array(
        'clientid' => '42',
        'apikey'   => 'FOOBAR=',
        'hosts'    => null,
    );

The following patch fixes it for me:

--- plugins/kolab_2fa/lib/Kolab2FA/Driver/Yubikey.php.orig      2016-10-24 20:35:28.059469605 +0200
+++ plugins/kolab_2fa/lib/Kolab2FA/Driver/Yubikey.php   2016-10-24 20:42:08.281474612 +0200
@@ -27,11 +27,7 @@
 {
     public $method = 'yubikey';
 
-    protected $config = array(
-        'clientid' => '42',
-        'apikey'   => 'FOOBAR=',
-        'hosts'    => null,
-    );
+    protected $config = array();
 
     protected $backend;
 
@@ -42,6 +38,9 @@
     {

Comments are welcome.

Details

Ticket Type
Task

Related Objects

Event Timeline

jh23453 created this task.Oct 26 2016, 5:49 AM
jh23453 added projects: Authentication, Roundcube Kolab Plugins .Oct 26 2016, 5:53 AM
jh23453 updated the task description. (Show Details)Oct 26 2016, 5:57 AM
bruederli subscribed.Oct 29 2016, 5:17 PM

The configuration options should already make their way into the $config member of the Yubikey class. They are read in kolab_2fa::get_driver(), passed through Kolab2FA\Driver\Base::factory() all the way to Kolab2FA\Driver\Base::init() where they are merged with $this->config. It's to be investigated why the following line in Base.php doesn't work as expected:

$this->config = array_merge($this->config, $config);

But since the defaults defined in Yubikey.php are useless anyway, one could simply remove the entire protected $config = definition here. It's already defined in the Base class.

jh23453 added a comment.Oct 30 2016, 12:04 AM

Hm, there seems to be some confusion between method (driver to use) and id (yubikey:<hexcode>). the plugin tried to load config['kolab_2fa_yubikey:<hexcode>'] which obvously fails. The following patch (against Winterfell) seems to fix it for me - but I didn't test HOTP or TOTP token. And already added yubikeys had to be removed and readded.

--- kolab_2fa/kolab_2fa.php.orig        2016-06-30 10:20:21.000000000 +0200
+++ kolab_2fa/kolab_2fa.php     2016-10-29 23:50:38.797453986 +0200
@@ -282,9 +282,10 @@
     /**
      * Load driver class for the given method
      */
-    public function get_driver($method)
+    public function get_driver($id)
     {
         $rcmail = rcmail::get_instance();
+        list($method) = explode(':', $id);
 
         if ($this->drivers[$method]) {
             return $this->drivers[$method];
@@ -753,4 +754,5 @@
         return $valid;
     }
 
-}
\ Kein Zeilenumbruch am Dateiende.
+}
+
--- kolab_2fa/lib/Kolab2FA/Driver/Base.php.orig 2016-06-30 10:20:21.000000000 +0200
+++ kolab_2fa/lib/Kolab2FA/Driver/Base.php      2016-10-29 23:42:50.256440164 +0200
@@ -73,7 +73,7 @@
 
         $cls = $classmap[strtolower($method)];
         if ($cls && class_exists($cls)) {
-            return new $cls($config, $id);
+            return new $cls($config, $method);
         }
 
         throw new Exception("Unknown 2FA driver '$method'");
@@ -86,7 +86,7 @@
     {
         $this->init($config);
 
-        if (!empty($id) && $id != $this->method) {
+        if (!empty($id) && $id != $this->id) {
             $this->id = $id;
         }
         else { // generate random ID
--- kolab_2fa/lib/Kolab2FA/Driver/Yubikey.php.orig      2016-06-30 10:20:21.000000000 +0200
+++ kolab_2fa/lib/Kolab2FA/Driver/Yubikey.php   2016-10-29 23:44:33.442443167 +0200
@@ -27,12 +27,6 @@
 {
     public $method = 'yubikey';
 
-    protected $config = array(
-        'clientid' => '42',
-        'apikey'   => 'FOOBAR=',
-        'hosts'    => null,
-    );
-
     protected $backend;
 
     /**
jh23453 added a comment.Oct 30 2016, 12:30 AM

A test with TOTP was successful.

bruederli mentioned this in D242: Fix driver initialization with config properties for the right method (T1784).Oct 30 2016, 2:54 PM
bruederli added a comment.Oct 30 2016, 2:56 PM

Better fix submitted in D242. Waiting for approval

machniak mentioned this in rRPKdb2d3bc800fe: Fix driver initialization with config properties for the right method (T1784).Nov 9 2016, 9:56 AM
machniak closed this task as Resolved.Nov 9 2016, 9:58 AM
machniak claimed this task.