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

Fix setting of IPV6_V6ONLY on Windows #5139

Conversation

bernd-edlinger
Copy link
Member

On Windows, the default of IPV6_V6ONLY is different than on Linux.

That means that on Windows (at least Windows 7)
openssl s_server -accept 4443 -6
listens on [::]:4443 only,
that means: openssl s_client -connect [::1]:4443 succeeds
while openssl s_client -connect [127.0.0.1]:4443 fails.

while on Linux both commands succeed.

@bernd-edlinger bernd-edlinger added the branch: master Merge to master branch label Jan 22, 2018
@bernd-edlinger
Copy link
Member Author

@levitte I noticed that the cygwin test stalls while the normal windows test worked on the
same machine.
I think there were different IP modules installed in cygwin perl and strawberry perl,
which must have triggered that.

if ((options & BIO_SOCK_V6_ONLY) && BIO_ADDR_family(addr) == AF_INET6) {
if (BIO_ADDR_family(addr) == AF_INET6) {
/* Note: Windows default of IPV6_V6ONLY is ON, and Linux is OFF.
* Therefore we always have to use setsockopt here. */
Copy link
Member

Choose a reason for hiding this comment

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

Block comment must be like this:

/*
 * text...
 * text...
 */

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, okay, I did just copy the style from a few lines above.
Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe I fix the other comment as well.

@levitte
Copy link
Member

levitte commented Jan 22, 2018

@levitte I noticed that the cygwin test stalls while the normal windows test worked on the same machine.
I think there were different IP modules installed in cygwin perl and strawberry perl, which must have triggered that.

Well, since the tests use hard-coded ports, the two runs are going to trample over each other. No surprise there...

@bernd-edlinger
Copy link
Member Author

Well, since the tests use hard-coded ports, the two runs are going to trample over each other. No surprise there...

I did of course not run both at the same time.
Just the outcome seems to be dependent on the available perl modules.

@levitte
Copy link
Member

levitte commented Jan 22, 2018

Could you check the presence of IO::Socket::INET, IO::Socket::IP and IO::Socket::INET6 on each of them?

@kroeckx
Copy link
Member

kroeckx commented Jan 22, 2018

I would like to point out that the Linux behavior is actually something you can control. Also some other OSs don't support IPv6 mapped IPv4.

@bernd-edlinger
Copy link
Member Author

The working setup is Strawberry Perl:
INET.pm IP.pm UNIX.pm

The non-working (but fixed by this PR) setup is cygwin-perl:
INET.pm UNIX.pm

There may also be a difference what "localhost" is resolved into.

The failed test run on cygwin stalls here:

../../openssl/test/recipes/70-test_comp.t ............
Proxy started on port 4453
Server command: ../../util/shlib_wrap.sh ../../apps/openssl.exe s_server -no_comp -rev -engine ossltest -accept 4443 -cert ../../../openssl/apps/server.pem -cert2 ../../../openssl/apps/server.pem -naccept 1 -cipher AES128-SHA:TLS13-AES-128-GCM-SHA256
Client command: echo test | ../../util/shlib_wrap.sh ../../apps/openssl.exe s_client -engine ossltest -connect localhost:4453
engine "ossltest" set.
Using default temp DH parameters
ACCEPT
engine "ossltest" set.
Connection opened
Failed to start up server (localhost,4443): Connection refused
1..0 # SKIP Unable to start up Proxy for tests
write:errno=104
CONNECTED(00000003)
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 0 bytes and written 202 bytes
Verification: OK
---
New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : 0000
    Session-ID:
    Session-ID-ctx:
    Master-Key:
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    Start Time: 1516690537
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: no

@bernd-edlinger
Copy link
Member Author

I would like to point out that the Linux behavior is actually something you can control.

Is there a configuration option other than using the sockopt for that?

@levitte
Copy link
Member

levitte commented Jan 23, 2018

Regarding Cygwin, that set of modules should have make the s_server and s_client commands IPv4 only, i.e. there should be a -4 among the flags, as of this commit: c44bab0

@bernd-edlinger
Copy link
Member Author

Whoops... yes, I forgot to update. With -4 that would not have happened.
But the server still should not bind to IPv6 only.

@levitte
Copy link
Member

levitte commented Jan 23, 2018

The server binds to the first address it gets when looking up localhost. If the address it gets is an IPv6 one, then it's going to bind to IPv6. -4 makes sure it doesn't...

@bernd-edlinger
Copy link
Member Author

With the client I can make it sure what I get when I say "-connect [::1]:4443" or
"-connect [127.0.0.1]:4443"
while it may depend on the lookup if I use "-connect localhost:4443"
The server says "-accept 4443 -4" or just "-accept 4443" which may bind
to IPv4 or IPv6 or both....

@bernd-edlinger
Copy link
Member Author

I know already that the sever uses getaddrinfo(NULL, ...)
so no "localhost", just NULL. and that returns IPv4 and IPv6 are returned
in different order on windows an linux.
I believe at least linux is hardcoded, it opens a few dgram sockets but no
configuration files, from what I saw with my strace execrises last week.

@levitte
Copy link
Member

levitte commented Jan 23, 2018

We really should change s_server to accept() on all addresses returned by BIO_lookup_ex, not just the first... But that's a question for a different PR

@bernd-edlinger
Copy link
Member Author

But the point of this PR is still that the socket should bind to IPv6 and IPv4
unless BIO_SOCK_V6_ONLY is given or the O/S does not support that.

@bernd-edlinger
Copy link
Member Author

Regarding Cygwin, that set of modules should have make the s_server and s_client commands IPv4 only, i.e. there should be a -4 among the flags, as of this commit: c44bab0

I updated to the git rev 50625bf but I still don't see a -4, neither on win32 nor cygwin.
Something is odd.

@levitte
Copy link
Member

levitte commented Jan 23, 2018

No surprise for Windows, as IO::Socket::IP is IPv6 capable... The issue is on Cygwin, as IO::Socket::INET doesn't have IPv6 capability. So the question is what happens here and here

@bernd-edlinger
Copy link
Member Author

Aehm, apparently there is an IP module as well, but in a different directory.
So the Proxy picks IP and fails then...

@bernd-edlinger
Copy link
Member Author

Hmm, and Windows Perl picks INET6, which I failed to find previously.
INET6 works probably because it resolves "localhost" differently.

@kroeckx
Copy link
Member

kroeckx commented Jan 24, 2018 via email

@bernd-edlinger
Copy link
Member Author

I think it's /proc/sys/net/ipv6/bindv6only

Yes, thanks, you're right. I would not want to depend on that setting, though.

@@ -206,7 +208,12 @@ int BIO_listen(int sock, const BIO_ADDR *addr, int options)
}

# ifdef IPV6_V6ONLY
if ((options & BIO_SOCK_V6_ONLY) && BIO_ADDR_family(addr) == AF_INET6) {
Copy link
Contributor

Choose a reason for hiding this comment

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

On a side note one can wonder if it's appropriate to ignore BIO_SOCK_V6_ONLY in cases when IPV6_V6ONLY is not defined. I mean following. If you have platform that supports AF_INET6, but not IPV6_V6ONLY, then wouldn't it be appropriate to return error to application that attempts BIO_SOCK_V6_ONLY on such platform? Instead of silently ignoring it as it is now that is.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see it as a kind of best effort.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see it as a kind of best effort.

And question is this is adequate view. I mean if you pass unrecognized option, wouldn't you like to know that it's not supported? Well, one can always argue that AF_INET6 platforms that don't have IPV6_V6ONLY don't deserve respect... For reference, so far I've spotted only one, Windows 2003/XP. So no biggie... Once again, this is a side note.

@@ -175,8 +175,10 @@ int BIO_listen(int sock, const BIO_ADDR *addr, int options)
return 0;

# ifndef OPENSSL_SYS_WINDOWS
/* SO_REUSEADDR has different behavior on Windows than on
Copy link
Contributor

Choose a reason for hiding this comment

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

On side note one can wonder how does it affect Cygwin. I mean Cygwin has no other option by to rely on Windows sockets... So maybe condition has to cover even Cygwin...

Copy link
Contributor

Choose a reason for hiding this comment

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

On related note, since SO_REUSEADDR is special on Windows, maybe it would be appropriate to support its "antipode", SO_EXCLUSIVEADDRUSE. I mean b_sock2.c is supposed to provide OS-neutral interface to sockets, right? So that one write multi-platform application. This kind of implies that it should be expressive enough to do various useful things. And it's not inappropriate to expect that BIO socket is created "exclusive". In fact one can wonder if they should be created "exclusive" unconditionally on Windows, as reflection of fact that it's "exclusive" on Unix... Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

So they are saying that another process from another user account for instance
can intercept new connections with SO_REUSEADDR, unless we use
SO_EXCLUSIVADDRUSE?

Copy link
Contributor

Choose a reason for hiding this comment

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

So they are saying

Originally it was the case, yes. By now they have some improvements, like bound port can be hijacked only from same account...

@mattcaswell mattcaswell added this to the 1.1.1 milestone Jan 24, 2018
levitte pushed a commit that referenced this pull request Jan 25, 2018
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5139)
@bernd-edlinger
Copy link
Member Author

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants