]> git.immae.eu Git - github/wallabag/wallabag.git/commitdiff
Crypt site credential password
authorJeremy Benoist <jeremy.benoist@gmail.com>
Sun, 11 Jun 2017 21:05:19 +0000 (23:05 +0200)
committerJeremy Benoist <jeremy.benoist@gmail.com>
Tue, 20 Jun 2017 14:03:35 +0000 (16:03 +0200)
15 files changed:
.gitignore
app/DoctrineMigrations/Version20170501115751.php
app/config/wallabag.yml
composer.json
src/Wallabag/CoreBundle/Command/InstallCommand.php
src/Wallabag/CoreBundle/Controller/SiteCredentialController.php
src/Wallabag/CoreBundle/DependencyInjection/Configuration.php
src/Wallabag/CoreBundle/DependencyInjection/WallabagCoreExtension.php
src/Wallabag/CoreBundle/Entity/SiteCredential.php
src/Wallabag/CoreBundle/Helper/CryptoProxy.php [new file with mode: 0644]
src/Wallabag/CoreBundle/Repository/SiteCredentialRepository.php
src/Wallabag/CoreBundle/Resources/config/services.yml
tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php [new file with mode: 0644]

index 709f78cf248708646b86c8bae907360722184e54..8fd762fa8abccbc5bafac85d246e88de829dc483 100644 (file)
@@ -56,3 +56,4 @@ app/Resources/build/
 # Test-generated files
 admin-export.json
 specialexport.json
+/data/site-credentials-secret-key.txt
index 846a87b5c462744c1e5d663c3084fdfc32bc88ff..2597f1ffd2dcbf48cd2e8bd77681a84a85ed302a 100644 (file)
@@ -39,7 +39,7 @@ class Version20170501115751 extends AbstractMigration implements ContainerAwareI
         $table->addColumn('user_id', 'integer');
         $table->addColumn('host', 'string', ['length' => 255]);
         $table->addColumn('username', 'string', ['length' => 255]);
-        $table->addColumn('password', 'string', ['length' => 255]);
+        $table->addColumn('password', 'text');
         $table->addColumn('createdAt', 'datetime');
         $table->addIndex(['user_id'], 'idx_user');
         $table->setPrimaryKey(['id']);
index 51b7e4e3034fcb9bf344d9e378ad2b6e5e284db1..b45934e4b6f31bf71f470dee54e4a998240c8a48 100644 (file)
@@ -26,6 +26,7 @@ wallabag_core:
     fetching_error_message: |
         wallabag can't retrieve contents for this article. Please <a href="http://doc.wallabag.org/en/user/errors_during_fetching.html#how-can-i-help-to-fix-that">troubleshoot this issue</a>.
     api_limit_mass_actions: 10
+    encryption_key_path: "%kernel.root_dir%/../data/site-credentials-secret-key.txt"
     default_internal_settings:
         -
             name: share_public
index 0a170c168db3162a3eb4d082d7f2956400c5faaa..7428f688c01ccc35482de84f2d818f2e5f2afdb9 100644 (file)
@@ -73,7 +73,7 @@
         "kphoen/rulerz-bundle": "~0.13",
         "guzzlehttp/guzzle": "^5.3.1",
         "doctrine/doctrine-migrations-bundle": "^1.0",
-        "paragonie/random_compat": "~1.0",
+        "paragonie/random_compat": "~2.0",
         "craue/config-bundle": "~2.0",
         "mnapoli/piwik-twig-extension": "^1.0",
         "ocramius/proxy-manager": "1.*",
@@ -83,7 +83,8 @@
         "javibravo/simpleue": "^1.0",
         "symfony/dom-crawler": "^3.1",
         "friendsofsymfony/jsrouting-bundle": "^1.6",
-        "bdunogier/guzzle-site-authenticator": "^1.0.0@dev"
+        "bdunogier/guzzle-site-authenticator": "^1.0.0@dev",
+        "defuse/php-encryption": "^2.1"
     },
     "require-dev": {
         "doctrine/doctrine-fixtures-bundle": "~2.2",
index 0f119377fe54d041a73e8df2ec4407628c28695d..eb725a59a90486a89a3d67345cc2dc2b9703a48d 100644 (file)
@@ -313,6 +313,8 @@ class InstallCommand extends ContainerAwareCommand
 
         $this
             ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]);
