feat: npt: added 'keep-alive' flag, and an adjustable session timeout

gkc commented 4 weeks ago

- What I did closes #979

feat: npt: added keep-alive, and an adjustable srv timeout

(a) adjustable timeout
(b) keep-alive - if the session ends, wait 5 seconds then create a new session
(c) does the right thing in all the cases I've tested which are
- (i) unavailability of rvd
- (ii) unavailability of sshnpd
- (iii) unavailability of atServer
Two new command-line args:
- --keep-alive or -K : a flag, defaults false
- --timeout or -T <int> : SocketConnector timeout in seconds, defaults to 30 (the current default)

NB: Required a change to the daemon as well, you need to be running the latest daemon in order to use the adustable timeout feature; the timeout will not be respected if you are running an older daemon. I have added a feature check so npt knows it needs to check that the adjustableTimeout feature is available if it is using a non-default timeout

- How I did it

See commits

- How to verify it

Tests pass
Manually tested as many different combinations as I could

gkc commented 4 weeks ago

@cconstab I'll leave this as Draft until you've had a chance to test

cconstab commented 3 weeks ago

I think I hit a wierd bug (how me!)... I could not get srvd to answer when using 'npt' at all.. In the end I decided to try sshnp to make sure it was working .. And I spotted this in the logs... Encrypted shared symmetric key for @stream not found in local storage

The first sshnp failed but popped this error, the second and every next one worked just fine too. Then of course npt starts working too.

Well the good news I went through all my logs and the original npt issue was a typo in the command that I could not see for looking..

cconstab@orac:~/.local/bin$ ./npt -K -T -1 -f @cconstab -t @ssh_1 -r @stream -r 22 -p 22 -l 9999 -d orac

Note the -r 22 my eyes for whatever reason could not see it at the time.. The sshnp "bug" might be worth chasing down but it as I mentioned worked the second time and forever since.

At this point I cannot break anything but...

INFO|2024-06-13 01:47:26.567548| srvd / socket_connector |Assigned ports [41357, 42123] for session c0c3b165-0a9e-4335-bc67-49de71bbff7f
INFO|2024-06-13 01:47:26.567604| srvd / socket_connector |Waiting for connector to close
INFO|2024-06-13 01:47:26.567654| srvd |Received ports (41357, 42123) in main isolate for session c0c3b165-0a9e-4335-bc67-49de71bbff7f
SHOUT|2024-06-13 01:47:26.567687| srvd |Starting session c0c3b165-0a9e-4335-bc67-49de71bbff7f for @cconstab to @ssh_1 using ports (41357, 42123)
SHOUT|2024-06-13 01:47:26.567707| srvd |Sending response data for requested session c0c3b165-0a9e-4335-bc67-49de71bbff7f : [85.239.53.170,41357,42123
,2024-06-13T01:47:26.454986]
INFO|2024-06-13 01:47:26.772258|AbstractAtKeyEncryption (@stream)|Encrypted shared symmetric key for @stream not found in local storage
INFO|2024-06-13 01:47:26.772334|AbstractAtKeyEncryption (@stream)|Deleting @cconstab:shared_key@stream from LocalSecondary
INFO|2024-06-13 01:47:26.772618|AbstractAtKeyEncryption (@stream)|Fetching shared symmetric key for @stream from atServer
INFO|2024-06-13 01:47:26.826892|AbstractAtKeyEncryption (@stream)|Retrieved my encrypted copy of shared symmetric key for @cconstab from atServer - s
aving to local storage

cconstab commented 3 weeks ago

How can you specify "never timeout?"

Perhaps reserve -T 0 for that ? But I might be missing something ?? If -K is set then -T should auto to 0 ?

gkc commented 3 weeks ago

@cconstab re the "not working first time" - do you have the sshnpd logs? Nothing has changed in the srvd, nor in the interactions with the srvd ... I do think there is a race condition in the AtClient if it receives two notifications in quick succession and is still in the process of resolving shared key for the first notification when the second one arrives. This is the case when sshnp runs and sends both an ssh public key and a ping. I'll investigate that.

The duplication of -r in the command line not causing an exception looks like a very-not-cool feature of the args package. Will look to see if anything can be done there

gkc commented 3 weeks ago

How can you specify "never timeout?"

No current mechanism.

Perhaps reserve -T 0 for that ? But I might be missing something ?? If -K is set then -T should auto to 0 ?

Yes I think that if -K is set then -T should default to something large, but I'm not inclined to make it "never timeout" by default

I agree -T 0 is a reasonable way to indicate "never timeout"

I'm considering having a more user-friendly way of supplying durations from command line args in standardized way e.g. 30s, 5m, 3h30m, 5d, 1w etc ... I'll see if I can find a suitable package. Might write one if I can't find any such package extant.

cconstab commented 3 weeks ago

I have the sshnpd in tmux so I should be able to grab them. Yup just minor tight corner stuff..

I did also find an ugly error on npt if the local port is already taken that I will raise an issue on when I get a mo..

gkc commented 3 weeks ago

How can you specify "never timeout?"

No current mechanism.

Perhaps reserve -T 0 for that ? But I might be missing something ?? If -K is set then -T should auto to 0 ?

Yes I think that if -K is set then -T should default to something large, but I'm not inclined to make it "never timeout" by default

I agree -T 0 is a reasonable way to indicate "never timeout"

I'm considering having a more user-friendly way of supplying durations from command line args in standardized way e.g. 30s, 5m, 3h30m, 5d, 1w etc ... I'll see if I can find a suitable package. Might write one if I can't find any such package extant.

Found this package which satisfies my requirement: https://pub.dev/documentation/duration/latest/duration/parseDuration.html

gkc commented 2 weeks ago

How can you specify "never timeout?"

No current mechanism.

Perhaps reserve -T 0 for that ? But I might be missing something ?? If -K is set then -T should auto to 0 ?

Yes I think that if -K is set then -T should default to something large, but I'm not inclined to make it "never timeout" by default I agree -T 0 is a reasonable way to indicate "never timeout" I'm considering having a more user-friendly way of supplying durations from command line args in standardized way e.g. 30s, 5m, 3h30m, 5d, 1w etc ... I'll see if I can find a suitable package. Might write one if I can't find any such package extant.

Found this package which satisfies my requirement: https://pub.dev/documentation/duration/latest/duration/parseDuration.html

@cconstab I have made the following changes:

if --keep-alive is set but no explicit --timeout is set, set it to 24 hours
timeout takes human-readable timeout values e.g. "45s" or "1h,30m" etc
take -T 0 to mean "never" timeout, which results in a timeout value of 365 days

XavierChanth commented 2 weeks ago

I would prefer if we cut the v5.4.0 release before merging this, waiting on a couple of bug fixes before we can do that release

atsign-foundation / noports

feat: npt: added 'keep-alive' flag, and an adjustable session timeout #1110