-
Notifications
You must be signed in to change notification settings - Fork 348
openissues
Create a "pal" directory for seperating out Unix/Windows code cmdlineparser.cpp - needs a getopt/getopt_long replacement getconsolewidth.cpp, getmilisecondcounter - move to a new PAL directory structure refcountedobject.cpp - move the "AtomicIncrement" and "AtomicDecrement" stuff to PAL MD5/SHA1/HMAC - need either a common solution, or split into PAL. Need to figure out HMAC-SHA1 on windows Threads - abstraction between pthreads and Windows threads. Likely only need to support "create" and "join" operations. networkutils - almost all of it. recvfromex, polling, adapter/ip enumeration
signals (aka the "exit now listener") - only needed for ctrl-c and SIGHUP stoppage. Need to get the threading on this right as well.
winsock requires initialization. As so does crypto calls on windows. run-as-service mode UI interface for configuration setup package
Some versions of GNU compiler will not abort with an error if the #include references inside "commonincludes.h" do not resolve. Usual symptom is when Boost is not properly installed or specified in the Makefile. The result is that commonincludes.gch still builds, but doesn't include <boost/assert.hpp>. Subsequent compile of "cmdlineparser" claims that BOOST_ASSERT is not defined.
Seems like a similar handling could be done for legacy "response-address" and the newer "response-port". We could attempt to honor "response-address", by mapping it to an equivalent "response-port" (but ignore the IP address part of the request) DECISION - PUNTED for 1.1
Revalidate that all the padding stuff works for CStunMessageBuilder in regards to string attributes. I think we fixed all this in 1.1.
Documentation. "info" page in addition to MAN page? Installer stuff for Unix? "make install" ? Autotools support?
Either ship xxd source code or convert the resource compile step to be a perl script. (Who does't have perl installed?). DECISION - COMPLETED.
We never added "unknown attribute" detection, even though we wrote code to handle sending the response back. Not a terrible bug, but something to think about fixing. DECISION - PUNT
If the client request is detected by the server as "legacy rfc 3489" (i.e. magic cookie is not present), we send back the legacy CHANGED-ADDRESS (0x0005) attribute instead of OTHER-ADDRESS (0x802c). But why do we not do the equivalent for SOURCE-ADDRESS (0x0004) and RESPONSE-ORIGIN (0x802b)? Consider adding this for better legacy interop. DECISION - COMPLETED (see next item below)
For legacy requests, we need to always send back XOR-MAPPED-ADDRESS in addition to plan MAPPED-ADDRESS, even for legacy requests (consistent with Vovida). DECISION - COMPLETED
Optional-understanding attributes (0x80??) attributes should always follows after the required-understand attributes. ACTUALLY NO! Fingerprint and Message Integrity always come last. I think we got a solution to mirror Vovida behavior. DECISION - COMPLETE (integrity and fingerprint still come last)
Got a very ambiguous report from a user that he couldn't get his "sip device" (frizbox) to "log in and register" with the stunserver code base. But when he switched to Vovida, it works. I am deeply confused because he has been very limited in providing details. I suspect he's confusing "sip server" and "stun server". But that doesn't explain why he thinks it works with Vovida's code base. This would be a good opportunity to review how closely our responses mirror Vovida for legacy requests. It would not surprise me if there is stun client code in the wild that expects attributes in a certain order and/or is hardcoded to expect a response formatted in a certain way. Or perhaps there is an auth issue? Consider contacting fritzbox after the holidays. DECISION - CURRENTLY WAITING TO HEAR BACK FROM FRITZBOX. Update: Fritzbox suggested the user get the log files from his fritzbox. User has been uncooperative. PUNT.
Now that ITransport thing has been removed, does it make sense for CStunServer to be a refcounted object with a mandatory factory? (Perhaps so...) DECISION - NO CHANGE.
RFC 3489 defines all attributes as multiples of 4-bytes. For attributes such as "error-code" that can contain a variable length text string, it explicitly states padding in the attribute string (with spaces) and the attribute length header is always a multiple of 4. But RFC 5389 suggests that attributes not on byte-boundaries are padded, but the attribute header is not adjusted. This creates a compatibility problem with responses such as "error-code". stunserver is sending back back padding without adjusting the attribute length header. This is compliant with 5389, but not compliant with 3489. Fix this such that error-code and attributes with strings send back padded values and length header is always multiple of 4. Ughh.... ... this means CStunMessageBuilder::AddAttribute and AddStringAttribute are going to have to be legacy aware. How do we fix this for "nonce", "username, and "realm" attributes? We need to consult RFC 3489 for how to handle this. Some auth types are specific to 5389 and might not matter. DECISION - YES, ******* MUST FIX FOR ERROR-CODE and STRING ATTRIBUTES. REVIEW CURRENT CODE. CONSIDER HAVING CStunMessageBuilder have a "legacy mode" so that the flag doesn't get passed in for every method.******
Other attributes that suffer from the four byte alignment inconsistency: UNKNOWN-ATTRIBUTES and RESPONSE-PORT. For Unknown-attributes, RFC 3489 says to repeat one attribute in the list. RESPONSE-PORT did not exist in RFC 3489.
Message-integrity attribute. RFC 3489 says something about padding the message fed to the algorithm to be a multiple of 64 bytes. I don't recall coding anything for this. Probably because it wasn't defined for RFC 5389. Yet another issue to resolve between RFC 3489 and 5389. Might be ok to just say, "auth is not supported for legacy clients". DECISION - AUTH IS NOT SUPPORTED FOR LEGACY CLEINTS, BUT THE LEGACY PASSWORD FIELD IS.
Man page. At some point, to make this package complete, we're going to have to write a man page. A good suggestion on stackoverflow is to use pandoc. http://stackoverflow.com/questions/7228461/man-page-editor-for-text-screen. DECISION - MUST FIX.
Fix csocketaddress::applyxormap to not assume the transaction Id starts with the magic cookie. This is so we can send XOR-mapped-address to legacy clients. Consistent with vovida. DECISION - COMPLETED
Something very odd about RFC 5780. For evaluating NAT port mapping behavior it describes the 3 tests.
Test 1 - Basic binding request to PP
If not NAT, then "endpoint independent", otherwise run test 2
Test 2 - Binding request to AP
If result is the same as test 1, then "endpoint independent". Otherwise, run test 3
Test 3 - Basic request to AA
If result is the same as test 2, then "address dependent mapping", otherwise, "address and port dependent mapping"
I personally think they got test 2 and test 3 above mixed up. Either that, or they were very deliberate about this order for a reason. Seems like to really confirm true endpoint independent mapping, you'd want test 2 to hit the AA instead of AP. If the result is the same as test 1, then you definitely know that you have endpoint independent mapping instead of some possible type of "dependent mapping".
My intuition tells me this is a better order with an additional test
Test 1 - Basic binding request to PP
If not NAT, then "endpoint independent"
Otherwise run test 2
Test 2 - Binding request to AA (instead of AP)
If result is the same as test 1, then "endpoint independent".
Otherwise run test 3
Test 3 - Binding request to AP (instead of AP)
If the result is completely different from test 1 AND test 2 --> Address and port dependent mapping
If the result is the same as test 2 --> Address dependent mapping
If the result is the same as test 1 --> Port dependent mapping (does this even exist?)
Optional: Run test 4
Test 4 - Binding request to PA
Could be used to further validate which of the dependent mapping schemess are in effect
Perhaps the 4th test isn't warranted. But once you confirm that the NAT does not have endpoint independent mapping, that essentially means that subsequent reflexive candidates obtained through STUN/TURN are unreliable at best for hole punching in P2P with a remote host (who's likely got a completely different IP and port).
I have an email out to the Behave alias about this. I didn't get any responses. Possibly because I botched the subject line and/or because of the holidays. Either way, I'll follow up with them and the RFC authors after the holidays.