twisted / twisted

Event-driven networking engine written in Python.
https://twisted.org
Other
5.6k stars 1.18k forks source link

Security: SO_EXCLUSIVEADDRUSE should be enabled when binding to ports on Windows #4195

Open twisted-trac opened 14 years ago

twisted-trac commented 14 years ago
davidsarah's avatar davidsarah reported
Trac ID trac#4195
Type defect
Created 2010-01-01 03:15:19Z

This is necessary to prevent socket hijacking for servers on ports >= 1024. On Windows, a port that has a socket bound to it can by default be taken over by any other process (even one run by a different user account, such as a Guest account), if that process uses SO_REUSEADDR. On Unix this would be considered a security bug. The SO_EXCLUSIVEADDRUSE option is required in order to get the default Unix behaviour of "first-come first-served" binding to ports. See wp_socket_hijacking.pdf (not readable in some PDF viewers), and here for more details. (The latter is slightly out of date since Windows does restrict binding to privileged ports since Windows XP SP2, but mostly still relevant.)

This affects the security of Tahoe-LAFS (http://allmydata.org/trac/tahoe/ticket/870) on Windows, and potentially any software using twisted on Windows to run a server on a non-privileged port.

There was some discussion of SO_EXCLUSIVEADDRUSE in bug #10756, but it wasn't the focus of the bug. #10756 was correctly closed as invalid because SO_REUSEADDR should not be used on Windows. The behaviour of Windows TCP when SO_EXCLUSIVEADDRUSE is set but SO_REUSEADDR isn't, is approximately the same as that of Unix TCP when SO_REUSEADDR is set (see the MSDN documentation).

SO_EXCLUSIVEADDRUSE was also mentioned by Trent.Nelson in bug #2981, as something that should be set on all connections, but twisted does not currently do so.

Note that before Windows 2000 SP4, Windows XP SP2 or Windows Server 2003, SO_EXCLUSIVEADDRUSE can only be used by processes running with administrator credentials. This bug is documented in the Microsoft knowledge base article #870562. So if it is intended to still run on these versions of Windows, twisted should fall back to not using the option if the Winsock call fails.

Searchable metadata ``` trac-id__4195 4195 type__defect defect reporter__davidsarah davidsarah priority__high high milestone__ branch__ branch_author__ status__new new resolution__None None component__core core keywords__security security time__1262315719000000 1262315719000000 changetime__1430532752647628 1430532752647628 version__None None owner__daira daira cc__david-sarah@... cc__exarkun ```
twisted-trac commented 9 years ago
daira's avatar daira set owner to daira
daira set status to new
twisted-trac commented 14 years ago
exarkun's avatar @exarkun set owner to davidsarah

Thank you for the very detailed description of this issue, and for investigating the past efforts related to this topic.

There are very few (if any) active Twisted developers with Windows access to develop or test a fix for this. It would be very helpful if you could put one together. Unit tests for the behavior are critical as well (perhaps even more important than the fix itself, which seems to be pretty obvious based on your description and the various discussions you've linked to).

Otherwise, someone will probably get to this eventually (and I'm certainly not trying to dissuade anyone else from working on this ticket with this comment), but it may take quite a while.

Thanks again.

twisted-trac commented 14 years ago
davidsarah's avatar davidsarah set status to assigned

Will do. I think the only additional unit test needed, is a test that we can no longer steal a port using SO_REUSEADDR (this can be enabled on all platforms). Existing tests should cover whether adding SO_EXCLUSIVEADDRUSE causes any regression.

twisted-trac commented 9 years ago
davidsarah's avatar davidsarah commented

I just noticed this ticket again. I should probably look at fixing it!

twisted-trac commented 9 years ago
glyph's avatar @glyph commented

Looking forward to the patch! This does look like something that should be addressed :). Thanks again for the detailed analysis.

twisted-trac commented 14 years ago
davidsarah's avatar davidsarah commented

There's an excellent discussion of this issue here. (The code given there has the right idea, but it shouldn't be doing a Windows version check; it should be using socket.SO_EXCLUSIVEADDRUSE whenever it exists. The version check given will fail on XP SP3 or if the major version v[0] is greater than 6, for example.)

twisted-trac commented 14 years ago
davidsarah's avatar davidsarah commented

Also somewhat relevant: http://bugs.python.org/issue2550

This thread suggests that there may be an additional, difficult-to-reproduce bug on Windows that sometimes causes two servers to be able to bind to the same port even when neither is using SO_REUSEADDR. That would be another reason to use SO_EXCLUSIVEADDRUSE for correctness, as well as security.

twisted-trac commented 14 years ago
davidsarah's avatar davidsarah commented

socket.SO_EXCLUSIVEADDRUSE was added in Python 2.4. Is twisted still supported on Python 2.3? If yes, does the test that we can no longer steal a port need to be skipped, or can it fail pre-Python 2.4?

twisted-trac commented 14 years ago
mwhudson's avatar @mwhudson commented

The next release of Twisted will depend on Python 2.4, so I don't think you have to worry about that.