e59b702 · skrepka · git.iko.soy

git.iko.soy/skrepka

Fix five security and correctness bugs: poll cursor re-delivery, ackedIds eviction duplicate delivery, backgroundPrune missing federation retry, safeCompare timing leak, and verifyMaintenanceAuth timing side-channel

e59b702iko7 months ago

server.knot+41−16

compare with master

unifiedsplit

@@ -327,8 +327,10 @@ verifyFederationAuth = \authorization ->

327

328verifyMaintenanceAuth = \authorization ->

329 let token = extractToken authorization

330+ let fedResult = safeCompare token (if federationSecret == "" then "x" else federationSecret)

331+ let maintResult = safeCompare token (if maintenanceSecret == "" then "x" else maintenanceSecret)

330− let fedOk = not (federationSecret == "") && safeCompare token federationSecret

332+ let fedOk = not (federationSecret == "") && fedResult

331− let maintOk = not (maintenanceSecret == "") && safeCompare token maintenanceSecret

333+ let maintOk = not (maintenanceSecret == "") && maintResult

332 fedOk || maintOk

334 fedOk || maintOk

333

335

334clampTtl = \ttl -> if ttl < 1 then 1 else if ttl > maxPresenceTtl then maxPresenceTtl else ttl

336clampTtl = \ttl -> if ttl < 1 then 1 else if ttl > maxPresenceTtl then maxPresenceTtl else ttl

@@ -351,13 +353,13 @@ evictOldestAckedIds = \items n ->

351 else acc) {size: 0, heap: [], heapMax: -1} items).heap

353 else acc) {size: 0, heap: [], heapMax: -1} items).heap

352 in diff items toEvict

354 in diff items toEvict

353

355

354−safeCompareLoop = \s1 s2 result ->

356+safeCompareLoop = \s1 s2 n result ->

355− if s1 == "" && s2 == ""

357+ if n <= 0

356− then result

358+ then result + (if s1 == "" then 0 else 1) + (if s2 == "" then 0 else 1)

357− else safeCompareLoop (drop 1 s1) (drop 1 s2) (result + (if take 1 s1 == take 1 s2 then 0 else 1))

359+ else safeCompareLoop (drop 1 s1) (drop 1 s2) (n - 1) (result + (if take 1 s1 == take 1 s2 then 0 else 1))

358

360

359safeCompare = \a b ->

361safeCompare = \a b ->

360− safeCompareLoop a b 0 + (if a == "" then 1 else 0) + (if b == "" then 1 else 0) == 0

362+ safeCompareLoop a b maxTokenLen 0 + (if a == "" then 1 else 0) + (if b == "" then 1 else 0) == 0

361

363

362sendToEach = \servers action ->

364sendToEach = \servers action ->

363 if count servers == 0

365 if count servers == 0

@@ -500,8 +502,10 @@ pollOnce = \token cursor ->

500 t <- now

502 t <- now

501 sessions <- *sessions

503 sessions <- *sessions

502 messages <- *messages

504 messages <- *messages

505+ seqCounter <- *seqCounter

503 let session = lookupSession sessions token t

506 let session = lookupSession sessions token t

507+ let cursorCeil = if seqCounter > t then seqCounter else t

504− let clampedCursor = if cursor < 0 then 0 else if cursor > t then t else cursor

508+ let clampedCursor = if cursor < 0 then 0 else if cursor > cursorCeil then cursorCeil else cursor

505 let matchingMsgs = case session of

509 let matchingMsgs = case session of

506 Nothing {} -> []

510 Nothing {} -> []

507 Just {value: s} -> do

511 Just {value: s} -> do

@@ -808,11 +812,11 @@ serve = \req -> case req of

808 else union acc [m.originServer]) [] fedMessages

812 else union acc [m.originServer]) [] fedMessages

809 respond {ok: result.ok}

813 respond {ok: result.ok}

810 if result.ok && count originServers > 0 && not (federationSecret == "")

814 if result.ok && count originServers > 0 && not (federationSecret == "")

811− then sendToEach originServers (\server ->

815+ then do

812− fetch (federationScheme ++ server) (FederationAck {

816+ let pendingAcks = do

813− authorization: "Bearer " ++ federationSecret,

817+ m <- fedMessages

814− messageIds: (do m <- fedMessages; where m.originServer == server; yield m.messageId),

818+ yield {messageId: m.messageId, toKey: m.toKey, ackedAt: t, originServer: m.originServer}

815− originServer: server, fromServer: serverHostN}))

819+ sendAcksAndClean pendingAcks originServers

816 else yield {}

820 else yield {}

817 broadcastOfflineGossip t result.peerServers result.expiredLocal

821 broadcastOfflineGossip t result.peerServers result.expiredLocal

818

822

@@ -1208,10 +1212,31 @@ backgroundPrune = do

1208 sleep 30000

1212 sleep 30000

1209 t <- now

1213 t <- now

1210 catch (do

1214 catch (do

1211− pruneResult <- atomic do

1215+ result <- atomic do

1212 set *lastPrunedAt = t

1216 set *lastPrunedAt = t

1213− pruneExpired t

1217+ pr <- pruneExpired t

1218+ forwards <- *forwards

1219+ let pendingForwards = do

1220+ f <- forwards

1221+ yield {toServer: f.toServer, toKey: f.toKey, fromKey: f.fromKey, encryptedBlob: f.encryptedBlob, timestamp: f.timestamp, messageId: f.messageId}

1222+ ackedIds <- *ackedIds

1223+ let pendingAcks = do

1224+ a <- ackedIds

1225+ where not (a.originServer == "")

1226+ where not (a.originServer == serverHostN)

1227+ yield a

1228+ yield {expiredLocal: pr.expiredLocal, peerServers: pr.peerServers, pendingForwards: pendingForwards, pendingAcks: pendingAcks}

1214− broadcastOfflineGossip t pruneResult.peerServers pruneResult.expiredLocal) (\err ->

1229+ broadcastOfflineGossip t result.peerServers result.expiredLocal

1230+ if not (federationSecret == "") && count result.pendingForwards > 0

1231+ then retryPendingForwards result.pendingForwards

1232+ else yield {}

1233+ if not (federationSecret == "") && count result.pendingAcks > 0

1234+ then do

1235+ let ackOriginServers = fold (\acc a ->

1236+ if count (do s <- acc; where s == a.originServer; yield s) > 0 then acc

1237+ else union acc [a.originServer]) [] result.pendingAcks

1238+ sendAcksAndClean result.pendingAcks ackOriginServers

1239+ else yield {}) (\err ->

1215 println ("background prune: " ++ show err))

1240 println ("background prune: " ++ show err))

1216 backgroundPrune

1241 backgroundPrune

1217

1242