Skip to content

gnrc_ipv6: only discard invalid source when assigned to interface [backport 2019.10]#12458

Merged
cgundogan merged 1 commit intoRIOT-OS:2019.10-branchfrom
miri64:backport/2019.10/gnrc_ipv6/fix/only-local-invalid-src
Oct 15, 2019
Merged

gnrc_ipv6: only discard invalid source when assigned to interface [backport 2019.10]#12458
cgundogan merged 1 commit intoRIOT-OS:2019.10-branchfrom
miri64:backport/2019.10/gnrc_ipv6/fix/only-local-invalid-src

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Oct 15, 2019

Backport of #12442

Contribution description

While it is correct to not use an invalid address as a source address, it is incorrect to assume that addresses not assigned to the interface (idx == -1 in the respective piece of code) are invalid: Other than classic forwarding via a FIB, forwarded packets utilizing a IPv6 routing header will pass this check, like any other packet sent by this node. The source address for these is not on the given node, so e.g. source routing is not possible at the moment.

Testing procedure

Testing procedures from #11970 should still work (was unable to test this atm due to lacking hardware).

With #12440 merged tests/gnrc_rpl_srh passes (requires dist/tools/tapsetup/tapsetup to be called before and also must be called with sudo).

Issues/PRs references

Follow-up to #11970
Fixes parts of #12436
Accompanies #12440

While it is correct to not use an invalid address as a source address,
it is incorrect to assume that addresses not assigned to the interface
(`idx == -1` in the respective piece of code) are invalid: Other than
classic forwarding via a FIB, forwarded packets utilizing a IPv6
routing header will pass this check, like any other packet sent by this
node. The source address for these is not on the given node, so e.g.
source routing is not possible at the moment.

(cherry picked from commit b5b52c7)
@miri64 miri64 added Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 15, 2019
@miri64 miri64 requested a review from cgundogan October 15, 2019 11:56
@miri64 miri64 added this to the Release 2019.10 milestone Oct 15, 2019
@cgundogan cgundogan added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Oct 15, 2019
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commits are identical.

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Oct 15, 2019

merge? :)

@cgundogan cgundogan merged commit 4f7c6af into RIOT-OS:2019.10-branch Oct 15, 2019
@miri64 miri64 deleted the backport/2019.10/gnrc_ipv6/fix/only-local-invalid-src branch October 15, 2019 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants