[PATCH 1 of 2] streamclone: consider secret changesets (BC) (issue5589)

Fri Jun 9 14:22:10 EDT 2017

On Fri, Jun 9, 2017 at 10:49 AM, Gregory Szorc <gregory.szorc at gmail.com>
wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1497030073 25200
> #      Fri Jun 09 10:41:13 2017 -0700
> # Node ID 9e4e3022c76912d5d0efd8b93b90854d0abbfbbf
> # Parent  326c0e2c1a1d59e07f4c9d86f81e4419c3d779d8
> streamclone: consider secret changesets (BC) (issue5589)
>
> Previously, a repo containing secret changesets would be served via
> stream clone, transferring those secret changesets. While secret
> changesets aren't meant to imply strong security (if you really
> want to keep them secret, others shouldn't have read access to the
> repo), we should at least make an effort to protect secret changesets
> when possible.
>
> After this commit, we no longer serve stream clones for repos
> containing secret changesets by default. This is backwards
> incompatible behavior. In case anyone is relying on the behavior,
> we provide a config option to opt into the old behavior.
>
> Note that this defense is only beneficial for remote repos
> accessed via the wire protocol: if a client has access to the
> files backing a repo, they can get to the raw data and see secret
> revisions.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1658,6 +1658,10 @@ Controls generic server settings.
>      the write lock while determining what data to transfer.
>      (default: True)
>
> +``uncompressedallowsecret``
> +    Whether to allow stream clones when the repository contains secret
> +    changesets. (default: False)
> +
>  ``preferuncompressed``
>      When set, clients will try to use the uncompressed streaming
>      protocol. (default: False)
> diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
> --- a/mercurial/streamclone.py
> +++ b/mercurial/streamclone.py
> @@ -13,6 +13,7 @@ from .i18n import _
>  from . import (
>      branchmap,
>      error,
> +    phases,
>      store,
>      util,
>  )
> @@ -162,9 +163,18 @@ def maybeperformlegacystreamclone(pullop
>
>          repo.invalidate()
>
> -def allowservergeneration(ui):
> +def allowservergeneration(repo):
>      """Whether streaming clones are allowed from the server."""
> -    return ui.configbool('server', 'uncompressed', True, untrusted=True)
> +    if not repo.ui.configbool('server', 'uncompressed', True,
> untrusted=True):
> +        return False
> +
> +    # The way stream clone works makes it impossible to hide secret
> changesets.
> +    # So don't allow this by default.
> +    secret = phases.hassecret(repo)
> +    if secret:
> +        return repo.ui.configbool('server', 'uncompressedallowsecret',
> False)
> +
> +    return True
>
>  # This is it's own function so extensions can override it.
>  def _walkstreamfiles(repo):
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -754,7 +754,7 @@ def _capabilities(repo, proto):
>      """
>      # copy to prevent modification of the global list
>      caps = list(wireprotocaps)
> -    if streamclone.allowservergeneration(repo.ui):
> +    if streamclone.allowservergeneration(repo):
>          if repo.ui.configbool('server', 'preferuncompressed', False):
>              caps.append('stream-preferred')
>          requiredformats = repo.requirements & repo.supportedformats
> @@ -946,7 +946,7 @@ def stream(repo, proto):
>      capability with a value representing the version and flags of the repo
>      it is serving. Client checks to see if it understands the format.
>      '''
> -    if not streamclone.allowservergeneration(repo.ui):
> +    if not streamclone.allowservergeneration(repo):
>          return '1\n'
>
>      def getstream(it):
> diff --git a/tests/test-clone-uncompressed.t b/tests/test-clone-
> uncompressed.t
> --- a/tests/test-clone-uncompressed.t
> +++ b/tests/test-clone-uncompressed.t
> @@ -49,6 +49,77 @@ Clone with background file closing enabl
>    bundle2-input-bundle: 1 parts total
>    checking for updated bookmarks
>
> +Cannot stream clone when there are secret changesets
> +
> +  $ hg -R server phase --force --secret -r tip
> +  $ hg clone --uncompressed -U http://localhost:$HGPORT secret-denied
> +  warning: stream clone requested but server has them disabled
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +
> +  $ killdaemons.py
> +
> +Streaming of secrets can be overridden by server config
> +
> +  $ cd server
> +  $ hg --config server.uncompressedallowsecret=true serve -p $HGPORT -d
> --pid-file=hg.pid
> +  $ cat hg.pid > $DAEMON_PIDS
> +  $ cd ..
> +
> +  $ hg clone --uncompressed -U http://localhost:$HGPORT secret-allowed
> +  streaming all changes
> +  1027 files to transfer, 96.3 KB of data
> +  transferred 96.3 KB in * seconds (*/sec) (glob)
> +  searching for changes
> +  no changes found
> +
> +  $ killdaemons.py
> +
> +Verify interaction between preferuncompressed and secret presence
> +
> +  $ cd server
> +  $ hg --config server.preferuncompressed=true serve -p $HGPORT -d
> --pid-file=hg.pid
> +  $ cat hg.pid > $DAEMON_PIDS
> +  $ cd ..
> +
> +  $ hg clone -U http://localhost:$HGPORT preferuncompressed-secret
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +
> +  $ killdaemons.py
> +
> +Clone not allowed when full bundles disabled and can't serve secrets
> +
> +  $ cd server
> +  $ hg --config server.disablefullbundle=true serve -p $HGPORT -d
> --pid-file=hg.pid
> +  $ cat hg.pid > $DAEMON_PIDS
> +  $ cd ..
> +
> +  $ hg clone --uncompressed http://localhost:$HGPORT secret-full-disabled
> +  warning: stream clone requested but server has them disabled
> +  requesting all changes
> +  remote: abort: server has pull-based clones disabled
> +  abort: pull failed on remote
> +  (remove --pull if specified or upgrade Mercurial)
> +  [255]
> +
> +Local stream clone with secrets involved
> +(This is just a test over behavior: if you have access to the repo's
> files,
> +there is no security so it isn't important to prevent a clone here.)
> +
> +  $ hg clone -U --uncompressed server local-secret
> +  warning: stream clone requested but server has them disabled
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
>
>  Stream clone while repo is changing:
>
> @@ -75,7 +146,7 @@ prepare repo with small and big file to
>    $ $TESTDIR/seq.py 50000 > repo/f2
>    $ hg -R repo ci -Aqm "0"
>    $ hg -R repo serve -p $HGPORT1 -d --pid-file=hg.pid --config
> extensions.delayer=delayer.py
> -  $ cat hg.pid >> $DAEMON_PIDS
> +  $ cat hg.pid > $DAEMON_PIDS
>

I changed this but forgot a killdaemons.py after the last test. Could you
please revert this to >> in flight?

>
>  clone while modifying the repo between stating file with write lock and
>  actually serving file content
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170609/56aed7d0/attachment.html>