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

libstore/openStore: fix stores with IPv6 addresses #4319

Merged
merged 1 commit into from Dec 9, 2020

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Dec 5, 2020

In nixStable (2.3.7 to be precise) it's possible to connect to stores
using an IPv6 address:

nix ping-store --store ssh://root@2001:db8::1

This is also useful for nixops(1) where you could specify an IPv6
address in deployment.targetHost.

However, this behavior is broken on nixUnstable and fails with the
following error:

$ nix store ping --store ssh://root@2001:db8::1
don't know how to open Nix store 'ssh://root@2001:db8::1'

This happened because openStore from libstore uses the parseURL
function from libfetchers which expects a valid URL as defined in
RFC2732. However, this is unsupported by ssh(1):

$ nix store ping --store 'ssh://root@[2001:db8::1]'
cannot connect to 'root@[2001:db8::1]'

This patch now allows both ways of specifying a store (root@2001:db8::1) and
also root@[2001:db8::1] since the latter one is useful to pass query
parameters to the remote store.

In order to achieve this, the following changes were made:

  • The URL regex from url-parts.hh now allows an IPv6 address in the
    form 2001:db8::1 and also [2001:db8::1].

  • In libstore, a new function named extractConnStr ensures that a
    proper URL is passed to e.g. ssh(1):

    • If a URL looks like either [2001:db8::1] or root@[2001:db8::1],
      the brackets will be removed using a regex. No additional validation
      is done here as only strings parsed by parseURL are expected.

    • In any other case, the string will be left untouched.

Unresolved questions:

  • I'm not really sure whether we want to allow both variants of IPv6
    addresses in the URL parser. However it should be noted that both seem
    to be possible according to RFC2732:

    This document incudes an update to the generic syntax for Uniform
    Resource Identifiers defined in RFC 2396 [URL]. It defines a syntax
    for IPv6 addresses and allows the use of "[" and "]" within a URI
    explicitly for this reserved purpose.

  • Currently, it's not supported to specify a port number behind the
    hostname, however it seems as this is not really supported by the URL
    parser. Hence, this is probably out of scope here.

@Ma27
Copy link
Member Author

Ma27 commented Dec 7, 2020

cc @edolstra would you mind taking a look? :)

src/libstore/store-api.cc Outdated Show resolved Hide resolved
In `nixStable` (2.3.7 to be precise) it's possible to connect to stores
using an IPv6 address:

  nix ping-store --store ssh://root@2001:db8::1

This is also useful for `nixops(1)` where you could specify an IPv6
address in `deployment.targetHost`.

However, this behavior is broken on `nixUnstable` and fails with the
following error:

  $ nix store ping --store ssh://root@2001:db8::1
  don't know how to open Nix store 'ssh://root@2001:db8::1'

This happened because `openStore` from `libstore` uses the `parseURL`
function from `libfetchers` which expects a valid URL as defined in
RFC2732. However, this is unsupported by `ssh(1)`:

  $ nix store ping --store 'ssh://root@[2001:db8::1]'
  cannot connect to 'root@[2001:db8::1]'

This patch now allows both ways of specifying a store (`root@2001:db8::1`) and
also `root@[2001:db8::1]` since the latter one is useful to pass query
parameters to the remote store.

In order to achieve this, the following changes were made:

* The URL regex from `url-parts.hh` now allows an IPv6 address in the
  form `2001:db8::1` and also `[2001:db8::1]`.

* In `libstore`, a new function named `extractConnStr` ensures that a
  proper URL is passed to e.g. `ssh(1)`:

  * If a URL looks like either `[2001:db8::1]` or `root@[2001:db8::1]`,
    the brackets will be removed using a regex. No additional validation
    is done here as only strings parsed by `parseURL` are expected.

  * In any other case, the string will be left untouched.

* The rules above only apply for `LegacySSHStore` and `SSHStore` (a.k.a
  `ssh://` and `ssh-ng://`).

Unresolved questions:

* I'm not really sure whether we want to allow both variants of IPv6
  addresses in the URL parser. However it should be noted that both seem
  to be possible according to RFC2732:

  > This document incudes an update to the generic syntax for Uniform
  > Resource Identifiers defined in RFC 2396 [URL].  It defines a syntax
  > for IPv6 addresses and allows the use of "[" and "]" within a URI
  > explicitly for this reserved purpose.

* Currently, it's not supported to specify a port number behind the
  hostname, however it seems as this is not really supported by the URL
  parser. Hence, this is probably out of scope here.
@Ma27
Copy link
Member Author

Ma27 commented Dec 9, 2020

@edolstra fixed :)

@blaggacao
Copy link
Contributor

blaggacao commented Mar 16, 2021

@Ma27 what do you think of #4490 ?
It seems that the %ifidentifier is not properly recognized. Would that be fixable, in principle?

Duplicate #4643 has additional context.

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

4 participants