starekrow / lockbox

Encrypted storage with built-in key management facilities
MIT License
95 stars 6 forks source link

remove dependency on php serialize #31

Open KJLJon opened 6 years ago

KJLJon commented 6 years ago

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

https://github.com/starekrow/lockbox/blob/db3dbc1779344a9357b4512da8b55363b1c89a8e/src/Secret.php#L128

https://github.com/starekrow/lockbox/blob/db3dbc1779344a9357b4512da8b55363b1c89a8e/src/Secret.php#L157

starekrow commented 6 years ago

I used serialize advisedly, because JSON is utterly useless for binary strings unless you pre-encode them and secret tokens are fairly likely to contain such strings. Eventually it should be migrated to msgpack or some other less language-specific binary encoding. That's one reason for the encoding type prefix (that "p" there).

Though actually, I wouldn't mind trying to pre-flight a conversion to JSON ("j" prefix) and fall back to serialize only if the value contains non-UTF8 strings.

alexmanno commented 6 years ago

We can use strategy pattern (https://github.com/alexmanno/DesignPatternsPHP/tree/master/Behavioral/Strategy)

For example:

interface SerializerInterface { 
public function serialize();
public function unseiralize();
}

class PHPSerializer implements SerializerInterface { /* implementation */ }

class JsonSerializer implements SerializerInterface { /* implementation */ }

class Secret
{
     // ....
     public function __constructor(SerializerInterface $serializer, $value, $_import = null)
    {
     // ...
     }
     //...
}