Merge pull request #1189 from tahoe-lafs/3875-http-storage-furls

HTTPS storage furls

Fixes ticket:3875

docs/proposed/http-storage-node-protocol.rst

@@ -35,10 +35,10 @@ Glossary
      (the storage service is an example of such an object)
 
    NURL
-     a self-authenticating URL-like string almost exactly like a fURL but without being tied to Foolscap
+     a self-authenticating URL-like string almost exactly like a NURL but without being tied to Foolscap
 
    swissnum
-     a short random string which is part of a fURL and which acts as a shared secret to authorize clients to use a storage service
+     a short random string which is part of a fURL/NURL and which acts as a shared secret to authorize clients to use a storage service
 
    lease
      state associated with a share informing a storage server of the duration of storage desired by a client
@@ -211,15 +211,15 @@ To further clarify, consider this example.
 Alice operates a storage node.
 Alice generates a key pair and secures it properly.
 Alice generates a self-signed storage node certificate with the key pair.
-Alice's storage node announces (to an introducer) a fURL containing (among other information) the SPKI hash.
+Alice's storage node announces (to an introducer) a NURL containing (among other information) the SPKI hash.
 Imagine the SPKI hash is ``i5xb...``.
-This results in a fURL of ``pb://i5xb...@example.com:443/g3m5...#v=1``.
+This results in a NURL of ``pb://i5xb...@example.com:443/g3m5...#v=1``.
 Bob creates a client node pointed at the same introducer.
 Bob's client node receives the announcement from Alice's storage node
 (indirected through the introducer).
 
-Bob's client node recognizes the fURL as referring to an HTTP-dialect server due to the ``v=1`` fragment.
-Bob's client node can now perform a TLS handshake with a server at the address in the fURL location hints
+Bob's client node recognizes the NURL as referring to an HTTP-dialect server due to the ``v=1`` fragment.
+Bob's client node can now perform a TLS handshake with a server at the address in the NURL location hints
 (``example.com:443`` in this example).
 Following the above described validation procedures,
 Bob's client node can determine whether it has reached Alice's storage node or not.
@@ -230,7 +230,7 @@ Additionally,
 by continuing to interact using TLS,
 Bob's client and Alice's storage node are assured of both **message authentication** and **message confidentiality**.
 
-Bob's client further inspects the fURL for the *swissnum*.
+Bob's client further inspects the NURL for the *swissnum*.
 When Bob's client issues HTTP requests to Alice's storage node it includes the *swissnum* in its requests.
 **Storage authorization** has been achieved.
 
@@ -266,8 +266,8 @@ Generation of a new certificate allows for certain non-optimal conditions to be
 * The ``commonName`` of ``newpb_thingy`` may be changed to a more descriptive value.
 * A ``notValidAfter`` field with a timestamp in the past may be updated.
 