+
+        return $this;
     }
 
     /**
index dc8e723da590f92a3d87f2924db5a75ef6fab388..0bacafb79a24ec6c7d46a0d35d6484571f4e5b44 100644 (file)
@@ -45,6 +45,8 @@ class SiteCredentialController extends Controller
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
+            $credential->setPassword($this->get('wallabag_core.helper.crypto_proxy')->crypt($credential->getPassword()));
+
             $em = $this->getDoctrine()->getManager();
             $em->persist($credential);
             $em->flush($credential);
index 33df92d3c8dc202671951d04e1f66ec9cb9b258f..a9791f6be2e3f08f558a3261c584f7b7f315c2fd 100644 (file)
@@ -63,6 +63,8 @@ class Configuration implements ConfigurationInterface
                         ->end()
                     ->end()
                 ->end()
+                ->scalarNode('encryption_key_path')
+                ->end()
             ->end()
         ;
 
index b4d8a3866a59db84a4b993c59a4498c3fb9957f4..532ce238645532f9ffdf7c6777fd96293aa9788e 100644 (file)
@@ -29,6 +29,7 @@ class WallabagCoreExtension extends Extension
         $container->setParameter('wallabag_core.fetching_error_message_title', $config['fetching_error_message_title']);
         $container->setParameter('wallabag_core.api_limit_mass_actions', $config['api_limit_mass_actions']);
         $container->setParameter('wallabag_core.default_internal_settings', $config['default_internal_settings']);
+        $container->setParameter('wallabag_core.site_credentials.encryption_key_path', $config['encryption_key_path']);
 
         $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
         $loader->load('services.yml');
index 85ee07d4a0917e4871d5cea0307079eaea4be7ba..732d95066a3a167ad9f99f1d3b75518f5750ad22 100644 (file)
@@ -46,8 +46,7 @@ class SiteCredential
      * @var string
      *
      * @Assert\NotBlank()
-     * @Assert\Length(max=255)
-     * @ORM\Column(name="password", type="string", length=255)
+     * @ORM\Column(name="password", type="text")
      */
     private $password;
 
diff --git a/src/Wallabag/CoreBundle/Helper/CryptoProxy.php b/src/Wallabag/CoreBundle/Helper/CryptoProxy.php
new file mode 100644 (file)
index 0000000..d0a9b85
--- /dev/null
@@ -0,0 +1,86 @@
+<?php
+
+namespace Wallabag\CoreBundle\Helper;
+
+use Psr\Log\LoggerInterface;
+use Defuse\Crypto\Key;
+use Defuse\Crypto\Crypto;
+use Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException;
+
+/**
+ * This is a proxy to crypt and decrypt password used by SiteCredential entity.
+ * BTW, It might be re-use for sth else.
+ */
+class CryptoProxy
+{
+    private $logger;
+    private $encryptionKey;
+
+    public function __construct($encryptionKeyPath, LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+
+        if (!file_exists($encryptionKeyPath)) {
+            $key = Key::createNewRandomKey();
+
+            file_put_contents($encryptionKeyPath, $key->saveToAsciiSafeString());
+            chmod($encryptionKeyPath, 0600);
+        }
+
+        $this->encryptionKey = file_get_contents($encryptionKeyPath);
+    }
+
+    /**
+     * Ensure the given value will be crypted.
+     *
+     * @param string $secretValue Secret valye to crypt
+     *
+     * @return string
+     */
+    public function crypt($secretValue)
+    {
+        $this->logger->debug('Crypto: crypting value: '.$this->mask($secretValue));
+
+        return Crypto::encrypt($secretValue, $this->loadKey());
+    }
+
+    /**
+     * Ensure the given crypted value will be decrypted.
+     *
+     * @param string $cryptedValue The value to be decrypted
+     *
+     * @return string
+     */
+    public function decrypt($cryptedValue)
+    {
+        $this->logger->debug('Crypto: decrypting value: '.$this->mask($cryptedValue));
+
+        try {
+            return Crypto::decrypt($cryptedValue, $this->loadKey());
+        } catch (WrongKeyOrModifiedCiphertextException $e) {
+            throw new \RuntimeException('Decrypt fail: '.$e->getMessage());
+        }
+    }
+
+    /**
+     * Load the private key.
+     *
+     * @return string
+     */
+    private function loadKey()
+    {
+        return Key::loadFromAsciiSafeString($this->encryptionKey);
+    }
+
+    /**
+     * Keep first and last character and put some stars in between.
+     *
+     * @param string $value Value to mask
+     *
+     * @return string
+     */
+    private function mask($value)
+    {
+        return $value[0].'*****'.$value[strlen($value) - 1];
+    }
+}
index 316ecc75020e5400ef272a7e97664a50b6aa8910..6f904f0ae86b2c8cc7e8236378e7e2f92578243a 100644 (file)
@@ -2,11 +2,20 @@
 
 namespace Wallabag\CoreBundle\Repository;
 
