multi: add deadline for first connection of new LNC conn #408
Conversation
ellemouton
force-pushed
the
timeLimitFirstLNCConn
branch
from
96dbc12 to
6ff563f
Compare
session_rpcserver.go
Outdated
|
|
||
| err = s.db.RevokeSession(pubKey) | ||
| if err != nil { | ||
| log.Debugf("error revoking session: "+"%v", err) |
There was a problem hiding this comment.
Any reason for not returning the error here? Or was this intended to be a return fmt.Errorf()? Also, string concatenation artifacts.
There was a problem hiding this comment.
erg yeah sorry 🙈 also, there should be a return after this 🙈
config.go
Outdated
| Loop: &loopDefaultConfig, | ||
| PoolMode: defaultPoolMode, | ||
| Pool: &poolDefaultConfig, | ||
| FirstLNCConnDeadline: 10 * time.Minute, |
session_rpcserver.go
Outdated
| } | ||
|
|
||
| // Start the deadline timer. | ||
| firstConnectionDeadline = time.NewTimer( |
There was a problem hiding this comment.
I think we could use time.AfterFunc() here that closes the firstConnTimeout. Then in the onNewStatus callback we only stop that timer and we don't need a whole other goroutine (the debug comment could be moved into the onNewStatus callback).
There was a problem hiding this comment.
omg yeeeesss!!! thank you :) updated!
ellemouton
force-pushed
the
timeLimitFirstLNCConn
branch
from
6ff563f to
603f4c7
Compare
There was a problem hiding this comment.
thanks @guggero ! updated as per you suggestion 🚀 much neater the way you have suggested :)
session_rpcserver.go
Outdated
|
|
||
| err = s.db.RevokeSession(pubKey) | ||
| if err != nil { | ||
| log.Debugf("error revoking session: "+"%v", err) |
There was a problem hiding this comment.
erg yeah sorry 🙈 also, there should be a return after this 🙈
session_rpcserver.go
Outdated
| } | ||
|
|
||
| // Start the deadline timer. | ||
| firstConnectionDeadline = time.NewTimer( |
There was a problem hiding this comment.
omg yeeeesss!!! thank you :) updated!
|
hmmm the itest keeps failing....i must have broken something on the latest update. looking into it |
ellemouton
force-pushed
the
timeLimitFirstLNCConn
branch
from
603f4c7 to
3ea0e5a
Compare
In this commit, we add a deadline for the initial connection of an LNC connection. So with this, the user is forced to use their pairing phrase within a certain time frame. After this initial connection, future connections are made with the second handshake version meaning that the pairing phrase is rendered useless. By adding a time limit to the time in which a user can use their pairing phrase, we reduce the risk created by the users pairing phrase being leaked. The default time limit is set to 10 minutes but can be customsed with the new `firstlncconndeadline` flag.
ellemouton
force-pushed
the
timeLimitFirstLNCConn
branch
from
3ea0e5a to
d5c2604
Compare
|
hahaha found the issue 🙈 when I extracted the timeout into a variable, I used seconds instead of minutes 🙈 should be g2g now! I also added another commit that adds LNC client logging to the itests just to make it easier to debug these things in future |
In this PR, we add a deadline for the initial connection of an LNC
connection. So with this, the user is forced to use their pairing phrase
within a certain time frame. After this initial connection, future
connections are made with the second handshake version meaning that the
pairing phrase is rendered useless. By adding a time limit to the time
in which a user can use their pairing phrase, we reduce the risk created
by the users pairing phrase being leaked. The default time limit is set
to 10 minutes but can be customsed with the new
firstlncconndeadlineflag.
The first commit in this PR adds a new
CreatedAtfield toSessionso thatwe can have an absolute deadline.
Depends on lightninglabs/lightning-node-connect#53
Fixes #406