-Storage nodes will announce a new fURL for this new HTTP-based server.
-This fURL will be announced alongside their existing Foolscap-based server's fURL.
+Storage nodes will announce a new NURL for this new HTTP-based server.
+This NURL will be announced alongside their existing Foolscap-based server's fURL.
 Such an announcement will resemble this::
 
   {
@@ -312,7 +312,7 @@ The follow sequence of events is likely:
 #. The client uses the information in its cache to open a Foolscap connection to the storage server.
 
 Ideally,
-the client would not rely on an update from the introducer to give it the GBS fURL for the updated storage server.
+the client would not rely on an update from the introducer to give it the GBS NURL for the updated storage server.
 Therefore,
 when an updated client connects to a storage server using Foolscap,
 it should request the server's version information.

src/allmydata/storage/http_client.py

@@ -2,8 +2,9 @@
 HTTP client that talks to the HTTP storage server.
 """
 
+from __future__ import annotations
+
 from typing import Union, Set, Optional
-from treq.testing import StubTreq
 
 from base64 import b64encode
 
@@ -16,12 +17,30 @@ from collections_extended import RangeMap
 from werkzeug.datastructures import Range, ContentRange
 from twisted.web.http_headers import Headers
 from twisted.web import http
+from twisted.web.iweb import IPolicyForHTTPS
 from twisted.internet.defer import inlineCallbacks, returnValue, fail, Deferred
+from twisted.internet.interfaces import IOpenSSLClientConnectionCreator
+from twisted.internet.ssl import CertificateOptions
+from twisted.web.client import Agent, HTTPConnectionPool
+from zope.interface import implementer
 from hyperlink import DecodedURL
 import treq
+from treq.client import HTTPClient
+from treq.testing import StubTreq
+from OpenSSL import SSL
+from cryptography.hazmat.bindings.openssl.binding import Binding
 
-from .http_common import swissnum_auth_header, Secrets, get_content_type, CBOR_MIME_TYPE
+from .http_common import (
+    swissnum_auth_header,
+    Secrets,
+    get_content_type,
+    CBOR_MIME_TYPE,
+    get_spki_hash,
+)
 from .common import si_b2a
+from ..util.hashutil import timing_safe_compare
+
+_OPENSSL = Binding().lib
 
 
 def _encode_si(si):  # type: (bytes) -> str
@@ -110,6 +129,97 @@ class ImmutableCreateResult(object):
     allocated = attr.ib(type=Set[int])
 
 
+class _TLSContextFactory(CertificateOptions):
+    """
+    Create a context that validates the way Tahoe-LAFS wants to: based on a
+    pinned certificate hash, rather than a certificate authority.
+
+    Originally implemented as part of Foolscap.  To comply with the license,
+    here's the original licensing terms:
+
+    Copyright (c) 2006-2008 Brian Warner
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included in
+    all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+    """
+
+    def __init__(self, expected_spki_hash: bytes):
+        self.expected_spki_hash = expected_spki_hash
+        CertificateOptions.__init__(self)
+
+    def getContext(self) -> SSL.Context:
+        def always_validate(conn, cert, errno, depth, preverify_ok):
+            # This function is called to validate the certificate received by
+            # the other end. OpenSSL calls it multiple times, for each errno
+            # for each certificate.
+
+            # We do not care about certificate authorities or revocation
+            # lists, we just want to know that the certificate has a valid
+            # signature and follow the chain back to one which is
+            # self-signed. We need to protect against forged signatures, but
+            # not the usual TLS concerns about invalid CAs or revoked
+            # certificates.
+            things_are_ok = (
+                _OPENSSL.X509_V_OK,
+                _OPENSSL.X509_V_ERR_CERT_NOT_YET_VALID,
+                _OPENSSL.X509_V_ERR_CERT_HAS_EXPIRED,
+                _OPENSSL.X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT,
+                _OPENSSL.X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN,
+            )
+            # TODO can we do this once instead of multiple times?
+            if errno in things_are_ok and timing_safe_compare(
+                get_spki_hash(cert.to_cryptography()), self.expected_spki_hash
+            ):
+                return 1
+            # TODO: log the details of the error, because otherwise they get
+            # lost in the PyOpenSSL exception that will eventually be raised
+            # (possibly OpenSSL.SSL.Error: certificate verify failed)
+            return 0
+
+        ctx = CertificateOptions.getContext(self)
+
+        # VERIFY_PEER means we ask the the other end for their certificate.
+        ctx.set_verify(SSL.VERIFY_PEER, always_validate)
+        return ctx
+
+
+@implementer(IPolicyForHTTPS)
+@implementer(IOpenSSLClientConnectionCreator)
+@attr.s
+class _StorageClientHTTPSPolicy:
+    """
+    A HTTPS policy that ensures the SPKI hash of the public key matches a known
+    hash, i.e. pinning-based validation.
+    """
+
+    expected_spki_hash = attr.ib(type=bytes)
+
+    # IPolicyForHTTPS
+    def creatorForNetloc(self, hostname, port):
+        return self
+
+    # IOpenSSLClientConnectionCreator
+    def clientConnectionForTLS(self, tlsProtocol):
+        return SSL.Connection(
+            _TLSContextFactory(self.expected_spki_hash).getContext(), None
+        )
+
+
 class StorageClient(object):
     """
     Low-level HTTP client that talks to the HTTP storage server.
@@ -117,11 +227,38 @@ class StorageClient(object):
 
     def __init__(
         self, url, swissnum, treq=treq
-    ):  # type: (DecodedURL, bytes, Union[treq,StubTreq]) -> None
+    ):  # type: (DecodedURL, bytes, Union[treq,StubTreq,HTTPClient]) -> None
+        """
+        The URL is a HTTPS URL ("https://...").  To construct from a NURL, use
+        ``StorageClient.from_nurl()``.
+        """
         self._base_url = url
         self._swissnum = swissnum
         self._treq = treq
 
+    @classmethod
+    def from_nurl(cls, nurl: DecodedURL, reactor, persistent: bool = True) -> StorageClient:
+        """
+        Create a ``StorageClient`` for the given NURL.
+
+        ``persistent`` indicates whether to use persistent HTTP connections.
+        """
+        assert nurl.fragment == "v=1"
+        assert nurl.scheme == "pb"
+        swissnum = nurl.path[0].encode("ascii")
+        certificate_hash = nurl.user.encode("ascii")
+
+        treq_client = HTTPClient(
+            Agent(
+                reactor,
+                _StorageClientHTTPSPolicy(expected_spki_hash=certificate_hash),
+                pool=HTTPConnectionPool(reactor, persistent=persistent),
+            )
+        )
+
+        https_url = DecodedURL().replace(scheme="https", host=nurl.host, port=nurl.port)
+        return cls(https_url, swissnum, treq_client)
+
     def relative_url(self, path):
         """Get a URL relative to the base URL."""
         return self._base_url.click(path)

src/allmydata/storage/http_common.py

@@ -3,9 +3,13 @@ Common HTTP infrastructure for the storge server.
 """
 
 from enum import Enum
-from base64 import b64encode
+from base64 import urlsafe_b64encode, b64encode
+from hashlib import sha256
 from typing import Optional
 
+from cryptography.x509 import Certificate
+from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat
+
 from werkzeug.http import parse_options_header
 from twisted.web.http_headers import Headers
 
@@ -23,7 +27,7 @@ def get_content_type(headers: Headers) -> Optional[str]:
     return content_type
 
 
-def swissnum_auth_header(swissnum):  # type: (bytes) -> bytes
+def swissnum_auth_header(swissnum: bytes) -> bytes:
     """Return value for ``Authentication`` header."""
     return b"Tahoe-LAFS " + b64encode(swissnum).strip()
 
@@ -34,3 +38,16 @@ class Secrets(Enum):
     LEASE_RENEW = "lease-renew-secret"
     LEASE_CANCEL = "lease-cancel-secret"
     UPLOAD = "upload-secret"
+
+
+def get_spki_hash(certificate: Certificate) -> bytes:
+    """
+    Get the public key hash, as per RFC 7469: base64 of sha256 of the public
+    key encoded in DER + Subject Public Key Info format.
+
+    We use the URL-safe base64 variant, since this is typically found in NURLs.
+    """
+    public_key_bytes = certificate.public_key().public_bytes(
+        Encoding.DER, PublicFormat.SubjectPublicKeyInfo
+    )
+    return urlsafe_b64encode(sha256(public_key_bytes).digest()).strip().rstrip(b"=")

src/allmydata/storage/http_server.py

@@ -8,8 +8,16 @@ from functools import wraps
 from base64 import b64decode
 import binascii
 
+from zope.interface import implementer
 from klein import Klein
 from twisted.web import http
+from twisted.internet.interfaces import IListeningPort, IStreamServerEndpoint
+from twisted.internet.defer import Deferred
+from twisted.internet.ssl import CertificateOptions, Certificate, PrivateCertificate
+from twisted.web.server import Site
+from twisted.protocols.tls import TLSMemoryBIOFactory
+from twisted.python.filepath import FilePath
+
 import attr
 from werkzeug.http import (
     parse_range_header,
@@ -18,12 +26,22 @@ from werkzeug.http import (
 )
 from werkzeug.routing import BaseConverter, ValidationError
 from werkzeug.datastructures import ContentRange
+from hyperlink import DecodedURL
+from cryptography.x509 import load_pem_x509_certificate
+
 
 # TODO Make sure to use pure Python versions?
 from cbor2 import dumps, loads
 from pycddl import Schema, ValidationError as CDDLValidationError
 from .server import StorageServer
-from .http_common import swissnum_auth_header, Secrets, get_content_type, CBOR_MIME_TYPE
+from .http_common import (
+    swissnum_auth_header,
+    Secrets,
+    get_content_type,
+    CBOR_MIME_TYPE,
+    get_spki_hash,
+)
+
 from .common import si_a2b
 from .immutable import BucketWriter, ConflictingWriteError
 from ..util.hashutil import timing_safe_compare
@@ -529,3 +547,78 @@ class HTTPServer(object):
         info = self._read_encoded(request, _SCHEMAS["advise_corrupt_share"])
         bucket.advise_corrupt_share(info["reason"].encode("utf-8"))
         return b""
+
+
+@implementer(IStreamServerEndpoint)
+@attr.s
+class _TLSEndpointWrapper(object):
+    """
+    Wrap an existing endpoint with the server-side storage TLS policy.  This is
+    useful because not all Tahoe-LAFS endpoints might be plain TCP+TLS, for
+    example there's Tor and i2p.
+    """
+
+    endpoint = attr.ib(type=IStreamServerEndpoint)
+    context_factory = attr.ib(type=CertificateOptions)
+
+    @classmethod
+    def from_paths(
+        cls, endpoint, private_key_path: FilePath, cert_path: FilePath
+    ) -> "_TLSEndpointWrapper":
+        """
+        Create an endpoint with the given private key and certificate paths on
+        the filesystem.
+        """
+        certificate = Certificate.loadPEM(cert_path.getContent()).original
+        private_key = PrivateCertificate.loadPEM(
+            cert_path.getContent() + b"\n" + private_key_path.getContent()
+        ).privateKey.original
+        certificate_options = CertificateOptions(
+            privateKey=private_key, certificate=certificate
+        )
+        return cls(endpoint=endpoint, context_factory=certificate_options)
+
+    def listen(self, factory):
+        return self.endpoint.listen(
+            TLSMemoryBIOFactory(self.context_factory, False, factory)
+        )
+
+
+def listen_tls(
+    server: HTTPServer,
+    hostname: str,
+    endpoint: IStreamServerEndpoint,
+    private_key_path: FilePath,
+    cert_path: FilePath,
+) -> Deferred[Tuple[DecodedURL, IListeningPort]]:
+    """
+    Start a HTTPS storage server on the given port, return the NURL and the
+    listening port.
+
+    The hostname is the external IP or hostname clients will connect to, used
+    to constrtuct the NURL; it does not modify what interfaces the server
+    listens on.
+
+    This will likely need to be updated eventually to handle Tor/i2p.
+    """
+    endpoint = _TLSEndpointWrapper.from_paths(endpoint, private_key_path, cert_path)
+
+    def build_nurl(listening_port: IListeningPort) -> DecodedURL:
+        nurl = DecodedURL().replace(
+            fragment="v=1",  # how we know this NURL is HTTP-based (i.e. not Foolscap)
+            host=hostname,
+            port=listening_port.getHost().port,
+            path=(str(server._swissnum, "ascii"),),
+            userinfo=(
+                str(
+                    get_spki_hash(load_pem_x509_certificate(cert_path.getContent())),
+                    "ascii",
+                ),
+            ),
+            scheme="pb",
+        )
+        return nurl
+
+    return endpoint.listen(Site(server.get_resource())).addCallback(
+        lambda listening_port: (build_nurl(listening_port), listening_port)
+    )

src/allmydata/test/certs.py

@@ -0,0 +1,66 @@
+"""Utilities for generating TLS certificates."""
+
+import datetime
+
+from cryptography import x509
+from cryptography.x509.oid import NameOID
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography.hazmat.primitives import serialization, hashes
+
+from twisted.python.filepath import FilePath
+
+
+def cert_to_file(path: FilePath, cert) -> FilePath:
+    """
+    Write the given certificate to a file on disk. Returns the path.
+    """
+    path.setContent(cert.public_bytes(serialization.Encoding.PEM))
+    return path
+
+
+def private_key_to_file(path: FilePath, private_key) -> FilePath:
+    """
+    Write the given key to a file on disk. Returns the path.
+    """
+    path.setContent(
+        private_key.private_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PrivateFormat.TraditionalOpenSSL,
+            encryption_algorithm=serialization.NoEncryption(),
+        )
+    )
+    return path
+
+
+def generate_private_key():
+    """Create a RSA private key."""
+    return rsa.generate_private_key(public_exponent=65537, key_size=2048)
+
+
+def generate_certificate(
+    private_key,
+    expires_days: int = 10,
+    valid_in_days: int = 0,
+    org_name: str = "Yoyodyne",
+):
+    """Generate a certificate from a RSA private key."""
+    subject = issuer = x509.Name(
+        [x509.NameAttribute(NameOID.ORGANIZATION_NAME, org_name)]
+    )
+    starts = datetime.datetime.utcnow() + datetime.timedelta(days=valid_in_days)
+    expires = datetime.datetime.utcnow() + datetime.timedelta(days=expires_days)
+    return (
+        x509.CertificateBuilder()
+        .subject_name(subject)
+        .issuer_name(issuer)
+        .public_key(private_key.public_key())
+        .serial_number(x509.random_serial_number())
+        .not_valid_before(min(starts, expires))
+        .not_valid_after(expires)
+        .add_extension(
+            x509.SubjectAlternativeName([x509.DNSName("localhost")]),
+            critical=False,
+            # Sign our certificate with our private key
+        )
+        .sign(private_key, hashes.SHA256())
+    )

src/allmydata/test/test_istorageserver.py

@@ -8,19 +8,9 @@ reused across tests, so each test should be careful to generate unique storage
 indexes.
 """
 
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
+from future.utils import bchr
 
