On Thu, Aug 15, 2024 at 10:14 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> I'm back to working on the main patch here, to make cancellation keys
> longer. New rebased version attached, with all the FIXMEs and TODOs from
> the earlier version fixed. There was a lot of bitrot, too.
I have a couple of questions/comments separate from the protocol changes:
Has there been any work/discussion around not sending the cancel key
in plaintext from psql? It's not a prerequisite or anything (the
longer length is a clear improvement either way), but it seems odd
that this longer "secret" is still just going to be exposed on the
wire when you press Ctrl+C.
> The first patch now introduces timingsafe_bcmp(), a function borrowed
> from OpenBSD to perform a constant-time comparison. There's a configure
> check to use the function from the OS if it's available, and includes a
> copy of OpenBSD's implementation otherwise. Similar functions exist with
> different names in OpenSSL (CRYPTO_memcmp) and NetBSD
> (consttime_memequal), but it's a pretty simple function so I don't think
> we need to work too hard to pick up those other native implementations.
One advantage to using other implementations is that _they're_ on the
hook for keeping constant-time guarantees, which is getting trickier
due to weird architectural optimizations [1]. CRYPTO_memcmp() has
almost the same implementation as 0001 here, except they made the
decision to mark the pointers volatile, and they also provide
hand-crafted assembly versions. This patch has OpenBSD's version, but
they've also turned on data-independent timing by default across their
ARM64 processors [2]. And Intel may require the same tweak, but it
doesn't look like userspace has access to that setting yet, and the
kernel thread [3] appears to have just withered...
For the cancel key implementation in particular, I agree with you that
it's probably not a serious problem. But if other security code starts
using timingsafe_bcmp() then it might be something to be concerned
about. Are there any platform/architecture combos that don't provide a
native timingsafe_bcmp() *and* need a DIT bit for safety?
Thanks,
--Jacob
[1] https://github.com/golang/go/issues/66450
[2] https://github.com/openbsd/src/commit/cf1440f11c
[3] https://lore.kernel.org/lkml/851920c5-31c9-ddd9-3e2d-57d379aa0671@intel.com/