+use Wallabag\CoreBundle\Helper\CryptoProxy;
+
 /**
  * SiteCredentialRepository.
  */
 class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository
 {
+    private $cryptoProxy;
+
+    public function setCrypto(CryptoProxy $cryptoProxy)
+    {
+        $this->cryptoProxy = $cryptoProxy;
+    }
+
     /**
      * Retrieve one username/password for the given host and userId.
      *
@@ -17,12 +26,21 @@ class SiteCredentialRepository extends \Doctrine\ORM\EntityRepository
      */
     public function findOneByHostAndUser($host, $userId)
     {
-        return $this->createQueryBuilder('s')
+        $res = $this->createQueryBuilder('s')
             ->select('s.username', 's.password')
             ->where('s.host = :hostname')->setParameter('hostname', $host)
             ->andWhere('s.user = :userId')->setParameter('userId', $userId)
             ->setMaxResults(1)
             ->getQuery()
             ->getOneOrNullResult();
+
+        if (null === $res) {
+            return;
+        }
+
+        // decrypt password before returning it
+        $res['password'] = $this->cryptoProxy->decrypt($res['password']);
+
+        return $res;
     }
 }
index 09bc77fe9be2506aa354410f14839b026ae3c04b..e09b0f1859ceb972a6e3561c9e79de5a96c3d81a 100644 (file)
@@ -126,6 +126,8 @@ services:
         factory: [ "@doctrine.orm.default_entity_manager", getRepository ]
         arguments:
             - WallabagCoreBundle:SiteCredential
+        calls:
+            - [ setCrypto, [ "@wallabag_core.helper.crypto_proxy" ] ]
 
     wallabag_core.helper.entries_export:
         class: Wallabag\CoreBundle\Helper\EntriesExport
@@ -208,3 +210,9 @@ services:
 
     wallabag_core.entry.download_images.client:
         class: GuzzleHttp\Client
+
+    wallabag_core.helper.crypto_proxy:
+        class: Wallabag\CoreBundle\Helper\CryptoProxy
+        arguments:
+            - "%wallabag_core.site_credentials.encryption_key_path%"
+            - "@logger"
index 104f60f175efb58db53e1f5a5bc287c1b15cf26f..803806853883ad6cc00eb979c97b4450cb5a9cf6 100644 (file)
@@ -1341,7 +1341,7 @@ class EntryControllerTest extends WallabagCoreTestCase
         $credential = new SiteCredential($user);
         $credential->setHost('monde-diplomatique.fr');
         $credential->setUsername('foo');
-        $credential->setPassword('bar');
+        $credential->setPassword($client->getContainer()->get('wallabag_core.helper.crypto_proxy')->crypt('bar'));
 
         $em->persist($credential);
         $em->flush();
index 1e1e8989b76417e9220db8b95094db7626103deb..b0c81e7b0cdf67290f28239d98ac40a879f85e3f 100644 (file)
@@ -6,12 +6,11 @@ use Monolog\Handler\TestHandler;
 use Monolog\Logger;
 use BD\GuzzleSiteAuthenticator\SiteConfig\SiteConfig;
 use Graby\SiteConfig\SiteConfig as GrabySiteConfig;
-use PHPUnit_Framework_TestCase;
 use Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
 
-class GrabySiteConfigBuilderTest extends PHPUnit_Framework_TestCase
+class GrabySiteConfigBuilderTest extends \PHPUnit_Framework_TestCase
 {
     /** @var \Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder */
     protected $builder;
diff --git a/tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php b/tests/Wallabag/CoreBundle/Helper/CryptoProxyTest.php
new file mode 100644 (file)
index 0000000..cede869
--- /dev/null
@@ -0,0 +1,40 @@
+<?php
+
+namespace Tests\Wallabag\CoreBundle\Helper;
+
+use Psr\Log\NullLogger;
+use Monolog\Logger;
+use Monolog\Handler\TestHandler;
+use Wallabag\CoreBundle\Helper\CryptoProxy;
+
+class CryptoProxyTest extends \PHPUnit_Framework_TestCase
+{
+    public function testCrypto()
+    {
+        $logHandler = new TestHandler();
+        $logger = new Logger('test', [$logHandler]);
+
+        $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', $logger);
+        $crypted = $crypto->crypt('test');
+        $decrypted = $crypto->decrypt($crypted);
+
+        $this->assertSame('test', $decrypted);
+
+        $records = $logHandler->getRecords();
+        $this->assertCount(2, $records);
+        $this->assertContains('Crypto: crypting value', $records[0]['message']);
+        $this->assertContains('Crypto: decrypting value', $records[1]['message']);
+    }
+
+    /**
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Decrypt fail
+     *
+     * @return [type] [description]
+     */
+    public function testDecryptBadValue()
+    {
+        $crypto = new CryptoProxy(sys_get_temp_dir().'/'.uniqid('', true).'.txt', new NullLogger());
+        $crypto->decrypt('badvalue');
+    }
+}