-from future.utils import PY2, bchr
-
-if PY2:
-    # fmt: off
-    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
-    # fmt: on
-else:
-    from typing import Set
+from typing import Set
 
 from random import Random
 from unittest import SkipTest
@@ -29,18 +19,20 @@ from twisted.internet.defer import inlineCallbacks, returnValue, succeed
 from twisted.internet.task import Clock
 from twisted.internet import reactor
 from twisted.internet.endpoints import serverFromString
-from twisted.web.server import Site
-from twisted.web.client import Agent, HTTPConnectionPool
-from hyperlink import DecodedURL
-from treq.client import HTTPClient
-
+from twisted.python.filepath import FilePath
 from foolscap.api import Referenceable, RemoteException
 
 from allmydata.interfaces import IStorageServer  # really, IStorageClient
 from .common_system import SystemTestMixin
 from .common import AsyncTestCase, SameProcessStreamEndpointAssigner
+from .certs import (
+    generate_certificate,
+    generate_private_key,
+    private_key_to_file,
+    cert_to_file,
+)
 from allmydata.storage.server import StorageServer  # not a IStorageServer!!
-from allmydata.storage.http_server import HTTPServer
+from allmydata.storage.http_server import HTTPServer, listen_tls
 from allmydata.storage.http_client import StorageClient
 from allmydata.storage_client import _HTTPStorageServer
 
