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

Full rewrite of entire NAT sec and some reordering #266

Closed
wants to merge 1 commit into from

Conversation

mirjak
Copy link
Contributor

@mirjak mirjak commented Feb 17, 2021

No description provided.

wire image. Extremely limited observation of upstream congestion may be
possible via the observation of CE markings on ECN-enabled QUIC traffic.
This section uses the colloquial term Network Address
Translation (NAT) to mean NAPT (section 2.2 of
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Translation (NAT) to mean NAPT (section 2.2 of
Translation (NAT) to mean NAPT (Section 2.2 of

Comment on lines +823 to +824
{{?RFC3022}}), which overloads several IP addresses to one IP address or to an
IP address pool, as commonly deployed in carrier-grade NATs or residential NATs.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{?RFC3022}}), which overloads several IP addresses to one IP address or to an
IP address pool, as commonly deployed in carrier-grade NATs or residential NATs.
{{?RFC3022}}), which rewrites addresses on flows from multiple IP addresses to
allow sharing of a limited pool of addresses, as commonly deployed in
carrier-grade NATs or residential NATs.

{{?RFC4787}} contains some guidance on building NATs to interact constructively
with a wide range of applications. This section extends the discussion to QUIC.

In summary, the advise is simple: devices that perform NAT should use the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In summary, the advise is simple: devices that perform NAT should use the
In summary, the advice is simple: devices that perform NAT should use the

with a wide range of applications. This section extends the discussion to QUIC.

In summary, the advise is simple: devices that perform NAT should use the
QUIC connection ID to map their internal state but only use the 4-tuple of IP
Copy link
Member

Choose a reason for hiding this comment

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

I don't know what "map their internal state means". If it means that the NAT can use extra knowledge of QUIC to learn more about how things are operating, then I don't know what use that has unless it is applied.

I would remove this extra piece, on the basis that it is confusing and misleading. Other text can cover the learnings.

addresses and ports as they would do for other UDP traffic or other transports
as well.

As already recognised in section {#sec-stateful} the QUIC connection ID is not
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
As already recognised in section {#sec-stateful} the QUIC connection ID is not
As already recognised in {{sec-stateful}} the QUIC connection ID is not

as well.

As already recognised in section {#sec-stateful} the QUIC connection ID is not
guaranteed to be constant over the life-time of a connection and a change of
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
guaranteed to be constant over the life-time of a connection and a change of
guaranteed to be constant over the lifetime of a connection. A NAT that uses connection IDs as part of its mapping will lose those mappings when a change of connection ID occurs.

This was missing the bit where you explain that the NAT is using connection IDs somehow.

Comment on lines +837 to +838
new state. Both should be avoided and indicate that a NAT device does not need
to be QUIC-aware.
Copy link
Member

Choose a reason for hiding this comment

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

This is just repetition of a previous statement.

new state. Both should be avoided and indicate that a NAT device does not need
to be QUIC-aware.

Section {#sec-stateful} also provides further recommendations on time-outs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Section {#sec-stateful} also provides further recommendations on time-outs
{{sec-stateful}} also provides further recommendations on timeouts

for stateful network functions such as NATs. For QUIC it is important that time-outs
are not extremely short, however, the general recommendation as provided in
{{RFC4787}} for UDP is also appropriate for QUIC and as such again not NAT
function are not required to be QUIC-aware.
Copy link
Member

Choose a reason for hiding this comment

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

This is getting quite repetitive. Maybe you can just list things that don't work:

  • Tracking connections based on connection ID (connection IDs change).
  • Changing NAT timeouts based on detecting QUIC (QUIC doesn't care).
  • Rewriting addresses to ensure stability (requires endpoint cooperation; enables amplification attacks).

@@ -809,20 +816,161 @@ off the connection ID may therefore cause undetectable and unrecoverable loss
of state in the middle of a connection. Use of connection ID specifically
discouraged for NAT applications.

## Passive Network Performance Measurement and Troubleshooting
## Network Address Translation (NAT)
Copy link
Member

Choose a reason for hiding this comment

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

This section is too long for me. I started reviewing it, but I lost the thread. I don't think that this is an improvement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually agree. I was just trying to keep all of the points in Martin Duke's original text (while removing redundancy). PR #272 is a proposal for a minimum text version. Maybe we can start from there and re-add things if needed.

@mirjak mirjak closed this Feb 18, 2021
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.

None yet

2 participants