starekrow/lockbox

Encrypted storage with built-in key management facilities

lockbox's Issues

remove dependency on php serialize

This is less of an issue with the KeyDrop (#27)
but if KeyDrop isn't use it would be nice to create this code in a different language and use the same keys.

Right now it is using php serialize which can probably be replaced with json_encode / json_decode

lockbox/src/Secret.php

Line 128 in db3dbc1

$plaintext = "p" . serialize($value);

lockbox/src/Secret.php

Line 157 in db3dbc1

$this->value = unserialize(substr($value, 1));

Fatal error: Uncaught Exception: Unknown algorithm

Hi

I'm trying to use lockbox but it fails on my computer (localhost, Windows 10).
I'm using the code below (a copy/paste from your readme.md) and I receive a fatal error when calling the Lock() method.

I'm under PHP 7.2.0 (it's working fine under PHP 5.6.25 or 7.0.10). I've just install PHP 7.2.1, same result : NOK.

Can you give advices please ? Any installation problem on my computer ?

Thanks.

define ('DS', DIRECTORY_SEPARATOR);

use starekrow\Lockbox\CryptoKey;
use starekrow\Lockbox\Secret;
use starekrow\Lockbox\Vault;

$lib = 'some_dir';

// Include Lockbox
require_once $lib."CryptoCore.php";
require_once $lib."CryptoCoreLoader.php";
require_once $lib."CryptoCoreFailed.php";
require_once $lib."CryptoCoreBuiltin.php";
require_once $lib."CryptoCoreOpenssl.php";
require_once $lib."Crypto.php";
require_once $lib."CryptoKey.php";
require_once $lib."Secret.php";
require_once $lib."Vault.php";

// CryptoKey defaults to AES-128-CBC encryption with a random key
$key = new CryptoKey();
$message = "You can't see me.";
echo $key->Lock( $message ).'<hr/>';

$key = new CryptoKey( "ILikeCheese", null, "AES-256-ECB" );
$no_see_um = $key->Lock( "This text is safe." );
echo $no_see_um.'<hr/>';
$see_um = $key->Unlock( $no_see_um );
echo $see_um.'<hr/>';

Full error message :

Fatal error: Uncaught Exception: Unknown algorithm in 
libs\lockbox\CryptoCoreBuiltin.php:86 Stack trace: 
#0 libs\lockbox\Crypto.php(84): starekrow\Lockbox\CryptoCoreBuiltin->ivlen('AES-128-CBC') 
#1 libs\lockbox\CryptoKey.php(129): starekrow\Lockbox\Crypto::ivlen('AES-128-CBC') 
#2 test.php(28): starekrow\Lockbox\CryptoKey->lock('You can't see m...') 
#3 {main} thrown in libs\lockbox\CryptoCoreBuiltin.php on line 86

Secure distribution of secrets (KeyDrop)

Getting distribution right is apparently hard. I've started a branch for a new module called KeyDrop that will handle that. It will supply all of the guts for a client/server model for secret distribution, with an offline master keyring.

• You designate a server to store the master keys and vaults and write a trivial wrapper to get HTTP queries into and out of the KeyDrop class for it. This is the KeyDrop server.
• The KeyDrop server has all the master keys and all the vaults for all the clients, however the master keys are encrypted, and the KeyDrop server is never given the information needed to decrypt them. Compromising the KeyDrop server gains an attacker nothing at all.
• Each KeyDrop client is assigned an ID and a client key. The client key is stored locally on the client, and its only purpose is to decrypt the master key. The master key is never stored on the client.
• When the client is running, if it doesn't have a copy of the master key in RAM, it asks the KeyDrop server for the master key. The KeyDrop server returns an encrypted master key, which the client must decode. For efficiency, the master key should then be tucked away somewhere (NOT on the filesystem).
• The client can also ask the KeyDrop server for a list of updates to that client's vault.
• A mechanism is provided to push updates to client vaults out to the KeyDrop server. You should provide your own mechanism to force your client to request an update.
• Another mechanism is provided to organize all the client, vault and shared keys in an encrypted master keyring. This obviously should be kept offline.

I think this actually solves the entire question of how to securely configure a server; each KeyDrop client only needs a couple of items - the client ID and client key - to automatically and securely pull, store and update when needed all the other secrets assigned to that client.

