router: add STUN implementation

[jdslab]

Modification to border router dataplane to process STUN requests, as described in the design document and discussion.

[tzaeschke]

It seems weird that this function is an almost exact duplicate of the function in line 571. I suppose this is out of scope for this PR?

[tzaeschke]

Maybe add something here like:
"This demo client uses manual STUN commands to demonstrate how STUN can be implemented with SCION.
Normal clients would not do that and instead use a library that performs STUN automatically and transparently."
?

[jdslab]

Done.

[jiceathome]

J-CIn router/underlayproviders/udpip/udpip.go:

p.Link = l // The src address does not need to be recorded in the packet. The link has all the relevant // information. + It seems weird that this function is an almost exact duplicate of the function in line 571. I suppose this is out of scope for this PR? Message ID: ***@***.***> I guess this is referring to the many similarities between the link subtypes implementations. FWIW, I deliberately avoided using inheritance in order to retain the freedom of optimizing each link subtype. Sharing code between them would have encouraged forced commonalities or added the risk of breaking one when intending to change the behavior of the other. I think this flexibility is worth a small amount of redundancy. The link subtypes do not semantically extend each other. For example, in the af packet implementation (how about reviewing that PR?), the link subtypes have different relationships. So I did not want my code to serve as a misleading example to someone implementing a new underlay.Just my $.02 … that kid has left the nest now :-)

[jdslab]

Reviewable status: 3 of 21 files reviewed, 7 unresolved discussions (waiting on @jiceathome, @marcfrei, and @tzaeschke)

---

demo/stun/test.py line 4 at r1 (raw file):

Previously, jiceathome wrote…

2025. It's new code, right?

Done.

---

demo/stun/test-client/main.go line 1 at r1 (raw file):

Previously, jiceathome wrote…

Copyright? License?

Done.

---

demo/stun/test-server/main.go line 1 at r1 (raw file):

Previously, jiceathome wrote…

Ditto

Done.

[jdslab]

Done.

[jdslab]

Reviewable status: 3 of 21 files reviewed, 7 unresolved discussions (waiting on @jiceathome, @marcfrei, and @tzaeschke)

---

demo/stun/test.py line 114 at r1 (raw file):

Previously, jiceathome wrote…

How reliable is that time guess? Can we probe readiness instead? It is OK for a test take 10 seconds or even much more, but provided we don't have too many of those.

The 10s wait time is sufficient for everything to start up. It is also the same wait time as used in the dr_key acceptance test, which also runs a docker compose topology. Given that optimizing this would likely only shave off a few seconds from the running time, I don't think it is worth the effort to overly optimize this.

[JordiSubira]

We could add metrics for STUN, it could be useful to debug the "STUN flow", i.e., BR is receiving requests, it is sending them back to clients, etc. This should be fairly easy extending router.trafficMetrics.

[JordiSubira]

If we ever have a protocol type assigned to 0x21 we may identify STUN packets as SCION packets (depending on the result of the if below). We do not need to tackle this now, but documenting it in the function description will help if we ever come across this.

[JordiSubira]

I would register these values as const in slayers.l4.go . I guess they haven't been needed in the implementation so far, but since they are already documented it would make more clear that the implementation is following what is expected in the doc for these values.

[JordiSubira]

I do not see any ouput when running the test. This is likely because you are using log in test-client and test-server. Go's log package writes to STDERR by default. dc.execute() only captures STDOUT (see docker.py:55 - return res.stdout).

In addition, I would add an assertion since we have a fixed output that we are expecting "abc".

[JordiSubira]

I am a bit doubtful how we should better organize the integration test. I believe the main reason for having demo was having the file_transfer demo. Later, Matthias suggested that we could incorporate DRKey in demo instead of acceptance, because we were taking it as an application running over SCION, rather than an inherent part of the SCION protocol (i.e., we do not need DRKey for SCION to work, actually DRKey needs SCION to work, at least as it is defined now). In the case of STUN, I do believe it is a necessary component for SCION to work properly. In any case, this is how I had understood the organization so far, but we can always restructure this.

[marcfrei]

Up till now, the intention of this demo was to show how to use STUN on an end host "manually" given that we have support for it in the border router. Once we have STUN support in snet, we can get rid of this demo application.