Skip to content

Conversation

@smoser
Copy link
Member

@smoser smoser commented Feb 23, 2024

The change here patches addresses CVE-2023-27043 by backporting a 'strict' keyword argument to the email.getaddresses and email.parseaddr function. The change may break some consumers as it does change the behavior.

After this change is landed, python 3.11 (and 3.12 via #13847) will behave like python master with regard to these two functions. Existing code for 3.11 or 3.12 will not pass the 'strict' argument and the default is None. The functions are modified to check for a None value and consult an environment variable for the desired behavior.

The environment variable PYTHON_EMAIL_STRICT_PARSING_DEFAULT will default will considered 'true' by default (unset or empty string). If you wish to enable the previous behavior, the the environment variable should be set to 'false'.

Please see comment below for background.

@smoser smoser marked this pull request as draft February 23, 2024 18:38
@smoser
Copy link
Member Author

smoser commented Feb 23, 2024

Some background and links on CVE-2023-27043 in python.

  1. CVE-2023-27043 is classified as medium (5.3) it was reported to cpython as issue 102988.
  2. A fix landed on main branch as commit 18dfbd0357 through PR #105127
  3. A regression issue was opened as issue 106669 and the prior fix was reverted in commit a31dea1feb61 in PR #106733
  4. A new fix was landed in main as commit
    4a153a1d3b via PR #111116.
  5. Current (2024-02-17) upstream branches for 3.11 and 3.12 do not include a fix.

The upstream fix in 4 adds a 'strict' parameter to getaddresses() and parseaddr() functions and enables it by default (which will break some users).

Thus, cherry-picking the upstream change to 3.11 and 3.12 would break some users as described in issue 106669

The options we have are:
a. cherry-pick the upstream fix as is, which will break some users.
b. cherry-pick the upstream fix but turn 'strict' off by default. Users won't be broken, but will have to 'opt-in' through code change to the remediation.
c. Follow RHEL change, allowing environment variable or config file to change the default behavior.
d. do not address the CVE.

Other responses are:

  • Ubuntu has deferred so far.
  • RedHat addressed by defaulting to strict=on and allowing behavior change without code change via either an environment variable (PYTHON_EMAIL_DISABLE_STRICT_ADDR_PARSING) or config file (/etc/python/email.cfg)

--
[edit - updated description of RHEL response, I had originally read it
incorrectly]
[edit 2 - clarified that 3.12 does not include fix]

@dlorenc
Copy link
Member

dlorenc commented Feb 23, 2024

c. Follow RHEL change, allowing environment variable or config file to change the default behavior.

This one gets my vote (just consider it one vote though)

@ware
Copy link

ware commented Feb 26, 2024

b. cherry-pick the upstream fix but turn 'strict' off by default. Users won't be broken, but will have to 'opt-in' through code change to the remediation.
c. Follow RHEL change, allowing environment variable or config file to change the default behavior.

I think either one of these two solutions would be acceptable.

@xnox
Copy link
Member

xnox commented Feb 26, 2024

Debian codesearch for getaddress in python filetypes

https://codesearch.debian.net/search?q=getaddress+filetype%3Apython&literal=1&perpkg=1

$ curl -s https://codesearch.debian.net/results/8ee7eac86202d906/packages.txt | wc
     45

Gives me 45 packages. I can cross check by hand, how many of these are in any of our packages or images. Assuming the scope is small, and because our goal is to provide security to people, my preference is to

  • ship updated change
  • default to secure
  • provide opt-out for 3-rd party customer code compat

As the point of chainguard is to ship security updates without friction, meaning no opt-in needed.

@xnox
Copy link
Member

xnox commented Feb 26, 2024

we need advisory text to say that new strict behaviour is by default; and that if one is broken by this change they are either being attacked or they rely on vulnerable behaviour. And rhel "fix" is not fixed, as one has to do something to actually get the fix. And this is forward compatible with future python releases.

Why is this fix important? All of these representations are typically used to stage Password reset poisoning attacks. See for example these blogs that explain how "i'm just going to be using canonical representation of things" can go very wrong

https://cendyne.dev/posts/2022-02-18-user-provided-primary-keys.html

https://portswigger.net/web-security/host-header/exploiting/password-reset-poisoning

When using chainguard images, one should be secure by default - without needed to opt-into security.... One already did the opt-into security by choosing chainguard. Maybe I should do a blog post about this update.

@smoser
Copy link
Member Author

smoser commented Feb 26, 2024

Thanks @xnox. Your prompting made me read a bit more. the RHEL change opts into the new behavior by default and allows the user a code-change-free option to opt out.

I like this approach, and think we should go with that.

@dlorenc
Copy link
Member

dlorenc commented Feb 26, 2024

+1 to this, hardened by default.

@smoser smoser force-pushed the fix/python3.11-CVE-2023-27043 branch from c3183cb to 3e5cb3c Compare February 28, 2024 21:04
@smoser smoser marked this pull request as ready for review February 28, 2024 21:09
@smoser smoser force-pushed the fix/python3.11-CVE-2023-27043 branch 2 times, most recently from 3749dca to 5c92b5a Compare February 28, 2024 21:50
@smoser smoser force-pushed the fix/python3.11-CVE-2023-27043 branch from 5c92b5a to 16a20fe Compare February 28, 2024 22:09
@smoser
Copy link
Member Author

smoser commented Feb 29, 2024

I think this is ready to land, would appreciate a review. thanks.

@smoser smoser merged commit a717371 into wolfi-dev:main Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants