After upgrading to Python 3.8, users of sshuttle report seeing this error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "assembler.py", line 38, in <module>
  File "sshuttle.server", line 298, in main
  File "/usr/lib/python3.8/socket.py", line 544, in fromfd
    return socket(family, type, proto, nfd)
  File "/usr/lib/python3.8/socket.py", line 231, in __init__
    _socket.socket.__init__(self, family, type, proto, fileno)
OSError: [Errno 88] Socket operation on non-socket

https://github.com/sshuttle/sshuttle/issues/381

The cause of the error is this line: 
https://github.com/sshuttle/sshuttle/blob/6ad4473c87511bcafaec3d8d0c69dfcb166b48ed/sshuttle/server.py#L297 which does:

socket.fromfd(sys.stdin.fileno(), socket.AF_INET, socket.SOCK_STREAM)
socket.fromfd(sys.stdout.fileno(), socket.AF_INET, socket.SOCK_STREAM)

Where sys.stdin and sys.stdout are stdin/stdout provided by the ssh server when it ran our remote ssh process.

I believe this change in behavior is as a result of a fix for the following bug: https://bugs.python.org/issue35415

I am wondering if this is a bug in Python for causing such a regression, or a bug in sshuttle. Possibly sshuttle is using socket.fromfd in a way that was never intended?

Would appreciate an authoritative answer on this.

Thanks

Blocking the use of stdin/stdout like this looks intentional. I have added dimaqq and asvetlov for comment.

The python docs [0] indicate that the file descriptor should be a socket:
"The file descriptor should refer to a socket, but this is not checked — subsequent operations on the object may fail if the file descriptor is invalid."

With bpo-35415, the fd is explicitly checked.

[0] https://docs.python.org/3/library/socket.html#socket.fromfd

The code in question is interesting... I don't claim to understand it.

A pair of file descriptors is passed wrapped in sockets, and the objects are used here:

https://github.com/sshuttle/sshuttle/blob/966fd0c5231d56c4f464752865d96f97bd3c0aac/sshuttle/ssnet.py#L238-L240

* s.setblocking(False)
* os.read(s.fileno())
* select.select([s], [s], ...) elsewhere in the file
* s.shutdown() elsewhere in the file

server.py uses Mux() with a pair of os.stdin/stdout, realistically pty/pipe/file
client.py uses Mux() with [an anonymous domain] socket from socketpair()

So, on one hand, it is used like a socket, on the other, it's used as a holder for a file-like file descriptor 🤷‍♂️

Back in the day, I too wrote (C) code that treated file/tty and socket file descriptors alike. I've since come to realise that's not exactly correct. IIRC doing so was elegant, UNIX-y, but implied platform specifics, i.e. was not cross-platform.

I've also done similar tricks with serial.Serial before: instantiate but don't open, set pty fd, use to test code that expects a serial port in memory. Luckily that code was not open-source ☺️. It was a hack, but I don't know if it's cpython's job to support hacks of this kind.

My 2c:
Personally, I'd rather see the this "fixed" in sshuttle than in cpython.
Perhaps someone with authority over the socket module can weight the balance between "a regression because using socket.socket as a container for generic file descriptors used to work before" vs. "least surprise for well-behaved and future code".

I don't consider this a regression. socket.fromfd() and socket(fd) never supported non-socket fds. It just happens to work in some cases by chance.

It looks like sshuttle got lucky in the past. It never called a function that ultimately requires a socket fd.

Consensus seems to be that this is a bug in sshuttle, not a bug in python. Thanks for the feedback.

I think this bug can be closed now...