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

socket module missing IPPROTO_IPV6, IPPROTO_IPV4 on Windows #73701

Closed
mhils mannequin opened this issue Feb 9, 2017 · 19 comments
Closed

socket module missing IPPROTO_IPV6, IPPROTO_IPV4 on Windows #73701

mhils mannequin opened this issue Feb 9, 2017 · 19 comments
Labels
3.8 only security fixes easy OS-windows type-bug An unexpected behavior, bug, or error

Comments

@mhils
Copy link
Mannequin

mhils mannequin commented Feb 9, 2017

BPO 29515
Nosy @pfmoore, @jaraco, @giampaolo, @tiran, @tjguk, @zware, @eryksun, @zooba, @mhils, @abrezovsky
PRs
  • bpo-29515: add missing socket.IPPROTO_* constants on Windows #12183
  • Dependencies
  • bpo-17561: Add socket.bind_socket() convenience function
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-02-13.03:53:58.390>
    created_at = <Date 2017-02-09.18:27:42.493>
    labels = ['easy', 'type-bug', '3.8', 'OS-windows']
    title = 'socket module missing IPPROTO_IPV6, IPPROTO_IPV4 on Windows'
    updated_at = <Date 2021-02-13.03:53:58.389>
    user = 'https://github.com/mhils'

    bugs.python.org fields:

    activity = <Date 2021-02-13.03:53:58.389>
    actor = 'jaraco'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-02-13.03:53:58.390>
    closer = 'jaraco'
    components = ['Windows']
    creation = <Date 2017-02-09.18:27:42.493>
    creator = 'mhils'
    dependencies = ['17561']
    files = []
    hgrepos = []
    issue_num = 29515
    keywords = ['patch', 'easy (C)']
    message_count = 19.0
    messages = ['287449', '287472', '287477', '287541', '333133', '333727', '335924', '335929', '337194', '337229', '337240', '337242', '337251', '337269', '337273', '338230', '339047', '339048', '386899']
    nosy_count = 11.0
    nosy_names = ['paul.moore', 'jaraco', 'giampaolo.rodola', 'christian.heimes', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'jgosmann', 'mhils', 'abrezovsky']
    pr_nums = ['12183']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29515'
    versions = ['Python 3.8']

    @mhils
    Copy link
    Mannequin Author

    mhils mannequin commented Feb 9, 2017

    The latest Windows builds for Python 3.5.3/3.6.0 do not export socket.IPPROTO_IPV6, even though e.g. socket.IPV6_V6ONLY is exported. This seems to be wrong to me as IPV6_V6ONLY requires the corresponding socket option level IPPROTO_IPV6 to be actually useful. The same issue at least also applies to IPPROTO_IPV4.

    christian.heimes mentioned that this is intended behaviour in https://bugs.python.org/issue6926 as Windows would not define the constants. Now I am all but an expert on this, but it seems to me that IPPROTO_IPV6 is defined in Windows:
    https://msdn.microsoft.com/en-us/library/windows/hardware/ff543746(v=vs.85).aspx. Apologies if I'm missing something here.

    Python 3.6.0 (v3.6.0:41df79263a11, Dec 23 2016, 07:18:10) [MSC v.1900 32 bit (Intel)] on win32
    >>> import socket; socket.IPPROTO_IPV6
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    AttributeError: module 'socket' has no attribute 'IPPROTO_IPV6'

    As a workaround, IPPROTO_IPV6 can be substituted with the hardcoded constant 41.

    @mhils mhils mannequin added OS-windows type-bug An unexpected behavior, bug, or error labels Feb 9, 2017
    @eryksun
    Copy link
    Contributor

    eryksun commented Feb 10, 2017

    The macro is defined but not defined. If I insert the following line before the #ifdef check:

        #define IPPROTO_IPV6 IPPROTO_IPV6

    then the constant gets added:

        >>> import _socket
        >>> _socket.IPPROTO_IPV6
        41

    The same applies to IPPROTO_IPV4 and probably others.

    @eryksun eryksun added the 3.7 (EOL) end of life label Feb 10, 2017
    @eryksun
    Copy link
    Contributor

    eryksun commented Feb 10, 2017

    Unless someone has a better (more automated) way to handle the Winsock IPPROTO enum, I suggest we either special-case the individual tests for MS_WINDOWS when we know that Winsock defines the value; or just define macros for the values in the enum:

        #ifdef MS_WINDOWS /* Macros based on the IPPROTO enum. */
        #define IPPROTO_ICMP IPPROTO_ICMP
        #define IPPROTO_IGMP IPPROTO_IGMP
        #define IPPROTO_GGP IPPROTO_GGP
        #define IPPROTO_TCP IPPROTO_TCP
        #define IPPROTO_PUP IPPROTO_PUP
        #define IPPROTO_UDP IPPROTO_UDP
        #define IPPROTO_IDP IPPROTO_IDP
        #define IPPROTO_ND IPPROTO_ND 
        #define IPPROTO_RAW IPPROTO_RAW
        #define IPPROTO_MAX IPPROTO_MAX
        #if (_WIN32_WINNT >= 0x0501)
        #define IPPROTO_HOPOPTS IPPROTO_HOPOPTS
        #define IPPROTO_IPV4 IPPROTO_IPV4
        #define IPPROTO_IPV6 IPPROTO_IPV6
        #define IPPROTO_ROUTING IPPROTO_ROUTING
        #define IPPROTO_FRAGMENT IPPROTO_FRAGMENT
        #define IPPROTO_ESP IPPROTO_ESP
        #define IPPROTO_AH IPPROTO_AH
        #define IPPROTO_ICMPV6 IPPROTO_ICMPV6
        #define IPPROTO_NONE IPPROTO_NONE
        #define IPPROTO_DSTOPTS IPPROTO_DSTOPTS
        #define IPPROTO_ICLFXBM IPPROTO_ICLFXBM
        #endif /* (_WIN32_WINNT >= 0x0501) */
        #if (_WIN32_WINNT >= 0x0600)
        #define IPPROTO_ST IPPROTO_ST
        #define IPPROTO_CBT IPPROTO_CBT
        #define IPPROTO_EGP IPPROTO_EGP
        #define IPPROTO_IGP IPPROTO_IGP
        #define IPPROTO_RDP IPPROTO_RDP
        #define IPPROTO_PIM IPPROTO_PIM
        #define IPPROTO_PGM IPPROTO_PGM
        #define IPPROTO_L2TP IPPROTO_L2TP
        #define IPPROTO_SCTP IPPROTO_SCTP
        #endif /* (_WIN32_WINNT >= 0x0600) */
        #endif /* MS_WINDOWS */

    or call PyModule_AddIntConstant(m, "IPPROTO_ICMP", IPPROTO_ICMP) and so on for each enum value.

    @zooba
    Copy link
    Member

    zooba commented Feb 10, 2017

    We may just want to copy the values from the enum if there are different versions when they were introduced. But if that's not a problem, adding the MS_WINDOWS check is better than defining new macros.

    @abrezovsky
    Copy link
    Mannequin

    abrezovsky mannequin commented Jan 7, 2019

    Can confirm this is still a problem in latest versions of 3.5, 3.6, and 3.7

    Any progress made on this?

    @zooba
    Copy link
    Member

    zooba commented Jan 15, 2019

    No progress, but I like the extra defines idea best (directly in socketmodule.c, not in a public header file). That's the easiest way to close the gap between (apparently) real constants used on Windows and the preprocessor defines (apparently) used elsewhere.

    @zooba zooba added the easy label Jan 15, 2019
    @giampaolo
    Copy link
    Contributor

    It turns out having this fix would be useful for proceeding with bpo-17561, which right now cannot support dual-stack IPv4/6 on Windows because IPPROTO_IPV6 is missing, see my comment:
    #11784 (comment)

    @giampaolo
    Copy link
    Contributor

    @eryksun I tried your patch (https://bugs.python.org/issue29515#msg287477) on Windows 10 and it looks good. It introduces the following new constants:

    IPPROTO_IGMP
    IPPROTO_GGP
    IPPROTO_PUP
    IPPROTO_IDP
    IPPROTO_ND
    IPPROTO_MAX
    IPPROTO_HOPOPTS
    IPPROTO_IPV4
    IPPROTO_IPV6
    IPPROTO_ROUTING
    IPPROTO_FRAGMENT
    IPPROTO_ESP
    IPPROTO_AH
    IPPROTO_ICMPV6
    IPPROTO_NONE
    IPPROTO_DSTOPTS
    IPPROTO_EGP
    IPPROTO_PIM
    IPPROTO_SCTP

    ...plus these Windows-only ones for which I had to add an additional PyModule_AddIntMacro() call:

    IPPROTO_ICLFXBM
    IPPROTO_ST
    IPPROTO_CBT
    IPPROTO_IGP
    IPPROTO_RDP
    IPPROTO_PGM
    IPPROTO_L2TP

    Up 'till now only these ones were exposed:

    IPPROTO_ICMP
    IPPROTO_TCP
    IPPROTO_UDP
    IPPROTO_RAW

    I think we can proceed with this. Care to provide a patch?

    @gpshead gpshead added the 3.8 only security fixes label Feb 19, 2019
    @giampaolo
    Copy link
    Contributor

    @eryk do you mind if I make a PR with your code(I will of course author you)? Regardless from bpo-17561 I think this is an important fix because without IPPROTO_IPV6 is not possible to implement a dual-stack IPv4/6 socket on Windows.

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 5, 2019

    do you mind if I make a PR with your code(I will of course author you)?

    Go for it. I prefer no credit, but you're free to do as you wish.

    @giampaolo
    Copy link
    Contributor

    Done:
    #12183
    This adds quite a bunch of new constants so I would be keen on considering it an enhancement rather than a bugfix and land it in 3.8 only. As for 3.7 and 3.6 (and 2.7?) it may make sense to fix IPPROTO_IPV6 only. Thoughts?

    @abrezovsky
    Copy link
    Mannequin

    abrezovsky mannequin commented Mar 5, 2019

    I lean towards it being considered a bug fix. First, the lack of parity between Windows and Linux-based OSs causes confusion. Second, the current workaround to hard-code the value in is far from best practice.

    @zooba
    Copy link
    Member

    zooba commented Mar 6, 2019

    The problem is that if we add lots of new constants in 3.7.3, code written in that version very easily becomes incompatible with 3.7.2 and earlier. So we don't like making changes like that.

    If we know which ones are "normally" defined on Linux machines, where specifying them on Windows will make it just work, then I'd just add those ones.

    Anything that is Windows-specific (if any?) or that may cause applications to break if they're testing for the constant but it doesn't work smoothly should not go back to 3.7.

    @abrezovsky
    Copy link
    Mannequin

    abrezovsky mannequin commented Mar 6, 2019

    I see your point.

    Then perhaps only a subset of the more vital ones get added to previous releases?

    #define IPPROTO_IPV4 IPPROTO_IPV4
    #define IPPROTO_IPV6 IPPROTO_IPV6
    #define IPPROTO_TCP IPPROTO_TCP
    #define IPPROTO_UDP IPPROTO_UDP
    #define IPPROTO_ICMP IPPROTO_ICMP
    #define IPPROTO_ICMPV6 IPPROTO_ICMPV6
    #define IPPROTO_RAW IPPROTO_RAW

    @giampaolo
    Copy link
    Contributor

    IPPROTO_TCP/UDP/RAW/ICMP are already there. AFAICT the only useful one here is IPPROTO_IPV6, because it's needed for enabling dual-stack. I don't think IPPROTO_IPV4 is a must (e.g. it's not available on Linux). Same for IPPROTO_ICMPV6. The only reason we may decide to backport one of these is because they are not officially documented (see bpo-27409). That gives us some flexibility but potentially it may do more harm than good because an app running on 3.7.3 would break on 3.7.2. Note: the original ticket (bpo-6926) is from 2009, meaning that whoever wanted these missing constants on Windows already hard-coded them in their Python code long ago.

    @giampaolo
    Copy link
    Contributor

    I'd like to move forward with this in order to fix bpo-17561. If there are no complaints I'm gonna merge this in ~ a week.

    @giampaolo
    Copy link
    Contributor

    New changeset 3eca28c by Giampaolo Rodola in branch 'master':
    bpo-29515: add missing socket.IPPROTO_* constants on Windows (GH-12183)
    3eca28c

    @giampaolo
    Copy link
    Contributor

    Merged. I'm agnostic about backporting IPPROTO_IPV6 (or others) on < 3.8 so I'll leave it up to somebody else to decide/implement.

    @giampaolo giampaolo removed the 3.7 (EOL) end of life label Mar 28, 2019
    @jaraco
    Copy link
    Member

    jaraco commented Feb 13, 2021

    As of June last year, Python 3.7 is in security fix only mode, so there's nothing more to be done here.

    @jaraco jaraco closed this as completed Feb 13, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes easy OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants