about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorbptato <nincsnevem662@gmail.com>2024-04-02 17:09:15 +0200
committerbptato <nincsnevem662@gmail.com>2024-04-02 17:19:47 +0200
commit481894279a86064ddfb1afddeae944f2e4bffb77 (patch)
tree6d5145e9819411c79bb9c8a4b7f196f8f5e855b3
parentdacba5fde35f07c86175db2e5ca22b6992681037 (diff)
downloadchawan-481894279a86064ddfb1afddeae944f2e4bffb77.tar.gz
loader: constant time key comparison
GCC seems to generate something that strongly resembles a constant time
comparison, so I guess this should be good enough.
-rw-r--r--doc/architecture.md8
-rw-r--r--src/loader/loader.nim14
2 files changed, 17 insertions, 5 deletions
diff --git a/doc/architecture.md b/doc/architecture.md
index 71a99478..9b62daf7 100644
--- a/doc/architecture.md
+++ b/doc/architecture.md
@@ -104,10 +104,10 @@ following steps:
   rewind the source.
 
 The loader process distinguishes between clients (i.e the main process or
-buffers) through client keys. Note that this does not defend against rogue
-clients; the key comparison is susceptible to timing attacks. (TODO: it should
-not be.) It does however help us block further requests from buffers that have
-been discarded by the pager.
+buffers) through client keys. In theory this should help against rogue clients,
+though in practice it is still trivial to crash the loader as a client. It also
+helps us block further requests from buffers that have been discarded by the
+pager, but still haven't found out yet that their life time has ended.
 
 ### Buffer
 
diff --git a/src/loader/loader.nim b/src/loader/loader.nim
index 9f64b440..37da64cb 100644
--- a/src/loader/loader.nim
+++ b/src/loader/loader.nim
@@ -613,6 +613,18 @@ proc resume(ctx: LoaderContext; stream: SocketStream; client: ClientData;
       output.registered = true
       ctx.selector.registerHandle(output.ostream.fd, {Write}, 0)
 
+proc equalsConstantTime(a, b: ClientKey): bool =
+  static:
+    doAssert a.len == b.len
+  {.push boundChecks:off, overflowChecks:off.}
+  var i {.volatile.} = 0
+  var res {.volatile.} = 0u8
+  while i < a.len:
+    res = res or (a[i] xor b[i])
+    inc i
+  {.pop.}
+  return res == 0
+
 proc acceptConnection(ctx: LoaderContext) =
   let stream = ctx.ssock.acceptSocketStream()
   try:
@@ -626,7 +638,7 @@ proc acceptConnection(ctx: LoaderContext) =
         stream.sclose()
         return
       let client = ctx.clientData[myPid]
-      if client.key != key:
+      if not client.key.equalsConstantTime(key):
         # ditto
         stream.sclose()
         return