Skip to content

socket.connect - support custom family value #92658

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

Closed
jborean93 opened this issue May 11, 2022 · 5 comments · Fixed by #92755
Closed

socket.connect - support custom family value #92658

jborean93 opened this issue May 11, 2022 · 5 comments · Fixed by #92755
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@jborean93
Copy link
Contributor

jborean93 commented May 11, 2022

Feature or enhancement

Currently socket.connect() accepts a wide range of values when connecting to a socket with the code in socketmodule.c transforming it based on the family the socket was created with. Unfortunately there is a check that fails the connection if the family is unrecognized

cpython/Modules/socketmodule.c

Lines 2530 to 2534 in 9d85aba

default:
PyErr_SetString(PyExc_OSError, "getsockaddrlen: bad family");
return 0;
}
.

My proposal is to allow a caller to bypass this check if passing in a raw byte value to be used as the addr info on the native call.

Pitch

The reason why I am hoping for this feature is to support clients connecting to a Hyper-V socket. This uses the AF_HYPERV family which isn't known to Python so any attempts to connect to it won't work.

Currently I am using the following to work around this restriction by using ctypes to call the C API directly and using the fileno:

import ctypes
import socket
import uuid

HV_GUID_VM_SESSION_SERVICE_ID = uuid.UUID("999e53d4-3d5c-4c3e-8779-bed06ec056e1")
HV_GUID_VM_SESSION_SERVICE_ID_2 = uuid.UUID("a5201c21-2770-4c11-a68e-f182edb29220")
AF_HYPERV = 34
HV_PROTOCOL_RAW = 1

# This is the GUID of the Win VM to connect to
vm_id = uuid.UUID("...")

win32sock = ctypes.WinDLL("Ws2_32.dll")

raw_sock = win32sock.socket(AF_HYPERV, socket.SOCK_STREAM, HV_PROTOCOL_RAW)
if raw_sock == -1:
    err = win32sock.WSAGetLastError()
    raise ctypes.WinError(code=err)

try:
    sock_addr = b"\x22\x00\x00\x00" + vm_id.bytes_le + HV_GUID_VM_SESSION_SERVICE_ID.bytes_le
    res = win32sock.connect(raw_sock, sock_addr, len(sock_addr))
    if res:
        err = win32sock.WSAGetLastError()
        raise ctypes.WinError(code=err)

    sock = socket.socket(fileno=raw_sock)
except:
    win32sock.closesocket(raw_sock)
    raise

...

sock.close()

It would be good to be able to do this instead

import socket

sock = socket.socket(AF_HYPERV, socket.SOCK_STREAM, HV_PROTOCOL_RAW)
sock_addr = b"\x22\x00\x00\x00" + vm_id.bytes_le + HV_GUID_VM_SESSION_SERVICE_ID.bytes_le
sock.connect(sock_addr)

...

sock.close()

Currently that fails due to the hardcoded check against unknown families

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: connect(): bad family

Another option is to add support for AF_HYPERV on Windows and support a tuple of (vm_id, service_id) and have Python create the struct. This could be done as a separate feature request potentially.

Linked PRs

@jborean93 jborean93 added the type-feature A feature request or enhancement label May 11, 2022
@ronaldoussoren
Copy link
Contributor

It would IMHO be better to add explicit support for this new address family.

@jborean93
Copy link
Contributor Author

jborean93 commented May 11, 2022

I agree, especially from a UX perspective but would it also be nice if it supported arbitrary bytes in the cases when Python does not support the family for other protocols, now and in the future?

@jborean93
Copy link
Contributor Author

I've added a PR that adds explicit support for the new address family. I'm not too experienced on the C side so any pointers or comments would be greatly appreciated #92755.

@vstinner
Copy link
Member

I agree, especially from a UX perspective but would it also be nice if it supported arbitrary bytes in the cases when Python does not support the family for other protocols, now and in the future?

I don't think that it's a good idea. Each protocol is different and requires special code.

@jborean93
Copy link
Contributor Author

Fair enough, I mostly only desired it for the Hyper V stuff and was mindful if I came across another family I would have the same problem but that makes sense.

vstinner added a commit that referenced this issue May 25, 2022
Only build the AF_HYPERV support on Windows for the _socket extension.
FreeBSD defines the AF_HYPERV macro but doesn't have the SOCKADDR_HV
type.
sobolevn added a commit to sobolevn/cpython that referenced this issue Jun 3, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 6, 2023
zooba pushed a commit that referenced this issue Jun 6, 2023
(cherry picked from commit 3907de1)

Co-authored-by: Nikita Sobolev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants