crypto_kdf_derive_from_key ctx argument is very easy to get wrong

[goblin]

The fact that the ctx argument to crypto_kdf_derive_from_key has to be exactly 8 bytes long is very tricky, prone to getting wrong, and under-documented. It would be much better to use something like a \0-delimited string, and limit its maximum length in documentation, especially given that the personal data that actually goes to blake2b is padded with 0s to 16 bytes anyway.

This is not very clearly explained in the documentation, and all the examples are "magically" exactly 8-bytes long (i.e. "__auth__", "username" and the like). Someone might not notice that all the strings are of this length and that the requirement is that they must be exactly that. A quick search on github shows at least one such occurrence where someone uses too short a string "rxtx" as ctx: https://github.com/matteomonti/leaf/blob/57aba45036ef58ba7a008308cd9638d65ddaf66c/src/drop/crypto/keyexchanger.cpp#L31

Using a shorter string results in code that unexpectedly produces different results depending on how it was compiled, and may lead to all sorts of incompatibilities, undecryptable messages and other nastiness. Here's what I mean, the same program compiled with GCC produces a different key than when compiled with clang:

goblin@host:/tmp/gh$ cat > ctxpad.c
#include <sodium.h>

int main()
{
	uint8_t key1[crypto_kdf_KEYBYTES];
	uint8_t subkey[32];

	if(sodium_init())
		return 1;
	
	sodium_memzero(key1, crypto_kdf_KEYBYTES);
	crypto_kdf_derive_from_key(subkey, 32, 0, "ctx", key1);

	char buf[256];
	sodium_bin2hex(buf, 256, subkey, 32);
	printf("%s\n", buf);

	return 0;
}
goblin@host:/tmp/gh$ gcc --version
gcc (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

goblin@host:/tmp/gh$ gcc -o ctxpad-gcc -lsodium ctxpad.c 
goblin@host:/tmp/gh$ clang-8 --version
clang version 8.0.1-3~bpo10+1 (tags/RELEASE_801/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
goblin@host:/tmp/gh$ clang-8 -o ctxpad-clang -lsodium ctxpad.c 
goblin@host:/tmp/gh$ ./ctxpad-gcc 
6addb8a03e09835b27d08013f2623159c8b1a38b8c7e58cfbac53db7324de07e
goblin@host:/tmp/gh$ ./ctxpad-clang 
b9be066ad4d5000172340dbaaa9927c6c631b09ceb4d25c91592531b989e0266
goblin@host:/tmp/gh$ 

[goblin]

I have now recompiled libsodium with --enable-debug, and it's clear in GDB that for this example program, the personal argument contains garbage after the 0-byte following the "ctx" string, and the garbage is different when compiled with gcc than when compiled with clang.

In GCC:

GNU gdb (Debian 8.2.1-2+b3) 8.2.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./ctxpad-gcc...done.
(gdb) break crypto_generichash_blake2b_salt_personal
Function "crypto_generichash_blake2b_salt_personal" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (crypto_generichash_blake2b_salt_personal) pending.
(gdb) run
Starting program: /tmp/gh/ctxpad-gcc 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, crypto_generichash_blake2b_salt_personal (out=0x7fffffffe0d0 "", outlen=32, in=in@entry=0x0, inlen=inlen@entry=0, key=0x7fffffffe0f0 "", keylen=keylen@entry=32, salt=0x7fffffffdf90 "", 
    personal=0x7fffffffdf80 "ctx") at crypto_generichash/blake2b/ref/generichash_blake2b.c:32
32	{
(gdb) x/16c personal
0x7fffffffdf80:	99 'c'	116 't'	120 'x'	0 '\000'	1 '\001'	27 '\033'	3 '\003'	59 ';'
0x7fffffffdf88:	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'

In Clang:

GNU gdb (Debian 8.2.1-2+b3) 8.2.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./ctxpad-clang...done.
(gdb) break crypto_generichash_blake2b_salt_personal
Function "crypto_generichash_blake2b_salt_personal" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (crypto_generichash_blake2b_salt_personal) pending.
(gdb) run
Starting program: /tmp/gh/ctxpad-clang 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, crypto_generichash_blake2b_salt_personal (out=0x7fffffffe0c0 "\001", outlen=32, in=in@entry=0x0, inlen=inlen@entry=0, key=0x7fffffffe0e0 "", keylen=keylen@entry=32, salt=0x7fffffffdf60 "", 
    personal=0x7fffffffdf50 "ctx") at crypto_generichash/blake2b/ref/generichash_blake2b.c:32
32	{
(gdb) x/16c personal
0x7fffffffdf50:	99 'c'	116 't'	120 'x'	0 '\000'	37 '%'	115 's'	10 '\n'	0 '\000'
0x7fffffffdf58:	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'	0 '\000'

[jedisct1]

This is the purpose of the crypto_kdf_CONTEXTBYTES constant.

Contexts have to match that size.

[jedisct1]

This has been clarified in the documentation, thanks!