@@ -1017,10 +1009,6 @@ class _SharedMixin(SystemTestMixin):
 
         AsyncTestCase.setUp(self)
 
-        self._port_assigner = SameProcessStreamEndpointAssigner()
-        self._port_assigner.setUp()
-        self.addCleanup(self._port_assigner.tearDown)
-
         self.basedir = "test_istorageserver/" + self.id()
         yield SystemTestMixin.setUp(self)
         yield self.set_up_nodes(1)
@@ -1065,8 +1053,9 @@ class _HTTPMixin(_SharedMixin):
     """Run tests on the HTTP version of ``IStorageServer``."""
 
     def setUp(self):
-        if PY2:
-            self.skipTest("Not going to bother supporting Python 2")
+        self._port_assigner = SameProcessStreamEndpointAssigner()
+        self._port_assigner.setUp()
+        self.addCleanup(self._port_assigner.tearDown)
         return _SharedMixin.setUp(self)
 
     @inlineCallbacks
@@ -1074,29 +1063,27 @@ class _HTTPMixin(_SharedMixin):
         swissnum = b"1234"
         http_storage_server = HTTPServer(self.server, swissnum)
 
-        # Listen on randomly assigned port:
-        tcp_address, endpoint_string = self._port_assigner.assign(reactor)
-        _, host, port = tcp_address.split(":")
-        port = int(port)
-        endpoint = serverFromString(reactor, endpoint_string)
-        listening_port = yield endpoint.listen(Site(http_storage_server.get_resource()))
+        # Listen on randomly assigned port, using self-signed cert:
+        private_key = generate_private_key()
+        certificate = generate_certificate(private_key)
+        _, endpoint_string = self._port_assigner.assign(reactor)
+        nurl, listening_port = yield listen_tls(
+            http_storage_server,
+            "127.0.0.1",
+            serverFromString(reactor, endpoint_string),
+            private_key_to_file(FilePath(self.mktemp()), private_key),
+            cert_to_file(FilePath(self.mktemp()), certificate),
+        )
         self.addCleanup(listening_port.stopListening)
 
         # Create HTTP client with non-persistent connections, so we don't leak
         # state across tests:
-        treq_client = HTTPClient(
-            Agent(reactor, HTTPConnectionPool(reactor, persistent=False))
-        )
-
         returnValue(
             _HTTPStorageServer.from_http_client(
-                StorageClient(
-                    DecodedURL().replace(scheme="http", host=host, port=port),
-                    swissnum,
-                    treq=treq_client,
-                )
+                StorageClient.from_nurl(nurl, reactor, persistent=False)
             )
         )
+
         # Eventually should also:
         #  self.assertTrue(IStorageServer.providedBy(client))
 

src/allmydata/test/test_storage_http.py

@@ -2,18 +2,6 @@
 Tests for HTTP storage client + server.
 """
 
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
-
-from future.utils import PY2
-
-if PY2:
-    # fmt: off
-    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
-    # fmt: on
-
 from base64 import b64encode
 from contextlib import contextmanager
 from os import urandom
@@ -33,6 +21,8 @@ from werkzeug import routing
 from werkzeug.exceptions import NotFound as WNotFound
 
 from .common import SyncTestCase
+from ..storage.http_common import get_content_type, CBOR_MIME_TYPE
+from ..storage.common import si_b2a
 from ..storage.server import StorageServer
 from ..storage.http_server import (
     HTTPServer,
@@ -51,8 +41,6 @@ from ..storage.http_client import (
     StorageClientGeneral,
     _encode_si,
 )
-from ..storage.http_common import get_content_type, CBOR_MIME_TYPE
-from ..storage.common import si_b2a
 
 
 class HTTPUtilities(SyncTestCase):
@@ -108,11 +96,6 @@ class ExtractSecretsTests(SyncTestCase):
     Tests for ``_extract_secrets``.
     """
 
-    def setUp(self):
-        if PY2:
-            self.skipTest("Not going to bother supporting Python 2")
-        super(ExtractSecretsTests, self).setUp()
-
     @given(secrets_to_send=SECRETS_STRATEGY)
     def test_extract_secrets(self, secrets_to_send):
         """
@@ -271,8 +254,6 @@ class CustomHTTPServerTests(SyncTestCase):
     """
 
     def setUp(self):
-        if PY2:
-            self.skipTest("Not going to bother supporting Python 2")
         super(CustomHTTPServerTests, self).setUp()
         # Could be a fixture, but will only be used in this test class so not
         # going to bother:
@@ -374,8 +355,6 @@ class GenericHTTPAPITests(SyncTestCase):
     """
 
     def setUp(self):
-        if PY2:
-            self.skipTest("Not going to bother supporting Python 2")
         super(GenericHTTPAPITests, self).setUp()
         self.http = self.useFixture(HttpTestFixture())
 
@@ -466,8 +445,6 @@ class ImmutableHTTPAPITests(SyncTestCase):
     """
 
     def setUp(self):
-        if PY2:
-            self.skipTest("Not going to bother supporting Python 2")
         super(ImmutableHTTPAPITests, self).setUp()
         self.http = self.useFixture(HttpTestFixture())
         self.imm_client = StorageClientImmutables(self.http.client)

src/allmydata/test/test_storage_https.py

@@ -0,0 +1,219 @@
+"""
+Tests for the TLS part of the HTTP Storage Protocol.
+
+More broadly, these are tests for HTTPS usage as replacement for Foolscap's
+server authentication logic, which may one day apply outside of HTTP Storage
+Protocol.
+"""
+
+from functools import wraps
+from contextlib import asynccontextmanager
+
+from cryptography import x509
+
+from twisted.internet.endpoints import serverFromString
+from twisted.internet import reactor
+from twisted.internet.defer import Deferred
+from twisted.internet.task import deferLater
+from twisted.web.server import Site
+from twisted.web.static import Data
+from twisted.web.client import Agent, HTTPConnectionPool, ResponseNeverReceived
+from twisted.python.filepath import FilePath
+from treq.client import HTTPClient
+
+from .common import SyncTestCase, AsyncTestCase, SameProcessStreamEndpointAssigner
+from .certs import (
+    generate_certificate,
+    generate_private_key,
+    private_key_to_file,
+    cert_to_file,
+)
+from ..storage.http_common import get_spki_hash
+from ..storage.http_client import _StorageClientHTTPSPolicy
+from ..storage.http_server import _TLSEndpointWrapper
+
+
+class HTTPSNurlTests(SyncTestCase):
+    """Tests for HTTPS NURLs."""
+
+    def test_spki_hash(self):
+        """The output of ``get_spki_hash()`` matches the semantics of RFC 7469.
+
+        The expected hash was generated using Appendix A instructions in the
+        RFC::
+
+            openssl x509 -noout -in certificate.pem -pubkey | \
+                openssl asn1parse -noout -inform pem -out public.key
+            openssl dgst -sha256 -binary public.key | openssl enc -base64
+        """
+        expected_hash = b"JIj6ezHkdSBlHhrnezAgIC_mrVQHy4KAFyL-8ZNPGPM"
+        certificate_text = b"""\
+-----BEGIN CERTIFICATE-----
+MIIDWTCCAkECFCf+I+3oEhTfqt+6ruH4qQ4Wst1DMA0GCSqGSIb3DQEBCwUAMGkx
+CzAJBgNVBAYTAlpaMRAwDgYDVQQIDAdOb3doZXJlMRQwEgYDVQQHDAtFeGFtcGxl
+dG93bjEcMBoGA1UECgwTRGVmYXVsdCBDb21wYW55IEx0ZDEUMBIGA1UEAwwLZXhh
+bXBsZS5jb20wHhcNMjIwMzAyMTUyNTQ3WhcNMjMwMzAyMTUyNTQ3WjBpMQswCQYD
+VQQGEwJaWjEQMA4GA1UECAwHTm93aGVyZTEUMBIGA1UEBwwLRXhhbXBsZXRvd24x
+HDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxFDASBgNVBAMMC2V4YW1wbGUu
+Y29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv9vqtA8Toy9D6xLG
+q41iUafSiAXnuirWxML2ct/LAcGJzATg6JctmJxxZQL7vkmaFFPBF6Y39bOGbbEC
+M2iQYn2Qemj5fl3IzKTnYLqzryGM0ZwwnNbPyetSe/sksAIYRLzn49d6l+AHR+Dj
+GyvoLzIyGUTn41MTDafMNtPgWx1i+65lFW3GHYpEmugu4bjeUPizNja2LrqwvwFu
+YXwmKxbIMdioCoRvDGX9SI3/euFstuR4rbOEUDxniYRF5g6reP8UMF30zJzF5j0k
+yDg8Z5b1XpKFNZAeyRYxcs9wJCqVlP6BLPDnvNVpMXodnWLeTK+r6YWvGadGVufk
+YNC1PwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQByrhn78GSS3dJ0pJ6czmhMX5wH
++fauCtt1+Wbn+ctTodTycS+pfULO4gG7wRzhl8KNoOqLmWMjyA2A3mon8kdkD+0C
+i8McpoPaGS2wQcqC28Ud6kP9YO81YFyTl4nHVKQ0nmplT+eoLDTCIWMVxHHzxIgs
+2ybUluAc+THSjpGxB6kWSAJeg3N+f2OKr+07Yg9LiQ2b8y0eZarpiuuuXCzWeWrQ
+PudP0aniyq/gbPhxq0tYF628IBvhDAnr/2kqEmVF2TDr2Sm/Y3PDBuPY6MeIxjnr
+ox5zO3LrQmQw11OaIAs2/kviKAoKTFFxeyYcpS5RuKNDZfHQCXlLwt9bySxG
+-----END CERTIFICATE-----
+"""
+        certificate = x509.load_pem_x509_certificate(certificate_text)
+        self.assertEqual(get_spki_hash(certificate), expected_hash)
+
+
+def async_to_deferred(f):
+    """
+    Wrap an async function to return a Deferred instead.
+
+    Maybe solution to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3886
+    """
+
+    @wraps(f)
+    def not_async(*args, **kwargs):
+        return Deferred.fromCoroutine(f(*args, **kwargs))
+
+    return not_async
+
+
+class PinningHTTPSValidation(AsyncTestCase):
+    """
+    Test client-side validation logic of HTTPS certificates that uses
+    Tahoe-LAFS's pinning-based scheme instead of the traditional certificate
+    authority scheme.
+
+    https://cryptography.io/en/latest/x509/tutorial/#creating-a-self-signed-certificate
+    """
+
+    def setUp(self):
+        self._port_assigner = SameProcessStreamEndpointAssigner()
+        self._port_assigner.setUp()
+        self.addCleanup(self._port_assigner.tearDown)
+        return AsyncTestCase.setUp(self)
+
+    @asynccontextmanager
+    async def listen(self, private_key_path: FilePath, cert_path: FilePath):
+        """
+        Context manager that runs a HTTPS server with the given private key
+        and certificate.
+
+        Returns a URL that will connect to the server.
+        """
+        location_hint, endpoint_string = self._port_assigner.assign(reactor)
+        underlying_endpoint = serverFromString(reactor, endpoint_string)
+        endpoint = _TLSEndpointWrapper.from_paths(
+            underlying_endpoint, private_key_path, cert_path
+        )
+        root = Data(b"YOYODYNE", "text/plain")
+        root.isLeaf = True
+        listening_port = await endpoint.listen(Site(root))
+        try:
+            yield f"https://127.0.0.1:{listening_port.getHost().port}/"
+        finally:
+            await listening_port.stopListening()
+            # Make sure all server connections are closed :( No idea why this
+            # is necessary when it's not for IStorageServer HTTPS tests.
+            await deferLater(reactor, 0.01)
+
+    def request(self, url: str, expected_certificate: x509.Certificate):
+        """
+        Send a HTTPS request to the given URL, ensuring that the given
+        certificate is the one used via SPKI-hash-based pinning comparison.
+        """
+        # No persistent connections, so we don't have dirty reactor at the end
+        # of the test.
+        treq_client = HTTPClient(
+            Agent(
+                reactor,
+                _StorageClientHTTPSPolicy(
+                    expected_spki_hash=get_spki_hash(expected_certificate)
+                ),
+                pool=HTTPConnectionPool(reactor, persistent=False),
+            )
+        )
+        return treq_client.get(url)
+
+    @async_to_deferred
+    async def test_success(self):
+        """
+        If all conditions are met, a TLS client using the Tahoe-LAFS policy can
+        connect to the server.
+        """
+        private_key = generate_private_key()
+        certificate = generate_certificate(private_key)
+        async with self.listen(
+            private_key_to_file(FilePath(self.mktemp()), private_key),
+            cert_to_file(FilePath(self.mktemp()), certificate),
+        ) as url:
+            response = await self.request(url, certificate)
+            self.assertEqual(await response.content(), b"YOYODYNE")
+
+    @async_to_deferred
+    async def test_server_certificate_has_wrong_hash(self):
+        """
+        If the server's certificate hash doesn't match the hash the client
+        expects, the request to the server fails.
+        """
+        private_key1 = generate_private_key()
+        certificate1 = generate_certificate(private_key1)
+        private_key2 = generate_private_key()
+        certificate2 = generate_certificate(private_key2)
+
+        async with self.listen(
+            private_key_to_file(FilePath(self.mktemp()), private_key1),
+            cert_to_file(FilePath(self.mktemp()), certificate1),
+        ) as url:
+            with self.assertRaises(ResponseNeverReceived):
+                await self.request(url, certificate2)
+
+    @async_to_deferred
+    async def test_server_certificate_expired(self):
+        """
+        If the server's certificate has expired, the request to the server
+        succeeds if the hash matches the one the client expects; expiration has
+        no effect.
+        """
+        private_key = generate_private_key()
+        certificate = generate_certificate(private_key, expires_days=-10)
+
+        async with self.listen(
+            private_key_to_file(FilePath(self.mktemp()), private_key),
+            cert_to_file(FilePath(self.mktemp()), certificate),
+        ) as url:
+            response = await self.request(url, certificate)
+            self.assertEqual(await response.content(), b"YOYODYNE")
+
+    @async_to_deferred
+    async def test_server_certificate_not_valid_yet(self):
+        """
+        If the server's certificate is only valid starting in The Future, the
+        request to the server succeeds if the hash matches the one the client
+        expects; start time has no effect.
+        """
+        private_key = generate_private_key()
+        certificate = generate_certificate(
+            private_key, expires_days=10, valid_in_days=5
+        )
+
+        async with self.listen(
+            private_key_to_file(FilePath(self.mktemp()), private_key),
+            cert_to_file(FilePath(self.mktemp()), certificate),
+        ) as url:
+            response = await self.request(url, certificate)
+            self.assertEqual(await response.content(), b"YOYODYNE")
+
+    # A potential attack to test is a private key that doesn't match the
+    # certificate... but OpenSSL (quite rightly) won't let you listen with that
+    # so I don't know how to test that! See
+    # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3884

tox.ini

@@ -141,6 +141,7 @@ deps =
     types-six
     types-PyYAML
     types-pkg_resources
+    types-pyOpenSSL
     git+https://github.com/warner/foolscap
     # Twisted 21.2.0 introduces some type hints which we are not yet
     # compatible with.