Automated Testing on PR

I think some sort of CI should be ran on pull requests
Travis CI is free for open source projects
https://docs.travis-ci.com/user/languages/php/

PHPDoc

Change comments format to phpdoc for better IDE support :)
https://github.com/phpDocumentor/fig-standards/blob/master/proposed/phpdoc.md

Raw encryption

For improved interoperability, CryptoKey should have some facility for returning and accepting raw binary ciphertext and IVs.

For example, update the signature of Lock() to accept a second argument $raw. If true, return an array of [ "iv" => "...", "data" => "..." ]. Likewise, Unlock() could accept such an array.

php version bump

I see you bumped the version to php 7 because of phpunit. I quick tested and it looks like I can get phpunit 4.x running with 2 minor changes to the tests (which supports php 5.5).

Although, I still like the idea of supporting 7+ (or 5.6+) because of PHP security patching support, but I also think the version shouldn't be bumped just because of the testing framework.

PHP support: http://php.net/supported-versions.php

see KJLJon@d8be026 for 5.5+ fix.

@starekrow what are your thoughts? I can send a PR for the commit listed above if you still want to support php 5.5+

Use PhpUnit instead of custom test framework

Hi guys,
We should use a well-tested library for tests (such as PhpUnit) instead of custom TestFramework

tag a version

I think a version should be tagged so it will be easier to install via composer. I recommend following symver

I am open to either one of these versions (depending on what your thoughts of the stability of the package is):

• 0.1.0 - if you think there still might be major changed / API changes
• 1.0.0 - if you are are pretty set on the packages public API.

Add test coverage for all crypto drivers

The CryptoKey tests offer an amount of indirect coverage, but there should be some explicit exercise of Failed, Builtin and Openssl (and Sodium when it arrives)

adopt psr-2 code style / formatting

I think psr-2 (which includes adopting psr-1) should be adopted.
http://www.php-fig.org/psr/psr-2/

The downside it is changes almost every line (since spaces are used instead of tabs).

make hmac algo configurable

I think the HMAC hashing algorithm should be configurable and included in the export function.
For example, if I wanted to use sha512 instead of sha256

polyfill for random_bytes

This is a potential solution to support random bytes. (it is also released under MIT license, and can be required with composer)

see https://github.com/paragonie/random_compat

In regards to

lockbox/src/CryptoCoreBuiltin.php

Lines 70 to 80 in 4af2d72

	public function random( $count )
	{
	if (function_exists( "random_bytes" )) {
	return random_bytes( $count );
	}
	// TODO: windows: COM stuff, linux: /dev/urandom
	if (function_exists( "openssl_random_pseudo_bytes" )) {
	return openssl_random_pseudo_bytes( $count );
	}
	throw new Exception( "No good source of randomness found" );
	}

abstract filesystem calls

I think the file system calls in Vault should be abstracted into an interface so the vault could be stored in other locations and someone could leverage packages like flysystem.

Of course default it with the file system for ease of use.

namespace starekrow\lockbox;

interface FileSystemInterface {
    /**
     * gets the content of a file
     * @param string $file
     * @return string
     */
    public function get($file);

    /**
     * puts content into a file
     * @param string $file
     * @param string $content
     * @return bool if it was successful
     */
    public function put($file, $content);

    /**
     * checks if the file exists
     * @param string $file
     * @return bool if it exists
     */
    public function has($file);
}

namespace starekrow\lockbox;

class LocalFileSystem implements FileSystemInterface {
    /**
     * @inheritdoc
     */
    public function get($file)
    {
        return @file_get_contents($file);
    }

    /**
     * @inheritdoc
     */
    public function put($file, $content)
    {
        return @file_put_contents($file, $content);
    }

    /**
     * @inheritdoc
     */
    public function has($file)
    {
        return file_exists($file);
    }
}

libsodium support

It would be nice to have optional support for libsodium as an alternative to the openssl extension, since libsodium is moving into core.

This could be hacked in place, or done by isolating the crypto use in CryptoKey. Either way might complicate cipher selection (needs research).

separate key for hmac and encryption

I am not a security expert, but I typically read that the HMAC and Encryption keys should be different.

A unique key can be derived from a hkdf function. If your targeting >= php 7 then you can use hash_hkdf()

What versions of PHP are you looking to support?