Patchwork [4,of,5] clonebundles: filter on SNI requirement

login
register
mail settings
Submitter Gregory Szorc
Date Sept. 30, 2015, 12:48 a.m.
Message ID <f59a55d55baa1f96898c.1443574127@gps-mbp.local>
Download mbox | patch
Permalink /patch/10696/
State Superseded
Commit 2faa7671a4b3686a433b1f490e5b37222a99cc0d
Delegated to: Augie Fackler
Headers show

Comments

Gregory Szorc - Sept. 30, 2015, 12:48 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1443573354 25200
#      Tue Sep 29 17:35:54 2015 -0700
# Node ID f59a55d55baa1f96898cc7bd6df29e487327e3cb
# Parent  2778d6d5213b71319279b82c251d0b43543e9709
clonebundles: filter on SNI requirement

Server Name Indication (SNI) is commonly used in CDNs and other hosted
environments. Unfortunately, Python <2.7.9 does not support SNI and when
these older Python versions attempt to negotiate TLS to an SNI server,
they raise an opaque error like
"_ssl.c:507: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert
handshake failure."
Augie Fackler - Sept. 30, 2015, 3:37 p.m.
On Tue, Sep 29, 2015 at 05:48:47PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1443573354 25200
> #      Tue Sep 29 17:35:54 2015 -0700
> # Node ID f59a55d55baa1f96898cc7bd6df29e487327e3cb
> # Parent  2778d6d5213b71319279b82c251d0b43543e9709
> clonebundles: filter on SNI requirement
>
> Server Name Indication (SNI) is commonly used in CDNs and other hosted
> environments. Unfortunately, Python <2.7.9 does not support SNI and when
> these older Python versions attempt to negotiate TLS to an SNI server,
> they raise an opaque error like
> "_ssl.c:507: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert
> handshake failure."
>
> diff --git a/hgext/clonebundles.py b/hgext/clonebundles.py
> --- a/hgext/clonebundles.py
> +++ b/hgext/clonebundles.py
> @@ -42,8 +42,21 @@ TYPE
>
>     The actual value doesn't impact client behavior beyond filtering:
>     clients will still sniff the bundle type from downloaded content.
>     We reuse the file header values for consistency.
> +
> +REQUIRESNI
> +   Whether Server Name Indication (SNI) is required to connect to the URL.
> +   SNI allows servers to use multiple certificates on the same IP. It is
> +   somewhat common in CDNs and other hosting providers. Older Python
> +   versions do not support SNI. Defining this attribute enables clients
> +   with older Python versions to filter this entry.
> +
> +   If this is defined, it is important to advertise a non-SNI fallback
> +   URL or clients running old Python releases may not be able to clone
> +   with the clonebundles facility.

Wouldn't the client just fall back to pulling things normally in that
case? I mean, clearly suboptimal, but still workable unless I missed
something.

> +
> +   Value should be "true".
>  """
>
>  from mercurial import (
>      extensions,
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -11,8 +11,9 @@ from node import hex, nullid
>  import errno, urllib, urllib2
>  import util, scmutil, changegroup, base85, error, store
>  import discovery, phases, obsolete, bookmarks as bookmod, bundle2, pushkey
>  import lock as lockmod
> +import sslutil
>  import tags
>  import url as urlmod
>
>  def readbundle(ui, fh, fname, vfs=None):
> @@ -1642,8 +1643,12 @@ def filterclonebundleentries(ui, entries
>              ui.debug('filtering %s because unknown bundle type %s\n' %
>                         (e['URL'], e['TYPE']))
>              continue
>
> +        if 'REQUIRESNI' in e and not sslutil.hassni:
> +            ui.debug('filtering %s because SNI not supported\n' % e['URL'])
> +            continue
> +
>          newentries.append(e)
>
>      return newentries
>
> diff --git a/tests/test-clonebundles.t b/tests/test-clonebundles.t
> --- a/tests/test-clonebundles.t
> +++ b/tests/test-clonebundles.t
> @@ -145,4 +145,38 @@ Automatic fallback when all entries are
>    adding changesets
>    adding manifests
>    adding file changes
>    added 2 changesets with 2 changes to 2 files
> +
> +URLs requiring SNI are filtered in Python <2.7.9
> +
> +  $ cp full.hg sni.hg
> +  $ cat > server/.hg/clonebundles.manifest << EOF
> +  > http://localhost:$HGPORT1/sni.hg REQUIRESNI=true
> +  > http://localhost:$HGPORT1/full.hg
> +  > EOF
> +
> +#if sslcontext
> +Python 2.7.9+ support SNI
> +
> +  $ hg clone -U http://localhost:$HGPORT sni-supported
> +  applying clone bundle from http://localhost:$HGPORT1/sni.hg
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 2 changesets with 2 changes to 2 files
> +  finished applying clone bundle
> +  searching for changes
> +  no changes found
> +#else
> +Python <2.7.9 will filter SNI URLs
> +
> +  $ hg clone -U http://localhost:$HGPORT sni-unsupported
> +  applying clone bundle from http://localhost:$HGPORT1/full.hg
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 2 changesets with 2 changes to 2 files
> +  finished applying clone bundle
> +  searching for changes
> +  no changes found
> +#endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Gregory Szorc - Sept. 30, 2015, 3:42 p.m.
On Wed, Sep 30, 2015 at 8:37 AM, Augie Fackler <raf@durin42.com> wrote:

> On Tue, Sep 29, 2015 at 05:48:47PM -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1443573354 25200
> > #      Tue Sep 29 17:35:54 2015 -0700
> > # Node ID f59a55d55baa1f96898cc7bd6df29e487327e3cb
> > # Parent  2778d6d5213b71319279b82c251d0b43543e9709
> > clonebundles: filter on SNI requirement
> >
> > Server Name Indication (SNI) is commonly used in CDNs and other hosted
> > environments. Unfortunately, Python <2.7.9 does not support SNI and when
> > these older Python versions attempt to negotiate TLS to an SNI server,
> > they raise an opaque error like
> > "_ssl.c:507: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert
> > handshake failure."
> >
> > diff --git a/hgext/clonebundles.py b/hgext/clonebundles.py
> > --- a/hgext/clonebundles.py
> > +++ b/hgext/clonebundles.py
> > @@ -42,8 +42,21 @@ TYPE
> >
> >     The actual value doesn't impact client behavior beyond filtering:
> >     clients will still sniff the bundle type from downloaded content.
> >     We reuse the file header values for consistency.
> > +
> > +REQUIRESNI
> > +   Whether Server Name Indication (SNI) is required to connect to the
> URL.
> > +   SNI allows servers to use multiple certificates on the same IP. It is
> > +   somewhat common in CDNs and other hosting providers. Older Python
> > +   versions do not support SNI. Defining this attribute enables clients
> > +   with older Python versions to filter this entry.
> > +
> > +   If this is defined, it is important to advertise a non-SNI fallback
> > +   URL or clients running old Python releases may not be able to clone
> > +   with the clonebundles facility.
>
> Wouldn't the client just fall back to pulling things normally in that
> case? I mean, clearly suboptimal, but still workable unless I missed
> something.
>

Yes, the current behavior is to fall back to regular clone if all entries
were filtered and incompatible.

I agree the wording here could be tweaked. I'm sure this series will need a
V2. I'll look at it again then.


>
> > +
> > +   Value should be "true".
> >  """
> >
> >  from mercurial import (
> >      extensions,
> > diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> > --- a/mercurial/exchange.py
> > +++ b/mercurial/exchange.py
> > @@ -11,8 +11,9 @@ from node import hex, nullid
> >  import errno, urllib, urllib2
> >  import util, scmutil, changegroup, base85, error, store
> >  import discovery, phases, obsolete, bookmarks as bookmod, bundle2,
> pushkey
> >  import lock as lockmod
> > +import sslutil
> >  import tags
> >  import url as urlmod
> >
> >  def readbundle(ui, fh, fname, vfs=None):
> > @@ -1642,8 +1643,12 @@ def filterclonebundleentries(ui, entries
> >              ui.debug('filtering %s because unknown bundle type %s\n' %
> >                         (e['URL'], e['TYPE']))
> >              continue
> >
> > +        if 'REQUIRESNI' in e and not sslutil.hassni:
> > +            ui.debug('filtering %s because SNI not supported\n' %
> e['URL'])
> > +            continue
> > +
> >          newentries.append(e)
> >
> >      return newentries
> >
> > diff --git a/tests/test-clonebundles.t b/tests/test-clonebundles.t
> > --- a/tests/test-clonebundles.t
> > +++ b/tests/test-clonebundles.t
> > @@ -145,4 +145,38 @@ Automatic fallback when all entries are
> >    adding changesets
> >    adding manifests
> >    adding file changes
> >    added 2 changesets with 2 changes to 2 files
> > +
> > +URLs requiring SNI are filtered in Python <2.7.9
> > +
> > +  $ cp full.hg sni.hg
> > +  $ cat > server/.hg/clonebundles.manifest << EOF
> > +  > http://localhost:$HGPORT1/sni.hg REQUIRESNI=true
> > +  > http://localhost:$HGPORT1/full.hg
> > +  > EOF
> > +
> > +#if sslcontext
> > +Python 2.7.9+ support SNI
> > +
> > +  $ hg clone -U http://localhost:$HGPORT sni-supported
> > +  applying clone bundle from http://localhost:$HGPORT1/sni.hg
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 2 changesets with 2 changes to 2 files
> > +  finished applying clone bundle
> > +  searching for changes
> > +  no changes found
> > +#else
> > +Python <2.7.9 will filter SNI URLs
> > +
> > +  $ hg clone -U http://localhost:$HGPORT sni-unsupported
> > +  applying clone bundle from http://localhost:$HGPORT1/full.hg
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 2 changesets with 2 changes to 2 files
> > +  finished applying clone bundle
> > +  searching for changes
> > +  no changes found
> > +#endif
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > https://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/hgext/clonebundles.py b/hgext/clonebundles.py
--- a/hgext/clonebundles.py
+++ b/hgext/clonebundles.py
@@ -42,8 +42,21 @@  TYPE
 
    The actual value doesn't impact client behavior beyond filtering:
    clients will still sniff the bundle type from downloaded content.
    We reuse the file header values for consistency.
+
+REQUIRESNI
+   Whether Server Name Indication (SNI) is required to connect to the URL.
+   SNI allows servers to use multiple certificates on the same IP. It is
+   somewhat common in CDNs and other hosting providers. Older Python
+   versions do not support SNI. Defining this attribute enables clients
+   with older Python versions to filter this entry.
+
+   If this is defined, it is important to advertise a non-SNI fallback
+   URL or clients running old Python releases may not be able to clone
+   with the clonebundles facility.
+
+   Value should be "true".
 """
 
 from mercurial import (
     extensions,
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -11,8 +11,9 @@  from node import hex, nullid
 import errno, urllib, urllib2
 import util, scmutil, changegroup, base85, error, store
 import discovery, phases, obsolete, bookmarks as bookmod, bundle2, pushkey
 import lock as lockmod
+import sslutil
 import tags
 import url as urlmod
 
 def readbundle(ui, fh, fname, vfs=None):
@@ -1642,8 +1643,12 @@  def filterclonebundleentries(ui, entries
             ui.debug('filtering %s because unknown bundle type %s\n' %
                        (e['URL'], e['TYPE']))
             continue
 
+        if 'REQUIRESNI' in e and not sslutil.hassni:
+            ui.debug('filtering %s because SNI not supported\n' % e['URL'])
+            continue
+
         newentries.append(e)
 
     return newentries
 
diff --git a/tests/test-clonebundles.t b/tests/test-clonebundles.t
--- a/tests/test-clonebundles.t
+++ b/tests/test-clonebundles.t
@@ -145,4 +145,38 @@  Automatic fallback when all entries are 
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files
+
+URLs requiring SNI are filtered in Python <2.7.9
+
+  $ cp full.hg sni.hg
+  $ cat > server/.hg/clonebundles.manifest << EOF
+  > http://localhost:$HGPORT1/sni.hg REQUIRESNI=true
+  > http://localhost:$HGPORT1/full.hg
+  > EOF
+
+#if sslcontext
+Python 2.7.9+ support SNI
+
+  $ hg clone -U http://localhost:$HGPORT sni-supported
+  applying clone bundle from http://localhost:$HGPORT1/sni.hg
+  adding changesets
+  adding manifests
+  adding file changes
+  added 2 changesets with 2 changes to 2 files
+  finished applying clone bundle
+  searching for changes
+  no changes found
+#else
+Python <2.7.9 will filter SNI URLs
+
+  $ hg clone -U http://localhost:$HGPORT sni-unsupported
+  applying clone bundle from http://localhost:$HGPORT1/full.hg
+  adding changesets
+  adding manifests
+  adding file changes
+  added 2 changesets with 2 changes to 2 files
+  finished applying clone bundle
+  searching for changes
+  no changes found
+#endif