Skip to content

Some IPv4 and IPv4-mapped IPv6 properties don't match #122792

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

Closed
sethmlarson opened this issue Aug 7, 2024 · 5 comments
Closed

Some IPv4 and IPv4-mapped IPv6 properties don't match #122792

sethmlarson opened this issue Aug 7, 2024 · 5 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error type-security A security issue

Comments

@sethmlarson
Copy link
Contributor

sethmlarson commented Aug 7, 2024

Bug report

Bug description:

The following properties on an IPv6Address don't match their IPv4Address counterparts when using an IPv6-mapped IPv4 address (ie ::ffff:<ipv4>):

  • is_multicast
  • is_reserved
  • is_link_local
  • is_global
  • is_unspecified

Proposed fix is to make all properties use their IPv4Address values for IPv4-mapped addresses.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@sethmlarson sethmlarson added type-bug An unexpected behavior, bug, or error type-security A security issue stdlib Python modules in the Lib dir labels Aug 7, 2024
gpshead pushed a commit that referenced this issue Sep 7, 2024
…Pv4 (GH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 7, 2024
…with IPv4 (pythonGH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 7, 2024
…with IPv4 (pythonGH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 7, 2024
…with IPv4 (pythonGH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 7, 2024
…with IPv4 (pythonGH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
gpshead pushed a commit that referenced this issue Sep 7, 2024
… with IPv4 (GH-122793) (GH-123814)

gh-122792: Make IPv4-mapped IPv6 address properties consistent with IPv4 (GH-122793)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
@jstasiak
Copy link
Contributor

jstasiak commented Sep 7, 2024

Hey, I learned about this issue just today and, as a user of this API, I'd like to express some opposition to it.

My reasons, in no particular order:

  1. The proposed change makes the API more unpredictable and confusing (somewhat subjective) by "unwrapping" the IPv4-mapped addresses.
  2. This change makes the properties inconsistent with the already documented behavior of the properties (and the PR, gh-122792: Make IPv4-mapped IPv6 address properties consistent with IPv4 #122793, doesn't attempt to change the documentation).
  3. The new behavior of the properties breaks backwards compatibility as code that already depends on the existing behavior (which I'll claim there are good reasons for) will now produce incorrect results. Breaking compatibility is fine if the old behavior is a legitimate mistake and didn't make much sense etc. but I don't see this here.
  4. The proposed change makes the API more limiting. Previously, as the user of this API, I was free to decide what to do in case of IPv4-mapped IPv6 addresses – I could unwrap them or not before checking is_link_local, for example. With this change I'm left with no API at all to do the latter.

cc @gpshead

@gpshead
Copy link
Member

gpshead commented Sep 7, 2024

I think we're lacking the motivating reason behind the change, @sethmlarson can explain more.

Agreed with (2), we need a docs update. But the merged change does not make it inconsistent, the ipaddress docs today are sparse and don't specify the exact behavior when these are used on IPv6Address. To me this change matches what I would assume they would do based on the existing terse docs. https://docs.python.org/3/library/ipaddress.html#ipaddress.IPv6Address.is_multicast

Regarding (3) / (4) do you have practical examples of actual code in use that does not want the new behavior? (got links?) That'd help reason about where this falls into breaking change vs bug fix categories.

Some general background: We've gotten security reports over the years about the ipaddress module in various places because people write code using it for security purposes and wind up surprised when it doesn't behave in the manner their specific application needed and thus maybe they did something undesirable with network traffic as a result.

@sethmlarson
Copy link
Contributor Author

@jstasiak Thanks for the question, indeed the primary motivation for this change is security, specifically folks using IP address filtering before establishing a connection. Since an IPv4-mapped IPv6 address gets established as an IPv4 address I figured these properties should match the mapped IPv4 address and not the IPv6 address that's mostly being used for framing. If you have better guidance about this case, please share!

@sethmlarson
Copy link
Contributor Author

@jstasiak Do you have an opinion on the above comment? Otherwise we will continue back-porting this change, we want to make sure we're not breaking any legitimate use-cases unintentionally.

ambv pushed a commit that referenced this issue Dec 3, 2024
… with IPv4 (GH-122793) (GH-123815)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
ambv added a commit that referenced this issue Dec 3, 2024
… with IPv4 (GH-122793) (GH-123818)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>

---------

Co-authored-by: Seth Michael Larson <seth@python.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv added a commit that referenced this issue Dec 3, 2024
… with IPv4 (GH-122793) (GH-123819)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

Co-authored-by: Seth Michael Larson <seth@python.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv added a commit to ambv/cpython that referenced this issue Dec 3, 2024
…s consistent with IPv4 (pythonGH-122793) (pythonGH-123819)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)

(cherry picked from commit b58da40)

Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Co-authored-by: Seth Michael Larson <seth@python.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv added a commit that referenced this issue Dec 3, 2024
…with IPv4 (GH-122793) (GH-123819) (GH-127571)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)
(cherry picked from commit b58da40)

Co-authored-by: Seth Michael Larson <seth@python.org>
@ambv
Copy link
Contributor

ambv commented Dec 3, 2024

Given no further comments by jstasiak I backported the rest. This issue is now resolved. Thanks, everyone.

@ambv ambv closed this as completed Dec 3, 2024
stsewd added a commit to readthedocs/readthedocs.org that referenced this issue Dec 5, 2024
urlparse can throw ValueError when parsing a URL, we didn't experience
this before because none of our tests were failing when calling
urlparse, but there was a new release of python that now fails parsing
some of our test cases python/cpython#122792.
arnout pushed a commit to buildroot/buildroot that referenced this issue Dec 13, 2024
gh-122792: Changed IPv4-mapped ipaddress.IPv6Address to consistently use the
mapped IPv4 address value for deciding properties.  Properties which have
their behavior fixed are is_multicast, is_reserved, is_link_local,
is_global, and is_unspecified.

python/cpython#122792

CVE-2024-9287, gh-124651: Properly quote template strings in venv activation
scripts.

python/cpython#124651

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
gentoo-bot pushed a commit to gentoo/cpython that referenced this issue Apr 9, 2025
…stent with IPv4 (pythonGH-122793) (pythonGH-123819) (pythonGH-127571)

Make IPv4-mapped IPv6 address properties consistent with IPv4.
(cherry picked from commit 76a1c5d)
(cherry picked from commit b58da40)

Co-authored-by: Seth Michael Larson <seth@python.org>
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-bug An unexpected behavior, bug, or error type-security A security issue
Projects
None yet
Development

No branches or pull requests

4 participants