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

IP Assignment and Non-requirement on Address Architecture #12

Closed
mirjak opened this issue Dec 16, 2020 · 19 comments · Fixed by #29
Closed

IP Assignment and Non-requirement on Address Architecture #12

mirjak opened this issue Dec 16, 2020 · 19 comments · Fixed by #29

Comments

@mirjak
Copy link

mirjak commented Dec 16, 2020

The document currently states as a non-requirement:

It does not discuss how the IPs assigned are determined, managed, or translated.

However, at the same time there is a requirement on IP assignment. It's actually bit unclear to me what this non-requirement is meaning.

Further I think, there is more then just the proxy or client requesting an IP address. It might be okay for the proxy to select an IP address but then we maybe need a way to expose that to the client. Or the client might want to specify some restrictions about assignment like "use the same IP for this CONNECT request than for another" or "make sure all my request to this/these server(s) use the same IP address". Would be nice to either cover these things in the IP assignment requirement as well or formulate the requirement on a more high-level, e.g. require signaling to exchange information and restrictions about IP address assignments.

@DavidSchinazi
Copy link
Collaborator

@mirjak can you clarify what the problem is? Currently assignment is in-scope, determining the address is not.

@mirjak
Copy link
Author

mirjak commented Dec 17, 2020

I was more wondering about the work "translated" here because I think we need to discuss address translation. However, I'd probably say that if e.g. the client is providing an IP address range that would be covered for me as how the proxy "determines" the address. Maybe the wording is just not clear here. Maybe this needs to be explained more what the concern/intention is here. I rather, however, had the feeling that having this text might be just be unnecessary.

@achernya
Copy link
Collaborator

achernya commented Jan 7, 2021

I agree with @DavidSchinazi that I do not see a problem here. I do not think we need to discuss address translation; can you describe a use case that informs requirements here that would not be well-served considering the current components out-of-scope?

@mirjak
Copy link
Author

mirjak commented Jan 8, 2021

During chartering we discussed the use case where NAT is a way to conceal the client IP address from the server for privacy purposed. In this case the client neesd to be sure that the proxy does NAT. Another case is WebRTC where the client needs to know the outfacing port and address of the proxy. These are also cases that I would like to see considered and supported and seem to be excluded by this non-requirement. On the other side I still don't understand what the purpose of this non-requirement actually is. If there is another purpose you need to be more specific. Otherwise I still propose to simply remove it.

@achernya
Copy link
Collaborator

achernya commented Jan 8, 2021

I just re-read the charter text -- the only mention of NAT is "Impacts on [...] NAT rebinding [...] are not anticipated".

I believe the current text allows for the NAT or IP/port binding to be done as extensions. All the current draft states is that it's outside the scope of the core protocol, as it is a complex and involved topic. Would you like to see a sentence afterwards that such discussion is expected to be as part of extensions?

@DavidSchinazi
Copy link
Collaborator

@mirjak The client does not need to know that the proxy is using NAT, the client only needs to know that its assigned IP address doesn't leak privacy information. I still don't see a reason to bring NAT in scope.

@mirjak
Copy link
Author

mirjak commented Jan 8, 2021

@achernya You need to read the text in the charter completely:

Impacts on address migration, NAT rebinding, and future multipath mechanisms of QUIC are not anticipated. However, the working group should document these impacts, or those of any other QUIC developments, if they arise.

This paragraph of on changes or extension to QUIC which are not anticipated. This is not about any proxy function itself.

@mirjak
Copy link
Author

mirjak commented Jan 8, 2021

@DavidSchinazi I think we are still taking past each other as we have a different IP proxying solution in mind. One solution to provide an IP address that doesn't leak privacy information is to perform NAT at the proxy. I'm not saying that this is the solution we need to end up with but I don't see a reason to rule it out from the beginning.

@DavidSchinazi
Copy link
Collaborator

@mirjak what makes you think we're ruling it out? It being out of scope just means it doesn't need to be in the core protocol. WireGuard and IPsec both do not discuss any need for NAT, but are commonly deployed in combination with a NAT. MASQUE will be the same.

@mirjak
Copy link
Author

mirjak commented Jan 8, 2021

The difference that I explicitly would like to see NAT in masque not as a separate function that can be deployed together with the masque proxy but as an integrated function that the proxy explicitly provides and the client explicitly can request.

@achernya
Copy link
Collaborator

achernya commented Jan 8, 2021

@mirjak I believe the charter text supports my position. We are not affecting, or impacting, QUIC developments.

I think whether NAT is a separate function or not is orthogonal: extensions can be implemented as integrated or external capabilities. A client can explicitly request a capability with an extension.

@mirjak
Copy link
Author

mirjak commented Jan 8, 2021

I don't think that we reached agreement that this should only be an extension and not part of the core protocol. An even if we end up as an extension that means to me that it is not a non-requirement. I think this need more discussion with the rest of the working group (hopefully in the meeting next week).

@mirjak
Copy link
Author

mirjak commented Jan 20, 2021

I created PR #21 for the address assignment section.

However, the now requirements part is still not clear to me. I would actually like to see a requirement that NAT can be handled as integrated part of the masque proxy and not only as a separate function.

@achernya
Copy link
Collaborator

I took a look at PR #21 but I don't think it's suitable for inclusion in the current state. I'm having a hard time understanding what you are trying to achieve, which is hindered by the typos and other issues in the proposed text. Let's continue this discussion on the PR thread.

@tfpauly
Copy link

tfpauly commented Apr 15, 2021

I think we can fix this by changing this language to be clearly about the "core" CONNECT-IP spec, but not rule out extensions.

So change this:

Similarly, "ownership" of an IP range is out of scope. If an endpoint communicates to its peer that it can allocate addresses from a range, or route traffic to a range, the peer has no obligation to trust that information. Whether or not to trust this information is left to individual implementations and deployments.

to this:

Similarly, "ownership" of an IP range is out of scope for the core protocol. If an endpoint communicates to its peer that it can allocate addresses from a range, or route traffic to a range, the peer has no obligation to trust that information. Whether or not to trust this information is left to extensions or implementations.

@achernya
Copy link
Collaborator

Tommy, would you like to submit a PR or have one of us prepare it?

@tfpauly
Copy link

tfpauly commented Apr 15, 2021

David beat me to it =)

@mirjak
Copy link
Author

mirjak commented Apr 16, 2021

I still don't really know why we need this text at all in the draft. And I really, really think that if a peer provides an IP range, we must at minimum discuss this in the security consideration section.

@chris-wood
Copy link

Based on the interim meeting and April 27 consensus call, there is consensus to merge #29 and close this issue. Editors, can you please do so?

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 a pull request may close this issue.

5 participants