013d6d4 · skrepka · git.iko.soy

git.iko.soy/skrepka

Fix five security and correctness bugs in server.knot: AckMessages eviction gated on auth, safeCompareLoop constant-time for secrets >256 chars, broadcastOfflineGossip TTL floor for short-lived entries, verifyFederationAuth constant-time when disabled, evictOldestAckedIds/pollOnce union dedup handling

013d6d4iko7 months ago

server.knot+56−43

compare with master

unifiedsplit

@@ -33,6 +33,7 @@ maxSessionsPerKey = 10

33challengeExpiry = 60000 -- 1 minute in milliseconds

34sessionExpiry = 3600000 -- 1 hour in milliseconds

35onlineGossipTtl = 3600 -- 1 hour in seconds

36+offlineGossipMinTtl = 60 -- minimum TTL for offline gossip events in seconds

37-- ============================================

38-- ============================================

38-- Types

39-- Types

@@ -323,7 +324,10 @@ extractToken = \authHeader ->

323 else ""

324 else ""

324

325

325verifyFederationAuth = \authorization ->

326verifyFederationAuth = \authorization ->

326− not (federationSecret == "") && safeCompare (extractToken authorization) federationSecret

327+ let token = extractToken authorization

328+ let result = safeCompare token (if federationSecret == "" then "x" else federationSecret)

329+ let secretSet = if federationSecret == "" then 0 else 1

330+ (if result then 1 else 0) * secretSet > 0

327

331

328verifyMaintenanceAuth = \authorization ->

332verifyMaintenanceAuth = \authorization ->

329 let token = extractToken authorization

333 let token = extractToken authorization

@@ -344,7 +348,10 @@ evictOldestAckedIds = \items n ->

344 if acc.size < n

348 if acc.size < n

345 then

349 then

346 let newHeap = union acc.heap [a]

350 let newHeap = union acc.heap [a]

351+ let newSize = count newHeap

352+ if newSize > acc.size

347− {size: count newHeap, heap: newHeap, heapMax: if a.ackedAt > acc.heapMax then a.ackedAt else acc.heapMax}

353+ then {size: newSize, heap: newHeap, heapMax: if a.ackedAt > acc.heapMax then a.ackedAt else acc.heapMax}

354+ else acc

348 else if a.ackedAt < acc.heapMax

355 else if a.ackedAt < acc.heapMax

349 then

356 then

350 let removed = fold (\r x ->

357 let removed = fold (\r x ->

@@ -358,8 +365,8 @@ evictOldestAckedIds = \items n ->

358 in diff items toEvict

365 in diff items toEvict

359

366

360safeCompareLoop = \s1 s2 n result ->

367safeCompareLoop = \s1 s2 n result ->

361− if n <= 0 && s1 == "" && s2 == ""

368+ if n <= 0

362− then result

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

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

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

364

371

365safeCompare = \a b ->

372safeCompare = \a b ->

@@ -457,7 +464,7 @@ broadcastOfflineGossip = \t peerServers expiredLocal -> do

457 if count (do a <- acc; where a.pubkey == p.pubkey; where a.expiresAt >= p.expiresAt; yield a) > 0 then acc

464 if count (do a <- acc; where a.pubkey == p.pubkey; where a.expiresAt >= p.expiresAt; yield a) > 0 then acc

458 else union

465 else union

459 (do a <- acc; where not (a.pubkey == p.pubkey); yield a)

466 (do a <- acc; where not (a.pubkey == p.pubkey); yield a)

460− [{pubkey: p.pubkey, ttl: clampTtl ((p.expiresAt - p.timestamp) / 1000), expiresAt: p.expiresAt}]) [] expiredLocal

467+ [{pubkey: p.pubkey, ttl: (let rawTtl = (p.expiresAt - p.timestamp) / 1000 in clampTtl (if rawTtl < offlineGossipMinTtl then offlineGossipMinTtl else rawTtl)), expiresAt: p.expiresAt}]) [] expiredLocal

461 if not (federationSecret == "") && count expiredEntries > 0 && count peerServers > 0

468 if not (federationSecret == "") && count expiredEntries > 0 && count peerServers > 0

462 then do

469 then do

463 let gossipEvents = do

470 let gossipEvents = do

@@ -535,7 +542,10 @@ pollOnce = \token cursor ->

535 if acc.size < maxPollEvents

542 if acc.size < maxPollEvents

536 then

543 then

537 let newHeap = union acc.heap [m]

544 let newHeap = union acc.heap [m]

545+ let newSize = count newHeap

546+ if newSize > acc.size

538− {size: count newHeap, heap: newHeap, heapMax: if m.receivedAt > acc.heapMax then m.receivedAt else acc.heapMax}

547+ then {size: newSize, heap: newHeap, heapMax: if m.receivedAt > acc.heapMax then m.receivedAt else acc.heapMax}

548+ else acc

539 else if m.receivedAt < acc.heapMax

549 else if m.receivedAt < acc.heapMax

540 then

550 then

541 let removed = fold (\r x ->

551 let removed = fold (\r x ->

@@ -857,45 +867,48 @@ serve = \req -> case req of

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

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

858 sendAcksAndClean pendingAcks originServers

868 sendAcksAndClean pendingAcks originServers

859 else yield {}

869 else yield {}

860− evictedEntries <- atomic do

870+ if result.ok

861− ackedIds <- *ackedIds

862− let excessCount = count ackedIds - maxAckedIds

863− if excessCount <= 0

864− then yield []

865− else do

866− let afterEviction = evictOldestAckedIds ackedIds excessCount

867− let evicted = diff ackedIds afterEviction

868− set *ackedIds = afterEviction

869− yield evicted

870− let evictedToRetry = do

871− a <- evictedEntries

872− where count (do m <- fedMessages; where m.messageId == a.messageId; where m.toKey == a.toKey; yield m) == 0

873− yield a

874− if count evictedToRetry > 0 && not (federationSecret == "")

875 then do

871 then do

876− let evictedOriginServers = fold (\acc a ->

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

878− else union acc [a.originServer]) [] evictedToRetry

879− sendToEach evictedOriginServers (\server ->

880− catch (do

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

882− authorization: "Bearer " ++ federationSecret,

883− messageIds: (do a <- evictedToRetry; where a.originServer == server; yield a.messageId),

884− originServer: server, fromServer: serverHostN})

885− yield {}) (\err -> do

886− println ("federation: error sending evicted ack to " ++ server ++ ": " ++ show err)

887− evicted <- atomic do

872+ evictedEntries <- atomic do

888− ackedIds <- *ackedIds

873+ ackedIds <- *ackedIds

889− let reinsert = do a <- evictedToRetry; where a.originServer == server; yield a

874+ let excessCount = count ackedIds - maxAckedIds

890− let combined = union ackedIds reinsert

875+ if excessCount <= 0

891− let excess = count combined - maxAckedIds

876+ then yield []

892− let afterEviction = if excess <= 0 then combined

877+ else do

893− else evictOldestAckedIds combined excess

878+ let afterEviction = evictOldestAckedIds ackedIds excessCount

879+ let evicted = diff ackedIds afterEviction

894 set *ackedIds = afterEviction

880 set *ackedIds = afterEviction

881+ yield evicted

882+ let evictedToRetry = do

883+ a <- evictedEntries

884+ where count (do m <- fedMessages; where m.messageId == a.messageId; where m.toKey == a.toKey; yield m) == 0

885+ yield a

886+ if count evictedToRetry > 0 && not (federationSecret == "")

887+ then do

888+ let evictedOriginServers = fold (\acc a ->

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

890+ else union acc [a.originServer]) [] evictedToRetry

891+ sendToEach evictedOriginServers (\server ->

892+ catch (do

893+ fetch (federationScheme ++ server) (FederationAck {

894+ authorization: "Bearer " ++ federationSecret,

895+ messageIds: (do a <- evictedToRetry; where a.originServer == server; yield a.messageId),

896+ originServer: server, fromServer: serverHostN})

897+ yield {}) (\err -> do

898+ println ("federation: error sending evicted ack to " ++ server ++ ": " ++ show err)

899+ evicted <- atomic do

900+ ackedIds <- *ackedIds

901+ let reinsert = do a <- evictedToRetry; where a.originServer == server; yield a

902+ let combined = union ackedIds reinsert

903+ let excess = count combined - maxAckedIds

904+ let afterEviction = if excess <= 0 then combined

905+ else evictOldestAckedIds combined excess

906+ set *ackedIds = afterEviction

895− yield (count combined - count afterEviction)

907+ yield (count combined - count afterEviction)

896− if evicted > 0

908+ if evicted > 0

897− then println ("federation: evicted " ++ show evicted ++ " old ack entries to reinsert failed acks for " ++ server)

909+ then println ("federation: evicted " ++ show evicted ++ " old ack entries to reinsert failed acks for " ++ server)

898− else yield {}))

910+ else yield {}))

911+ else yield {}

899 else yield {}

912 else yield {}

900 broadcastOfflineGossip t result.peerServers result.expiredLocal

913 broadcastOfflineGossip t result.peerServers result.expiredLocal

901

914