Title: Many command execution functions are not raising auditing events
Type: security Stage: patch review
Components: Versions: Python 3.9, Python 3.8
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, gousaiyang, miss-islington, steve.dower
Priority: normal Keywords: patch

Created on 2020-01-01 23:10 by gousaiyang, last changed 2020-07-04 22:27 by gousaiyang.

Pull Requests
URL Status Linked Edit
PR 17824 merged gousaiyang, 2020-01-04 23:44
PR 18353 merged miss-islington, 2020-02-05 00:15
PR 18407 merged gousaiyang, 2020-02-07 23:53
PR 18500 merged steve.dower, 2020-02-13 08:04
PR 18580 merged steve.dower, 2020-02-20 22:05
PR 18582 merged miss-islington, 2020-02-20 22:25
PR 21322 open gousaiyang, 2020-07-04 22:24
Messages (12)
msg359177 - (view) Author: Saiyang Gou (gousaiyang) * Date: 2020-01-01 23:10
Similar to `os.system` (which is already raising auditing event), the following functions are also capable of command execution, so they also need auditing:

- os.execl
- os.execle
- os.execlp
- os.execlpe
- os.execv
- os.execve
- os.execvp
- os.execvpe
- os.posix_spawn
- os.posix_spawnp
- os.spawnl
- os.spawnle
- os.spawnlp
- os.spawnlpe
- os.spawnv
- os.spawnve
- os.spawnvp
- os.spawnvpe
- os.startfile
- pty.spawn

By the way, since `os.listdir`, `shutil.copytree` and `shutil.rmtree` are already being audited, is it necessary to audit file operations in the `os` module like `os.remove`?
msg359246 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-03 18:40
Agreed, we should add events for all of these. They can go into the next 3.8 release.

Ideally, the lowest level operations (typically os module) should raise events. Where it's convenient to also audit operations at a higher level (shutil, subprocess, etc.) then we can do that too, as it provides helpful context for *why* the lower-level operation ran.

Contributions welcome :) I'll likely get to them eventually, but no reason it has to be me adding everything.
msg359311 - (view) Author: Saiyang Gou (gousaiyang) * Date: 2020-01-05 00:11
I have made PR 17824 to add auditing events for the command execution functions mentioned above.

After a review on other related Python modules, I think maybe the following functions can also be audited, but a discussion may be required to determine whether they are necessary (whether these actions are sensitive enough to record, and performance trade off). 

- os.getenv/putenv/unsetenv
- os.getcwd/chdir
- os.chown/chmod
- os.stat/access
- os.rename/renames/replace
- os.mkdir/mkdirs
- os.remove/removedirs/rmdir/unlink  (`shutil.rmtree` is already audited)
- os.add_dll_directory
- os.fork
- os.kill/killpg
- os.path.exists/isfile/isdir/...
- signal.pthread_kill
- shutil.copy*  (`shutil.copytree` is already audited)
- shutil.move
- shutil.chown
- shutil.unpack_archive  (`shutil.make_archive` is already audited)
- resource.prlimit
- file operations in `msvcrt`
- functions in `fcntl`, `syslog`
- many high level networking modules such as `http.client/server`, `socketserver`, `xmlrpc`  (the low-level `socket` calls are already audited)
msg361385 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-02-05 00:15
New changeset 95f60010219e142a436fae18e1695cbc45407afe by Saiyang Gou in branch 'master':
bpo-39184: Add audit events to command execution functions in os and pty modules (GH-17824)
msg361386 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-02-05 00:18
Thanks for the spawn patch, I've merged it.

On the second list, I'd say go for it. The only one I'd skip are the stat() calls (and those that just do a stat call, such as exists/isfile, etc.), and getcwd() (which has many other ways to implicitly use the information). Maybe skip getenv as well, but modifying the environment is worth collecting.
msg361391 - (view) Author: miss-islington (miss-islington) Date: 2020-02-05 00:32
New changeset 3498ac55bcfc18d698ea605424ec65a6e1457a39 by Miss Islington (bot) in branch '3.8':
bpo-39184: Add audit events to command execution functions in os and pty modules (GH-17824)
msg361616 - (view) Author: Saiyang Gou (gousaiyang) * Date: 2020-02-08 00:09
Thanks for your review! PR 18407 is for the second list. For now I haven't added audit hooks for the http, socketserver and xmlrpc modules because they look a bit complex. There seems to be so many classes and methods to hook, we may need to find good places to hook (similar to what has been done on ftplib, imaplib, nntplib, poplib, smtplib, telnetlib and urllib).
msg361940 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-02-13 07:47
New changeset 7514f4f6254f4a2d13ea8e5632a8e5f22b637e0b by Saiyang Gou in branch 'master':
bpo-39184: Add audit events to functions in `fcntl`, `msvcrt`, `os`, `resource`,  `shutil`, `signal`, `syslog` (GH-18407)
msg361945 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-02-13 08:30
New changeset a00b5be5f71b702ab80b0a7c046485046aaae160 by Steve Dower in branch '3.8':
bpo-39184: Add audit events to functions in `fcntl`, `msvcrt`, `os`, `resource`,  `shutil`, `signal`, `syslog` (GH-18407)
msg362357 - (view) Author: miss-islington (miss-islington) Date: 2020-02-20 22:24
New changeset 6c444d0dab8f06cf304263b34beb299101cef3de by Steve Dower in branch 'master':
bpo-39184: Fix incorrect return value (GH-18580)
msg362358 - (view) Author: miss-islington (miss-islington) Date: 2020-02-20 22:44
New changeset d0a464e31ac67685ef8ad35ecd993f17dfd6ab35 by Miss Islington (bot) in branch '3.8':
bpo-39184: Fix incorrect return value (GH-18580)
msg373000 - (view) Author: Saiyang Gou (gousaiyang) * Date: 2020-07-04 22:27
Since the original problem (command execution functions missing audit events) is already solved, we can close this issue now. Further discussions on additional audit hooks (e.g. for the networking modules) could go to issue 37363.
Date User Action Args
2020-07-04 22:27:25gousaiyangsetmessages: + msg373000
2020-07-04 22:24:53gousaiyangsetpull_requests: + pull_request20473
2020-02-20 22:44:53miss-islingtonsetmessages: + msg362358
2020-02-20 22:25:00miss-islingtonsetpull_requests: + pull_request17952
2020-02-20 22:24:50miss-islingtonsetmessages: + msg362357
2020-02-20 22:05:42steve.dowersetpull_requests: + pull_request17950
2020-02-13 08:30:34steve.dowersetmessages: + msg361945
2020-02-13 08:04:59steve.dowersetpull_requests: + pull_request17874
2020-02-13 07:47:49steve.dowersetmessages: + msg361940
2020-02-08 00:09:20gousaiyangsetmessages: + msg361616
2020-02-07 23:53:05gousaiyangsetpull_requests: + pull_request17782
2020-02-05 00:32:36miss-islingtonsetnosy: + miss-islington
messages: + msg361391
2020-02-05 00:18:19steve.dowersetmessages: + msg361386
2020-02-05 00:15:39miss-islingtonsetpull_requests: + pull_request17728
2020-02-05 00:15:09steve.dowersetmessages: + msg361385
2020-01-05 00:11:33gousaiyangsetmessages: + msg359311
2020-01-04 23:44:05gousaiyangsetkeywords: + patch
stage: patch review
pull_requests: + pull_request17252
2020-01-03 18:40:08steve.dowersetnosy: + christian.heimes
messages: + msg359246
2020-01-01 23:13:59vstinnersetnosy: + steve.dower
2020-01-01 23:10:03gousaiyangcreate