0

We're trying to implement an authenticated symmetric encryption in PHP, which works most of the times, but occasionally fails, and we do not really understand why. The code is:

// Encryption of $secret_code, e.g. integer = 152
$secret_code = 152;

$nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES);

// This is just an example, keys are generated in a more secure way, returning a base64
// (Compatible with sodium_crypto_secretbox according to https://www.php.net/manual/de/function.sodium-crypto-secretbox.php)
$key = sodium_base642bin(
'Jdjkfhuehrfkjshdfkjhskjfoshdfishf',
SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING
);

$cipher = sodium_crypto_secretbox(
random_bytes(8) . $secret_code
$nonce,
$key
);

sodium_memzero($key);

$ciphertext_base64 = sodium_bin2base64(
$nonce . $cipher,
SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING
);

sodium_memzero($nonce);
sodium_memzero($cipher);

// Decryption then sometimes yields inconsistent values for $secret_code, e.g. 154

$ciphertext = sodium_base642bin(
$ciphertext_base64,
SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING
);

$nonce = mb_substr(
$ciphertext,
0,
SODIUM_CRYPTO_SECRETBOX_NONCEBYTES,
'8bit'
);

$cipher = mb_substr(
$ciphertext,
SODIUM_CRYPTO_SECRETBOX_NONCEBYTES,
null,
'8bit'
);

$secret_code_string = sodium_crypto_secretbox_open(
$cipher,
$nonce,
$key
);

sodium_memzero($key);
sodium_memzero($nonce);
sodium_memzero($cipher);

$secret_code = (int) mb_substr(
$secret_code_string,
8,
null,
'8bit'
);

We suspect that the issue lies somewhere in the splitting of the string on an 8bit base and base64 string encoding; we're assuming some data is getting truncated occasionally, resulting in the decryption not to fail, but instead yielding the wrong $secret_code. Any idea?

DevelJoe
  • 856
  • 1
  • 10
  • 24
  • Can you explain the need for splitting in the first place? – Chris Haas May 20 '23 at 15:35
  • To include a random string in the message to be encrypted, as that adds some randomness to the message to be encrypted, to my knowldege? Or would you simply omit the random_bytes(8) in the generation of the message to be encrypted alltogether? My thought was that it would make the encryption more secure..? – DevelJoe May 20 '23 at 15:48
  • 2
    When it comes to security, specifically encryption, my recommendation is to follow the docs verbatim, don’t try to make it “better”. At best, tweaks just add unnecessary steps, but at worst they either break (like this) or actually downgrade the encryption. Unfortunately the Sodium docs are lacking (IMHO) compared to other more mature parts of PHP, however [secretbox](https://www.php.net/manual/en/function.sodium-crypto-secretbox.php) has a very simple working example with not many steps. – Chris Haas May 20 '23 at 15:57
  • Yeah I‘ve read quite some blog articles saying that adding random bytes to the message to be encrypted makes the outcome less predictable. But it‘s probably better to keep it simple and rotate the symmetric key, I guess, if I understand you properly. Cheers! Still curious though where and why my code is breaking the proper functioning. – DevelJoe May 20 '23 at 16:57
  • 3
    I feel fairly confident in saying that if libsodium had problems where the outcome was “predictable”, either they’d fix the code immediately, or it would be scrapped as a flawed design. Further, any claims to the predictability I’d also assume would be backed by published papers. I guess symmetric encryption has its general flaws key-sharing and lack of authentication, but I don’t think this solves that. – Chris Haas May 20 '23 at 17:13
  • 1
    To your last part about understanding why it isn’t working, I’d think you could just add some tests inline. I still don’t really understand what it is doing, but you should be able to do some length checks and such after splitting to make sure your inputs match your outputs – Chris Haas May 20 '23 at 17:16
  • 2
    With what probability should the problem occur? I cannot reproduce it: https://paiza.io/projects/aUZUcjwFjzVHbrWpNP40vw – Topaco May 20 '23 at 17:18
  • 1
    It's breaking because you're slapping 8 random bytes onto the front of the plaintext, and then trying to use `mb_substr()` to pare them off on the other side. The problem being that two or more of those 8 random bytes might comprise a valid-looking UTF-8 codepoint, then `mb_substr()` will consider those a single unit, throwing off the split. Use regular `substr()` or, better yet, don't pad random noise into your plaintext in the first place. – Sammitch May 21 '23 at 02:19
  • 1
    @Sammitch - Was my first thought too and is reproducible with `utf8` as encoding, but not with `8bit` which is what the OP uses. Even with 100000 runs, the problem does not occur, s. link above. – Topaco May 21 '23 at 05:52
  • Thanks to all, will defo ommit the random(8) stuff! – DevelJoe May 21 '23 at 07:58
  • After this discussion I'm getting the feeling that it's probably also better to get rid of the logic which prepends the nonce to the encryption result. The [link](https://www.php.net/manual/en/function.sodium-crypto-secretbox.php) of the docs does not do so; but I wonder: If you're encrypting something, forward it to the client, and then get it back for decryption. From where are you supposed to get the nonce for decryption, if it's not prepended to the encrypted string? Would it be better practice to store a nonce used in encryption in the DB, and recreate it after every decryption? – DevelJoe Jun 04 '23 at 19:46

0 Answers0