Skip to content
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

Clarification on validateEgressID Logic for SCION Packet Processing #4497

Closed
mlimbeck opened this issue Mar 28, 2024 · 1 comment · Fixed by #4502
Closed

Clarification on validateEgressID Logic for SCION Packet Processing #4497

mlimbeck opened this issue Mar 28, 2024 · 1 comment · Fixed by #4502
Assignees
Labels
bug Something isn't working

Comments

@mlimbeck
Copy link

mlimbeck commented Mar 28, 2024

Description:
cc: @jcp19
During the verification of router logic within the VerifiedSCION project, I encountered a possible problem in the scionPacketProcessor's validateEgressID function that might miss an additional check. The code snippet in question is as follows:

func (p *scionPacketProcessor) validateEgressID() (processResult, error) {
	pktEgressID := p.egressInterface()
	_, ih := p.d.internalNextHops[pktEgressID]
	_, eh := p.d.external[pktEgressID]
	if !ih && !eh {
		errCode := slayers.SCMPCodeUnknownHopFieldEgress
	....

This checks whether a received packet can be forwarded either internally or externally based on the pktEgressID. My concern revolves around the scenario where p.ingressID == 0, indicating that the packet has already entered the Autonomous System (AS) and will be forwarded by the router internally. This situation seems to contradict the SCION protocol's design, where a packet is only handled twice within an AS: once by the ingress router and once by the egress router. This case is also not included in the following comment in the scionPacketProcessor's process function:

// Outbound: pkt leaving the local IA. This Could be:
// * Pure outbound: from this AS, in via internal, out via external.
// * ASTransit in: from another AS, in via external, out via internal to other BR.
// * ASTransit out: from another AS, in via internal from other BR, out via external.
// * BRTransit: from another AS, in via external, out via external.`

Question:

Whenever a packet is leaving the AS through another Border Router, shouldn't it be ensured that p.ingressID != 0?

func (p *scionPacketProcessor) process() (processResult, error) {
	...
	...
	// Is it required to ensure that p.ingressID != 0 at this stage?
	// ASTransit in: pkt leaving this AS through another BR.
	if a, ok := p.d.internalNextHops[egressID]; ok {

Am I misunderstanding something, or is there actually a situation where p.ingressID == 0 and the packet is still meant to be forwarded internally?

@mlimbeck mlimbeck added the bug Something isn't working label Mar 28, 2024
@matzf
Copy link
Contributor

matzf commented Mar 28, 2024

Thanks, @mlimbeck, for the detailed bug report.

Is there actually a situation where p.ingressID == 0 and the packet is still meant to be forwarded internally?

You're right, there is no situation where a packet is received on interface 0 and should be forwarded internally.

Here's a small script that creates a packet that will be internally "bounced" between the routers of an AS: https://gist.github.com/matzf/13de17cef9d0b29811a1660b3f52426f

You found a nasty bug! 💯

@matzf matzf self-assigned this Apr 8, 2024
matzf added a commit to matzf/scion that referenced this issue Apr 8, 2024
A packet received on the internal interface would previously be
"bounced" to the responsible egress router.
This is forbidden in the SCION design and was an accidental mis-feature
of the processing logic.

Fixes scionproto#4497
matzf added a commit to matzf/scion that referenced this issue Apr 11, 2024
A packet received on the internal interface would previously be
"bounced" to the responsible egress router.
This is forbidden in the SCION design and was an accidental mis-feature
of the processing logic.

Fixes scionproto#4497
matzf added a commit to matzf/scion that referenced this issue Apr 19, 2024
A packet received on the internal interface would previously be
"bounced" to the responsible egress router.
This is forbidden in the SCION design and was an accidental mis-feature
of the processing logic.

Fixes scionproto#4497
matzf added a commit to matzf/scion that referenced this issue May 2, 2024
A packet received on the internal interface would previously be
"bounced" to the responsible egress router.
This is forbidden in the SCION design and was an accidental mis-feature
of the processing logic.

Fixes scionproto#4497
matzf added a commit that referenced this issue May 2, 2024
A packet received on the internal interface would previously be
"bounced" to the responsible egress router.
This is forbidden in the SCION design and was an accidental mis-feature
of the processing logic.

Fixes #4497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants