Module Name: src
Committed By: riastradh
Date: Fri Jul 26 18:27:48 UTC 2024
Modified Files:
src/sys/external/isc/libsodium/include: crypto_verify_16.h
Log Message:
sys/crypto/sodium: Fill out crypto_verify_16 stub.
Without this change, libsodium silently accepts forgeries.
This one's a doozy, and it's a sobering reminder that:
(a) wg(4) is still experimental (only user of libsodium in kernel;
both are available only through default-off optional modules).
(b) Known-answer test vectors are critical, including negative tests
(test that forgeries are rejected), and must be mandatory for all
new crypto code -- and should be added to old crypto code too.
(c) Crypto code must also have self-tests that run in the same
environment, not just the same code in a different build or test
environment -- the libsodium code itself is fine, but we built it
differently and need to exercise it differently from upstream's
automatic tests.
It's my fault for not catching this earlier. What happened is:
1. ozaki-r@ adapted libsodium to build in the kernel with various
glue to build code meant for standard userland C, like errno.h and
string.h.
2. Since libsodium's crypto_verify_16.c uses various SIMD intrinsics
on various architectures, it couldn't be used directly in the
kernel build, because -- at the time -- we hadn't wired up any
header files for SIMD intrinsics or any runtime support for saving
and restoring SIMD state appropriately in the kernel.
3. ozaki-r@ put a similar glue header file crypto_verify_16.h to
override libsodium's, with a stub to be implemented later, and
presumably forgot to remind me about it.
4. I missed the stub in crypto_verify_16.h when reviewing the
libsodium import and wg(4) code because it was in the same
directory as various other simple glue code that I deemed
low-risk.
(I did make one change to that glue code, to replace cprng_fast by
cprng_strong, but I suspect I found that by searching for
cprng_fast users rather than by reviewing this code.)
5. I broke my own rule about always having known-answer test vectors
for crypto code because I figured libsodium was well-enough
exercised that we could skimp on it for now, and my focus was more
on the state machine and synchronization logic than on the crypto.
6. I had not yet written known-answer test vectors for the
higher-level wg(4) protocol messages.
Before we can remove the `experimental' tag from wg(4) we will need
to (among other things):
i. Write self-tests for the rest of (what we use from) libsodium.
ii. Write extensive known-answer test vectors for all the wg(4)
protocol messages (and ideally state machine transitions).
iii. Write self-tests for a reasonable subset of the wg(4) KATs.
iv. Review all of the libsodium glue code I neglected to review.
PR kern/58468
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 \
src/sys/external/isc/libsodium/include/crypto_verify_16.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/external/isc/libsodium/include/crypto_verify_16.h
diff -u src/sys/external/isc/libsodium/include/crypto_verify_16.h:1.1 src/sys/external/isc/libsodium/include/crypto_verify_16.h:1.2
--- src/sys/external/isc/libsodium/include/crypto_verify_16.h:1.1 Thu Aug 20 21:20:16 2020
+++ src/sys/external/isc/libsodium/include/crypto_verify_16.h Fri Jul 26 18:27:48 2024
@@ -1,9 +1,17 @@
/* This overwrites dist/src/libsodium/include/sodium/crypto_verify_16.h */
-/* dummy */
+#include <lib/libkern/libkern.h>
+
static inline int
crypto_verify_16(const unsigned char *x, const unsigned char *y)
{
- return 0;
+ /*
+ * crypto_verify_16 must return 0 if equal, -1 if not.
+ *
+ * consttime_memequal returns 1 if equal, 0 if not.
+ *
+ * Hence we simply subtract one.
+ */
+ return consttime_memequal(x, y, 16) - 1;
}
