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

[BUG] malformed sockaddr returned by srt_getsockname for IPv4mapped IPv6 address #1318

Closed
jeandube opened this issue Jun 3, 2020 · 8 comments · Fixed by #1355
Closed

[BUG] malformed sockaddr returned by srt_getsockname for IPv4mapped IPv6 address #1318

jeandube opened this issue Jun 3, 2020 · 8 comments · Fixed by #1355
Labels
[core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@jeandube
Copy link
Collaborator

jeandube commented Jun 3, 2020

Listening on [::]:, with SRTO_IPV6ONLY==0 and peer connecting using 127.0.0.1:. srt_getsockname on the accepted socket returns a struct sockaddr_in6 (family=AF_INET6) with sin6_addr = [7f00:1::] which is 127,0,0,1,0.... while an ipv4mapped IPv6 address [::ffff:127.0.0.1] is expected as reported by UDP on the same circumstances.

[ 1]srt    S [7f00:1::]:9930||[::ffff:127.0.0.1]:9932                 1.4.1  STREAMING  
[ 1]udp    S [::ffff:127.0.0.1]:9070|[::ffff:127.0.0.1]:45242

SRT v1.4.1

@jeandube jeandube added the Type: Bug Indicates an unexpected problem or unintended behavior label Jun 3, 2020
@maxsharabayko maxsharabayko added the [core] Area: Changes in SRT library core label Jun 3, 2020
@maxsharabayko maxsharabayko added this to the v1.5.0 milestone Jun 3, 2020
@jeandube
Copy link
Collaborator Author

jeandube commented Jun 3, 2020

same issue if non-loopback address used. For example connecting with 10.65.10.36:9930 result in:

[a41:a24::]:9930|10.65.10.36:9932

@jeandube
Copy link
Collaborator Author

jeandube commented Jun 3, 2020

My initial investigation leads to the C++ ::getsockname function returning bad struct.

@ethouris
Copy link
Collaborator

ethouris commented Jun 3, 2020

getsockname should simply return the exactly same thing that has been previously set on the UDP socket by bind.

The problem is that it's hard to guess what you meant when you didn't bind the socket. It's not a problem of a buggy software, rather what you were meaning. I guess this might be fixed from the application side by explicitly calling srt_bind, passing it a sockaddr_in6 structure filled with appropriate type (AF_INET6) and its mutation of INADDR_ANY.

This definitely is the result of taking out the obligatory AF_ type to be specified when creating a socket, but with the group feature I didn't have much choice (the group allows that member sockets are connected also over different IP version).

@jeandube
Copy link
Collaborator Author

jeandube commented Jun 3, 2020

This socket is created from a listening socket. Even if I think it would make sense for the application to bind it, I would not know which address I would use, the listen binding is [::]:9930 meaning it accepts any IPv4 or IPv6 connection on port 9930. by calling srt_getsockname this is the information I am looking for: on which address family and interface did the call come in. I don't think it is the role of the application to set it.

@ethouris
Copy link
Collaborator

ethouris commented Jun 3, 2020

Of course, in this case you are right. Listener sockets get bound, but it's then the accepted socket out of the listener socket that gets configured ACCORDING TO the configuration of the listener (and I doubt this thing has even changed since UDT times - nothing got changed in the listener rules since).

Please try to make a detailed reproduction steps for it, cause I'm getting a little bit confused. I believe this is a mistake in the internal code and easy to fix, but as far as I know SRT code, in case when your listener socket that accepts connections on any and both IPv4 and IPv6, the actual family for the newly accepted socket should be determined from the incoming address, not the family of the listener socket - and I believe this is not being done.

@ethouris
Copy link
Collaborator

ethouris commented Jun 4, 2020

Wait. The problem is kinda bigger - the accepted socket shares the internal UDP socket with the listener socket. Therefore there's no binding being done at all - at least not on a UDP socket.

@maxsharabayko
Copy link
Collaborator

@jeandube What would be the minimum sample to reproduce the issue?
I guess we should modify the test-c-server example to listen on an IPv6 address. And listen on [::]:.
Then modify the test-c-client to connect to 127.0.0.1:.
Does the port number need to be specified as well?

@jeandube
Copy link
Collaborator Author

jeandube commented Jun 4, 2020

@maxsharabayko ,
listen on [::]:P, V6ONLY=0, peer calls A.B.C.D:P (A.B.C.D being 127.0.0.1 or the listener's IPv4 address).
On server's accepted socket, srt_getpeername() returns a proper IPV4-mapped IPv6 address but not srt_getsockname(). I expect the same behavior as UDP. I cannot easily test with earlier version of SRT and this is not something my test app was doing before I adapted it to handle streamid. So I cannot say it ever worked as expected before.

@maxsharabayko maxsharabayko modified the milestones: v1.5.0, v1.5.0 - Sprint 17 Jun 12, 2020
@mbakholdina mbakholdina modified the milestones: v1.5.0 - Sprint 17, v1.4.2 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants