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

[security][subinterpreters] Add auditing hooks to subinterpreter module #87638

Closed
tiran opened this issue Mar 11, 2021 · 6 comments
Closed

[security][subinterpreters] Add auditing hooks to subinterpreter module #87638

tiran opened this issue Mar 11, 2021 · 6 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-security A security issue

Comments

@tiran
Copy link
Member

tiran commented Mar 11, 2021

BPO 43472
Nosy @vstinner, @tiran, @ericsnowcurrently, @zooba, @miss-islington, @gousaiyang
PRs
  • bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module #25506
  • [3.9] bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module (GH-25506) #25508
  • [3.8] bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module (GH-25506) #25509
  • 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 = 'https://github.com/zooba'
    closed_at = <Date 2021-04-21.22:40:55.372>
    created_at = <Date 2021-03-11.09:31:02.526>
    labels = ['type-security', 'interpreter-core', '3.8', '3.9', '3.10', 'expert-subinterpreters']
    title = '[security][subinterpreters] Add auditing hooks to subinterpreter module'
    updated_at = <Date 2021-04-28.16:20:49.418>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2021-04-28.16:20:49.418>
    actor = 'vstinner'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2021-04-21.22:40:55.372>
    closer = 'steve.dower'
    components = ['Interpreter Core', 'Subinterpreters']
    creation = <Date 2021-03-11.09:31:02.526>
    creator = 'christian.heimes'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43472
    keywords = ['patch']
    message_count = 6.0
    messages = ['388489', '390387', '391548', '391551', '391554', '392232']
    nosy_count = 6.0
    nosy_names = ['vstinner', 'christian.heimes', 'eric.snow', 'steve.dower', 'miss-islington', 'gousaiyang']
    pr_nums = ['25506', '25508', '25509']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue43472'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @tiran
    Copy link
    Member Author

    tiran commented Mar 11, 2021

    The subinterpreters module does not emit any audit events yet. It's possible to create a subinterpreter and run arbitrary code through run_string().

    We should also improve documentation of sys.addaudithook() and explain what 'current interpreter' actually means. I guess most users don't realize the consequences for subinterpreters.

    $ ./python auditsub.py
    ('os.system', (b'echo main interpreter',))
    main interpreter
    you got pwned
    [heimes@seneca cpython]$ cat au
    auditsub.py     autom4te.cache/ 
    [heimes@seneca cpython]$ cat auditsub.py 
    import sys
    import _xxsubinterpreters
    def hook(*args):
        print(args)
    
    sys.addaudithook(hook)
    
    import os
    os.system('echo main interpreter')
    
    sub = _xxsubinterpreters.create()
    _xxsubinterpreters.run_string(sub, "import os; os.system('echo you got pwned')", None)
    $ ./python auditsub.py 
    ('os.system', (b'echo main interpreter',))
    main interpreter
    you got pwned

    @tiran tiran added 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-security A security issue labels Mar 11, 2021
    @gousaiyang
    Copy link
    Mannequin

    gousaiyang mannequin commented Apr 6, 2021

    One problem is the naming of audit events. Actually I didn't even notice that _xxsubinterpreters was already there since Python 3.8, because PEP-554 is still in draft status as for now. Looks like _xxsubinterpreters is an internal low-level interface to subinterpreters (and probably only meant for testing purposes for now), while PEP-554 will bring a high-level interface interpreters for users. Naming the audit events as interpreters.* will be more readable, although the interpreters module doesn't actually exist today.

    @zooba
    Copy link
    Member

    zooba commented Apr 21, 2021

    I'll need Eric to confirm, but I think the best thing to do here is to not release the thread state in _xxsubinterpreters.interp_create, but let _Py_NewInterpreter() do it. That way the existing event will be raised in interpreter-level hooks, rather than only the process-wide hook.

    PR incoming.

    @zooba
    Copy link
    Member

    zooba commented Apr 21, 2021

    New changeset 7b86e47 by Steve Dower in branch 'master':
    bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module (GH-25506)
    7b86e47

    @zooba zooba added 3.8 only security fixes 3.9 only security fixes labels Apr 21, 2021
    @zooba zooba closed this as completed Apr 21, 2021
    @zooba zooba self-assigned this Apr 21, 2021
    @zooba zooba added 3.8 only security fixes 3.9 only security fixes labels Apr 21, 2021
    @zooba zooba closed this as completed Apr 21, 2021
    @zooba zooba self-assigned this Apr 21, 2021
    @miss-islington
    Copy link
    Contributor

    New changeset 602eefe by Miss Islington (bot) in branch '3.8':
    bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module (GH-25506)
    602eefe

    @vstinner
    Copy link
    Member

    New changeset 0252ce3 by Miss Islington (bot) in branch '3.9':
    bpo-43472: Ensure PyInterpreterState_New audit events are raised when called through _xxsubinterpreters module (GH-25506) (GH-25508)
    0252ce3

    @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 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-security A security issue
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants