Implement DNS-based mirror bootstrap protocol #3947

hvr · 2016-10-07T20:59:11Z

This way cabal can bootstrap secure repos even if the primary Hackage
instance is currently unreachable, as long as there's at least one
reachable and working secure mirror available.

NB: This new code-path is only used for the initial bootstrap. Once the
repository cache has been bootstrapped, its @mirrors.json@ meta-data is
used instead.

See also haskell/hackage-security#171

mention-bot · 2016-10-07T20:59:13Z

@hvr, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edsko, @dcoutts and @ezyang to be potential reviewers.

hvr · 2016-10-07T21:03:24Z

For reference, here's how cabal update -v looks for the bootstrap-case:

$ ./dist-newstyle/build/x86_64-linux/ghc-8.0.1/cabal-install-1.25.0.0/c/cabal/build/cabal/cabal update -v
/usr/bin/nslookup '-query=TXT' _mirrors.hackage.haskell.org
located 2 mirrors for http://hackage.haskell.org/ :
- http://hackage.fpcomplete.com/
- http://objects-us-west-1.dream.io/hackage-mirror/
Selected mirror http://hackage.haskell.org/
Downloading root
/usr/bin/curl 'http://hackage.haskell.org/root.json' --output /tmp/transportAdapterGet16816927771714636915 --location --write-out '%{http_code}' --user-agent 'cabal-install/1.25.0.0 (linux; x86_64)' --silent --show-error --dump-header /tmp/curl-headers1957747793424238335.txt --header 'Cache-Control: no-transform'
Downloading the latest package list from hackage.haskell.org
Selected mirror http://hackage.haskell.org/
Downloading timestamp
/usr/bin/curl 'http://hackage.haskell.org/timestamp.json' --output /tmp/transportAdapterGet5965166491189641421 --location --write-out '%{http_code}' --user-agent 'cabal-install/1.25.0.0 (linux; x86_64)' --silent --show-error --dump-header /tmp/curl-headers10252023621350490027.txt --header 'Cache-Control: no-transform'
Downloading snapshot
/usr/bin/curl 'http://hackage.haskell.org/snapshot.json' --output /tmp/transportAdapterGet20448977631967513926 --location --write-out '%{http_code}' --user-agent 'cabal-install/1.25.0.0 (linux; x86_64)' --silent --show-error --dump-header /tmp/curl-headers13651805401540383426.txt --header 'Cache-Control: no-transform'
Downloading mirrors
/usr/bin/curl 'http://hackage.haskell.org/mirrors.json' --output /tmp/transportAdapterGet35005211521595368 --location --write-out '%{http_code}' --user-agent 'cabal-install/1.25.0.0 (linux; x86_64)' --silent --show-error --dump-header /tmp/curl-headers2947025671726956429.txt --header 'Cache-Control: no-transform'
Updating index
/usr/bin/curl 'http://hackage.haskell.org/01-index.tar.gz' --output /tmp/transportAdapterGet278722862233665123 --location --write-out '%{http_code}' --user-agent 'cabal-install/1.25.0.0 (linux; x86_64)' --silent --show-error --dump-header /tmp/curl-headers2145174067468703135.txt --header 'Cache-Control: no-transform' --header 'Range: bytes=52328178-52394862'
Updating index cache file
/home/hvr/.cabal/packages/hackage.haskell.org/01-index.cache ...
Index cache updated to index-state 2016-10-07T20:57:08Z

23Skidoo · 2016-10-07T21:24:36Z

cabal-install/Distribution/Client/Security/DNS.hs

@@ -0,0 +1,134 @@
+module Distribution.Client.Security.DNS


Isn't hackage-security a more appropriate place for this functionality?

@23Skidoo Well, @dcoutts asked me to implement it in cabal-install for the time being...

ezyang · 2016-10-09T05:26:03Z

cabal-install/Distribution/Client/Security/DNS.hs

+      where
+        ns = take (length s - 8) s
+
+-- | Parse output of @nslookup -query=TXT $HOSTNAME@ tolerantly


Are you really sure you don't want to use a parser combinator for this? ;)

Well, what's the benefit? :-)

...well, ideally you would have written it with parser combinators to begin with :P

ezyang · 2016-10-09T05:29:49Z

cabal-install/Distribution/Client/Security/DNS.hs

+--
+queryBootstrapMirrors :: Verbosity -> URI -> IO [URI]
+queryBootstrapMirrors verbosity repoUri
+  | uriScheme repoUri `elem` ["http:","https:"]


I don't understand this test. Whether or not the hostname component of a URI is DNS resolvable has nothing to do with the scheme; e.g., ftp also does DNS. file is not a good excuse because these have a blank hostname, e.g. file:///foo (that's why there's three slashes!)

...what kind of test are you suggesting instead?

Test if uriAuthority is Just.

ezyang · 2016-10-09T05:31:28Z

cabal-install/Distribution/Client/Security/DNS.hs

+                  evaluate (force $ extractMirrors mirrorsDnsName out)
+
+              mirrors <- case mirrors' of
+                Left e -> (e::SomeException) `seq` return []


Are you sure you don't want to warn in this case?

Well, I do so implicitly when null mirrors a few lines below... or do you want to see e displayed?

I want to see e. If the code is doing something weird that e could have critical information for debugging.

ezyang · 2016-10-09T05:32:30Z

cabal-install/Distribution/Client/GlobalFlags.hs

-      requiresBootstrap <- Sec.requiresBootstrap r
+    requiresBootstrap <- withRepo [] Sec.requiresBootstrap
+
+    mirrors <- if requiresBootstrap


It seems worth mentioning (in info) that we're querying DNS to get some mirrors. And it'll only happen once.

I considered that, but I didn't want to have to duplicate the detection in queryBootstrapMirrors for when a DNS query is going to occur at all...

So what about:

if requiresBootstrap then do info verbosity $ "Trying to locate mirrors via DNS for initial bootstrap of secure repository (" ++ show remoteRepoURI ++ ") ..." Sec.DNS.queryBootstrapMirrors verbosity remoteRepoURI else ...

?

Yes, that is what I had in mind.

ezyang · 2016-10-09T05:32:54Z

Seems reasonable.

23Skidoo · 2016-10-09T09:47:48Z

I'm absolutely not an expert on these matters, but can this be vulnerable to DNS spoofing/cache poisoning or some similar attack?

hvr · 2016-10-09T12:35:49Z

@23Skidoo we don't rely on DNS to be trustworthy; the trust is established via the signatures inside .json meta-data we retrieve. We already have this problem with the DNS entry for hackage.haskell.org which could be compromised, so relying on a 2nd one _mirrors.hackage.haskell.org in the same SOA doesn't make it worse IMO. I'll add a comment to that effect in the code (as discussed on IRC)

This way `cabal` can bootstrap secure repos even if the primary Hackage instance is currently unreachable, as long as there's at least one reachable and working secure mirror available. NB: This new code-path is only used for the initial bootstrap. Once the repository cache has been bootstrapped, its `mirrors.json` meta-data is used instead. See also haskell/hackage-security#171

23Skidoo · 2016-10-10T08:51:51Z

/cc @edsko

edsko · 2016-10-10T08:57:27Z

Commenting on the idea, not on the code: it seems reasonable to me. Like @hvr says, we don't rely on DNS information to be accurate to guard against attacks; as long as we get pointed to a server and that server has metadata signed by the root keys we expect, we're fine (of course, the distribution of those root keys is a different matter but that's orthogonal to this PR).

dcoutts · 2016-10-11T13:15:13Z

I havn't reviewed the code yet, but I concur with edsko about the security.

23Skidoo · 2016-10-11T22:09:13Z

Merged, thanks!

hvr added the cabal-install: hackage-security label Oct 7, 2016

hvr added this to the 2.0 milestone Oct 7, 2016

23Skidoo reviewed Oct 7, 2016

View reviewed changes

hvr force-pushed the pr/mirrors-bootstrap branch 3 times, most recently from 75dbee7 to d23b1e3 Compare October 8, 2016 13:34

ezyang reviewed Oct 9, 2016

View reviewed changes

hvr force-pushed the pr/mirrors-bootstrap branch from d23b1e3 to 7bc11c6 Compare October 9, 2016 13:45

hvr added 2 commits October 9, 2016 23:04

Use local Prelude in D.C.GlobalFlags

3fb519a

hvr force-pushed the pr/mirrors-bootstrap branch 2 times, most recently from 176ff86 to ae24c5c Compare October 10, 2016 06:35

23Skidoo merged commit 34eecf4 into haskell:master Oct 11, 2016

hvr deleted the pr/mirrors-bootstrap branch August 10, 2017 08:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement DNS-based mirror bootstrap protocol #3947

Implement DNS-based mirror bootstrap protocol #3947

hvr commented Oct 7, 2016

mention-bot commented Oct 7, 2016

hvr commented Oct 7, 2016

23Skidoo Oct 7, 2016

hvr Oct 8, 2016

ezyang Oct 9, 2016

hvr Oct 9, 2016

ezyang Oct 9, 2016

ezyang Oct 9, 2016

hvr Oct 9, 2016

ezyang Oct 9, 2016

hvr Oct 10, 2016

ezyang Oct 9, 2016

hvr Oct 9, 2016

ezyang Oct 9, 2016

hvr Oct 10, 2016

ezyang Oct 9, 2016

hvr Oct 9, 2016 •

edited by 23Skidoo

Loading

ezyang Oct 9, 2016

hvr Oct 10, 2016

ezyang commented Oct 9, 2016

23Skidoo commented Oct 9, 2016

hvr commented Oct 9, 2016

23Skidoo commented Oct 10, 2016

edsko commented Oct 10, 2016

dcoutts commented Oct 11, 2016

23Skidoo commented Oct 11, 2016

Implement DNS-based mirror bootstrap protocol #3947

Implement DNS-based mirror bootstrap protocol #3947

Conversation

hvr commented Oct 7, 2016

mention-bot commented Oct 7, 2016

hvr commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hvr Oct 9, 2016 • edited by 23Skidoo Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezyang commented Oct 9, 2016

23Skidoo commented Oct 9, 2016

hvr commented Oct 9, 2016

23Skidoo commented Oct 10, 2016

edsko commented Oct 10, 2016

dcoutts commented Oct 11, 2016

23Skidoo commented Oct 11, 2016

hvr Oct 9, 2016 •

edited by 23Skidoo

Loading