Consider applying flags for warnings about potential security issues #112301
Comments
mdboom
added
type-feature
|
I don't think we want |
These warnings do no make much sense in current use-cases: I think that they should be silenced / ignored. |
|
@mdboom Are you okay with me editing your topic to create a checklist style table with links to either why we're not implementing or the actual implementation? My guess is we'll be adopting these one by one :) |
@sethmlarson: Good idea. |
Sounds a good approach. To share another method that could additionally help: as part of #101100, we're working through a lot of docs "nit-picky" warnings. When building the docs, we only allow warnings to occur in files that already have warnings and are listed in a We also fail the docs build if we "accidentally" clean a file: if warnings do not occur in a file where we previously expected warnings, so the file must also be removed from the list, again to prevent regressions. This does need some custom tooling, but it's helped us make gradual progress, and we've fixed 40% so far. |
I am in favor of a solution like this. It would not require any custom tooling as we could change the build arguments to whatever we find consensus in and then silence compiler warnings for offending lines until somebody comes along and fixes them. This also allows us to silence errors locally, but enforce them globally. That way we could still have |
|
Hello all I have been selected by GSoC to work on this! @mdboom I am curious how you were able to get the unit tests to pass with the linker option
|
|
Welcome, @nohlson! I was really excited to hear about this GSoC project at PyCon. This whole investigation for me was a quick afternoon hack. I only ever got as far as getting the build to complete -- I never even ran Python, let alone its test suite. I just got as far as thinking "someone with more time should work on this", and here you are ;) Also looking at this again, I see I didn't set the linker flags on IMHO, this seems like a hard flag to support. "Python without dlopen" would be a different beast -- maybe some very security conscious people would want that, but it would require tooling to statically link in all expected extension modules (some of that tooling already exists elsewhere). So, personally I'd defer solving that one for now (but I'm not the GSoC mentor, that's just my opinion). |
|
I would like to get some discussion going about performance impacts of enabling options and how much we would be willing to conceed in performance for safety. Here is an example of a cypthon baseline pyperformance benchmark vs. a build with multiple performance-impacting options enabled broke down by benchmark category:
To see all the benchmarks run and the comparison between builds here is more detail: Benchamrk Comparison (baseline vs. hardened build)Benchmarks with tag 'apps':2to3: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 306 ms +- 1 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 314 ms +- 1 ms: 1.03x slower Geometric mean: 1.03x slower Benchmarks with tag 'asyncio':async_tree_none: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 443 ms +- 22 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 460 ms +- 22 ms: 1.04x slower Benchmark hidden because not significant (3): async_tree_eager_io, async_tree_memoization_tg, async_tree_none_tg Geometric mean: 1.04x slower Benchmarks with tag 'math':float: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 86.6 ms +- 0.7 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 92.2 ms +- 0.9 ms: 1.06x slower Geometric mean: 1.03x slower Benchmarks with tag 'regex':regex_compile: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 155 ms +- 1 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 158 ms +- 1 ms: 1.02x slower Geometric mean: 1.00x faster Benchmarks with tag 'serialize':json_dumps: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 13.9 ms +- 0.2 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 15.6 ms +- 0.2 ms: 1.12x slower Geometric mean: 1.08x slower Benchmarks with tag 'startup':python_startup: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 10.3 ms +- 0.0 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 10.6 ms +- 0.0 ms: 1.03x slower Geometric mean: 1.03x slower Benchmarks with tag 'template':genshi_text: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 26.9 ms +- 0.2 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 27.6 ms +- 0.2 ms: 1.02x slower Geometric mean: 1.03x slower All benchmarks:2to3: Mean +- std dev: [two_baselines_and_tldr/config_1/pyperf_output.json] 306 ms +- 1 ms -> [two_baselines_and_tldr/config_3/pyperf_output.json] 314 ms +- 1 ms: 1.03x slower Benchmark hidden because not significant (6): async_tree_eager_io, async_tree_memoization_tg, async_tree_none_tg, asyncio_tcp, asyncio_websockets, mdp Geometric mean: 1.04x slower I am putting together some analysis of how individual options affect performance that I will share but would like to get some optionions concerning which benchmarks can't afford to take a performance hit. For example I would be less concerned about startup and docutils benchmarks as regex and math benchmarks which could be used at high frequency in applications. |
|
I think, unfortunately, the answer to that is "it depends". Startup really matters for some applications, and not others, for example. Likewise, security really matters in some contexts, but not others. It's hard to speculate at the beginning of this project, but maybe the end result will be to make it easy to make a security-hardened build at the expense of performance when the end user wants to make that tradeoff. I like the idea of breaking this out by individual flags, so we can see which have the most impact. It might also be possible that we can reduce the impact of some of the options by changing how some code is written in CPython, i.e. if an option makes some unsafe C feature slower, maybe we try to stop using that unsafe C feature if we can ;) We have a whole set of standard benchmarking machines at Microsoft that are set up to get the results as-reproducible-as-possible. If you create a branch on your fork of CPython with some proposed changes, you can ping me and I can kick off a run, and the results will show up automatically on iscpythonfastyet.com. Unfortunately, we can't automate triggering those runs for security reasons, but it's really easy for me so don't hesitate to ask me and I can get to it pretty quickly during my working hours. |
Feature or enhancement
Proposal:
At a recent meeting of OpenSSF's Memory Safety SIG, I became aware of the C/C++ hardening guide they are putting together.
At a high-level, they recommend compiling with the following flags:
(
-shareddoesn't really make sense as a global CFLAG, so I removed it.)When compiling on most x86 architectures (amd64, i386 and x32), add:
At @sethmlarson's urging, I compiled CPython on Linux/x86_64/gcc with these flags. From the complete build log, there are 3,084 warnings, but otherwise the result builds and passes all unit tests.
The warnings are of these types: (EDIT: Table updated to not double count the same line)
**Top warnings per file.**
I am not a security expert, so I don't know a good way to assess how many of these are potentially exploitable, and how many are harmless false positives. Some are probably un-resolvable (format-literal is pretty hard to avoid when wrapping
sprintf, for example).At a high level, I think the process to address these and make incremental progress maybe looks something like:
But this is just to start the discussion about how to move forward.
Has this already been discussed elsewhere?
No response given
Links to previous discussion of this feature:
No response
Linked PRs
The text was updated successfully, but these errors were encountered: