On Thu, Aug 15, 2024 at 10:14 AM Heikki Linnakangas <hlinn...@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-57d379aa0...@intel.com/
