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

securing pydoc server #37822

Closed
kasplat mannequin opened this issue Jan 22, 2003 · 8 comments
Closed

securing pydoc server #37822

kasplat mannequin opened this issue Jan 22, 2003 · 8 comments
Labels
stdlib Python modules in the Lib dir type-security A security issue

Comments

@kasplat
Copy link
Mannequin

kasplat mannequin commented Jan 22, 2003

BPO 672656
Nosy @orsenthil, @ned-deily
Files
  • pydoc_security_patch.diff: restrict IP address pydoc accepts connections
  • 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 2010-08-18.19:36:58.575>
    created_at = <Date 2003-01-22.19:45:59.000>
    labels = ['type-security', 'library']
    title = 'securing pydoc server'
    updated_at = <Date 2014-09-15.20:38:12.329>
    user = 'https://bugs.python.org/kasplat'

    bugs.python.org fields:

    activity = <Date 2014-09-15.20:38:12.329>
    actor = 'devin'
    assignee = 'ping'
    closed = True
    closed_date = <Date 2010-08-18.19:36:58.575>
    closer = 'orsenthil'
    components = ['Library (Lib)']
    creation = <Date 2003-01-22.19:45:59.000>
    creator = 'kasplat'
    dependencies = []
    files = ['4952']
    hgrepos = []
    issue_num = 672656
    keywords = ['patch']
    message_count = 8.0
    messages = ['42516', '42517', '42518', '114204', '114269', '226927', '226930', '226938']
    nosy_count = 8.0
    nosy_names = ['ping', 'kasplat', 'pboddie', 'orsenthil', 'aptshansen', 'ned.deily', 'BreamoreBoy', 'devin']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue672656'
    versions = ['Python 3.1', 'Python 2.7']

    @kasplat
    Copy link
    Mannequin Author

    kasplat mannequin commented Jan 22, 2003

    It would be very simple to secure the pydoc server so
    that it doesn't accept connections from external boxes
    as well as provide for a way of extending connections to
    trusted hosts by keeping a list of valid IP addresses.
    This would make pydoc suitable for running on boxes
    that aren't behind firewalls, which currently it is not;
    most home machines don't have a firewall and are
    regularly port scanned by script kiddies...

    Since pydoc does not log connections, you can't tell
    who is connecting to your machine or what they are
    trying to reach. My solution is to simply make the
    default pydoc server only accept connections from the
    host it was started on.

    The change is for the DocServer class. a validIPList
    keeps track of the IP addresses that can legally connect
    to the server. The verify_request method is overridden to
    enforce this rule.

                import socket
                self.validIPList = ['127.0.0.1']
                self.validIPList.append(socket.gethostbyname
    (socket.gethostname()))
    
    
            def verify_request(self, request, client_address):
                if client_address[0] in self.validIPList:
                    return 1
                else:
                    return 0

    This patch does not provide a UI change to allow the
    user to easily add additional IP addresses. If that is
    desired because of the assumption that people typically
    run the pydoc server not for personal use, but for a group
    of machines to reach, then the simplest change would
    be to have a checkbox for "Allow any host to connect"
    and then have a self.allowAny member variable to reflect
    that checkbox state, so the verify_request becomes

        def verify_request(self, request, client_address):
            if self.allowAny or client_address[0] in 
    self.validIPList:
                return 1
            else:
                return 0

    ka

    @kasplat kasplat mannequin assigned ping Jan 22, 2003
    @kasplat kasplat mannequin added the stdlib Python modules in the Lib dir label Jan 22, 2003
    @aptshansen
    Copy link
    Mannequin

    aptshansen mannequin commented Mar 17, 2007

    I think this is actually a good idea; but I don't think the implementation is really sufficient as it stands. Particularly, it's going to require that someone hand edit a file in Lib to adjust the behavior from the "default" of only allowing connections from localhost. A user interface is not required, but an easy to reach configuration file is, I think.

    Instead, I think it should read a pydoc.cfg ConfigParser file-- and just apply the defaults if said file doesn't exist. (Where to put it? I don't know. ~/pydoc.cfg?)

    Also, having to list specific IP addresses is going to greatly limit utility for those people who do want it more open. Some people might want to allow everyone in their subnet to access it, instead of just 'everyone' or 'specific people' as this patch implies. I don't think there's an easy way to do CIDR math in the Python library, but a simple regex in said configuration file would be plenty I imagine. Or even a list of strings you check to see if the ip address startswith.

    In the current form, I'd recommend rejection. I don't know if the submitter is interested in any major updates after a few years, but if they are.. :)

    @pboddie
    Copy link
    Mannequin

    pboddie mannequin commented Mar 21, 2007

    Wouldn't it be easier to just bind the server to localhost? That way, the server should only listen on the loopback interface and not any of the external network interfaces. At around line 1974 of pydoc.py (Python 2.4.3)...

                host = (sys.platform == 'mac') and '127.0.0.1' or 'localhost'
                self.address = ('', port)
                self.url = 'http://%s:%d/' % (host, port)

    Replace the '' with host in self.address by default, perhaps. Then, add a host parameter to the serve function and let this be used to override the above. Expose the parameter as a command line argument. I'll come up with a patch for this at some point, I suppose.

    @devdanzin devdanzin mannequin added the type-security A security issue label Mar 30, 2009
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Aug 18, 2010

    This looks weird, a security issue with a low priority???

    @orsenthil
    Copy link
    Member

    As the pydoc server "advertises" that it is running from localhost in both CLI and GUI, it is best to bind the socket to 'localhost' instead of '' (which would bind it to all the interfaces).

    So, a simple fix for this issue, which will remove the security concern:
                 host = 'localhost'
    -            self.address = ('', port)
    +            self.address = (host, port)

    If is to be run from user-defined interface with a new --host <interface> option, that it can be dealt with as new feature request.

    This issue can be considered fixed with commits r84173 and r84174.

    @devin
    Copy link
    Mannequin

    devin mannequin commented Sep 15, 2014

    It looks like this bug was reintroduced in a5a3ae9be1fb.

    @ned-deily
    Copy link
    Member

    Devin, please open a new issue describing the current problem you see. Comments to long-closed issues will likely be overlooked.

    @devin
    Copy link
    Mannequin

    devin mannequin commented Sep 15, 2014

    Sure, thanks.

    New issue: http://bugs.python.org/issue22421

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-security A security issue
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants