From f7d588757b89af72d69c3c6a128edb1eb1ec4813 Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Mon, 4 Sep 2023 17:19:59 +0200 Subject: [PATCH 1/2] Verify the handoff count on handoff-receive This is important to protect against replay attacks, a handoff receive should only be valid once. Once it's been accepted, a replay of that same handoff-receive shouldn't succeed as the handoff count has already been seen. --- goblins/ocapn/captp.rkt | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/goblins/ocapn/captp.rkt b/goblins/ocapn/captp.rkt index f0b6489..a3acb1f 100644 --- a/goblins/ocapn/captp.rkt +++ b/goblins/ocapn/captp.rkt @@ -25,6 +25,7 @@ (submod "../core.rkt" for-captp) "../message.rkt" "../vat.rkt" + "../actor-lib/cell.rkt" "../actor-lib/methods.rkt" "../actor-lib/swappable.rkt" "../actor-lib/common.rkt" @@ -937,10 +938,8 @@ ;; made in this session to prevent replay attacks. ;; every time a *request* is made, this should be incremented. (define our-handoff-count 0) - ;; TODO TODO TODO: We need to make use of this and also check the - ;; session listed on the receive certificate to prevent a replay - ;; attack - (define remote-handoff-count 0) + (define remote-handoff-count + (spawn ^cell -1)) (define handoff-pubkey (pk-key->public-only-key handoff-privkey)) @@ -1135,7 +1134,7 @@ ;; with the gifter, but with the receiver) (? bytes? _handoff-session) (? bytes? _handoff-session-side) - (? integer? _this-handoff-count) + (? integer? this-handoff-count) signed-handoff-give)) (? gcrypt-signature? receive-sig)) signed-handoff-receive) @@ -1149,9 +1148,18 @@ (define give-recipient-key (datum->pk-key give-recipient-encoded-key 'rkt-public)) - (and (give-handoff-legit? signed-handoff-give) - (pk-verify give-recipient-key encoded-handoff-receive - (gcrypt->racket/signature receive-sig)))) + (define valid-handoff? + (and (give-handoff-legit? signed-handoff-give) + (> this-handoff-count ($ remote-handoff-count)) + (pk-verify give-recipient-key encoded-handoff-receive + (gcrypt->racket/signature receive-sig)))) + + ;; If it is in fact a valid handoff, lets increment the count so + ;; it can't be replayed. + (when valid-handoff? + ($ remote-handoff-count this-handoff-count)) + + valid-handoff?) (methods [(get-remote-side-name) remote-side-name] From 1cf16985dffa95439aef1d1dc02214b9fb2584ea Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Fri, 13 Oct 2023 16:04:10 +0000 Subject: [PATCH 2/2] Initial remote handoff count should be 0 --- goblins/ocapn/captp.rkt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/goblins/ocapn/captp.rkt b/goblins/ocapn/captp.rkt index a3acb1f..39519fc 100644 --- a/goblins/ocapn/captp.rkt +++ b/goblins/ocapn/captp.rkt @@ -939,7 +939,7 @@ ;; every time a *request* is made, this should be incremented. (define our-handoff-count 0) (define remote-handoff-count - (spawn ^cell -1)) + (spawn ^cell 0)) (define handoff-pubkey (pk-key->public-only-key handoff-privkey)) @@ -1150,14 +1150,14 @@ (define valid-handoff? (and (give-handoff-legit? signed-handoff-give) - (> this-handoff-count ($ remote-handoff-count)) + (>= this-handoff-count ($ remote-handoff-count)) (pk-verify give-recipient-key encoded-handoff-receive (gcrypt->racket/signature receive-sig)))) ;; If it is in fact a valid handoff, lets increment the count so ;; it can't be replayed. (when valid-handoff? - ($ remote-handoff-count this-handoff-count)) + ($ remote-handoff-count (+ this-handoff-count 1))) valid-handoff?)