Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stun: minimal working client/server #545

Merged
merged 56 commits into from
Dec 6, 2023
Merged

stun: minimal working client/server #545

merged 56 commits into from
Dec 6, 2023

Conversation

yosoyubik
Copy link
Contributor

@yosoyubik yosoyubik commented Oct 25, 2023

WIP branch (on top on ted/ping; will be merged there). Uses urbit/urbit#6836 for the %arvo side.

  • Send/Receive STUN request/response, handled in %arvo by stopping the %ping app.
  • Check DNS resolution -- currently we send STUNs in fakeship mode
    • (check changing hosts.txt)
  • refactor _stun_czar, _ames_czar,
    • make a generic _dns_czar(imp_y) that gets called every ~m5 for our sponsoring galaxy, and every time an %ames packet is sent to another galaxy
  • refactor _ames_czar/stun_gone
  • Use XOR-MAPPED-ADDRESS to send the sponsee's IP
  • Better STUN (re)sends, regarding timers (e.g. number of tries per attempt... etc)
    • "send requests at times 0 ms, 500 ms, 1500 ms, 3500 ms, 7500
      ms, 15500 ms, and 31500 ms. If the client has not received a
      response after 39500 ms", stop, inject %fail, (re)resolve DNS, and restart STUN.
  • Callback error handling
    • if a response failes, ignore—sender will try again
    • if a request fails, resend it
    • if the DNS resolution fails, stop everyting and restart
    • if the response doesn't arrive in ~s39, inject %fail, stop everything and restart
  • DNS Error handling
    • Failure to resolve DNS (first time) -- have a global ~s25 timer?
    • Change in sponsor's IP -- resolve DNS on failure to hear a response?
    • exponentially backoffresolve DNS every 5 minutes
  • Better STUN response check
  • Add sponsee's IP to MAPPED-ADDRESS attribute field
  • cache ip address
  • (re)enable %ping app after failure to hear STUN response, if we eventually hear one
  • Test plan: see last point in ames: add STUN response handling urbit#6836 (comment)

@yosoyubik
Copy link
Contributor Author

Successful STUN responses flowing through!

Screenshot 2023-10-25 at 2 51 22 PM

@yosoyubik
Copy link
Contributor Author

Current state machine for STUNing.

(Note: doesn't include the %stun tasks sent to %ames: %stop, %once, %fail)

stateDiagram

  state %ping {
    
    state "STUN off" as off
    state "STUN trying" as try
    state "STUN keepalive" as keep
 
    [*] --> off
    off --> try: on %saxo
    try --> keep: on RESPONSE
    keep --> try: no response after ~s25
    keep --> keep: response < ~s25
   
    try --> try: after ~s1 (5x) // after ~s25 // on %saxo (escape)
    #try --> try: after ~s2
    #try --> try: after ~s3
    #try --> try: after ~s4
    #try --> try: after ~s5

    #try --> try: after ~s25


    #try --> try: on %saxo (escape)
    keep --> try: on %saxo (escape)

  }
Loading

@yosoyubik
Copy link
Contributor Author

RFC5389 has a section where it describes "[...] an optional mechanism for STUN that aids in distinguishing STUN messages from packets of other protocols when the two are multiplexed on the same transport address."

I've added this FINGERPRINT attribute to both request/response, and also a check for both possible STUN request/response on the receiver. I've also replaced the ad-hoc retrieval of these attributes with a parser that goes over the message buffer and tries to identify the selected attribute (currently FINGERPRINT for both request and response, and XOR-MAPPED-ADDRESS only for the response).

Now, in _ames_recv_cb we still check first if the message is STUN (request or response) and if not, we check if it's ames.

Here's a capture of Wireshark with both attributes:

Screenshot 2023-11-20 at 11 54 31 AM

@yosoyubik yosoyubik marked this pull request as ready for review November 20, 2023 13:40
@yosoyubik yosoyubik requested a review from a team as a code owner November 20, 2023 13:40
@pkova pkova force-pushed the yu/stun branch 2 times, most recently from e97f07f to 3024882 Compare November 30, 2023 17:53
...and if not, keep trying until we go over the whole buffer

(Note: the parsde port is wrong)
@yosoyubik
Copy link
Contributor Author

@joemfb I implemented here 6f8e718 a way to go over the whole length of the buffer and try if the first found attribute looks "correct" (if it's either a fingerprint or an IP address), otherwise continue until the end. This is to account for "duplicate" attributes (e.g. two XOR-MAPPED-ADDRESS attributes where the first one has a malformed/incorrect IP address, but the second does not) in STUN response/requests.

This seems to be a bit of an edge case but I added it to be on the safe side. LMKWYT.

@yosoyubik
Copy link
Contributor Author

@joemfb it looks like the RFC:

Any attribute type MAY appear more than once in a STUN message.
   Unless specified otherwise, the order of appearance is significant:
   only the first occurrence needs to be processed by a receiver, and
   any duplicates MAY be ignored by a receiver.

So we might not need the loop to go over everything.

Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will do

@yosoyubik yosoyubik changed the base branch from ted/ping to next/kelvin/411 December 6, 2023 12:28
@yosoyubik
Copy link
Contributor Author

As of this latest changes, everything seems to be working as expected. Next step is to start testing this as outlined here: urbit/urbit#6836 (comment)

@pkova pkova merged commit 4a88bd9 into next/kelvin/411 Dec 6, 2023
5 checks passed
@pkova pkova deleted the yu/stun branch December 6, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants