Support for compressed_certificate extension

[GeorgePantelakis]

Added support in client and server for the TLS Certificate Compression as described in https://www.rfc-editor.org/rfc/rfc8879.html

---

This change is 

[tomato42]

The CI will need to be extended with a combination of the packages that implement the zstd and brotli compression...

Reviewed 10 of 23 files at r1.
Reviewable status: 8 of 23 files reviewed, 32 unresolved discussions (waiting on @GeorgePantelakis)

---

-- commits line 2 at r1:
"Support for compressed_certificate extension"?

---

scripts/tls.py line 81 at r1 (raw file):

    print("")
    print("Compression:")

"Certificate Compression"?

---

tests/tlstest.py line 2041 at r1 (raw file):

    assert connection.extendedMasterSecret
    assert connection.session.appProto is None
    assert connection.server_cert_compression_algo == "zlib"

since client cert was not used here, we should also assert that the client side is None
and also add it to the client side of this test

similarly, we should add this assert to one of the tests that do use client certificates, to show that it is set for client certificates too

---

tests/tlstest.py line 2082 at r1 (raw file):

    settings.certificate_compression_send = []
    connection = connect()
    connection.handshakeClientCert(serverName=address[0],

this should be HandshakeServer...

---

tlslite/constants.py line 585 at r1 (raw file):

    psk_dhe_ke = 1

class CompressionAlgorithm(TLSEnum):

In the RFC it's a CertificateCompressionAlgorithm...

---

tlslite/constants.py line 586 at r1 (raw file):

class CompressionAlgorithm(TLSEnum):
    """Compression algorithms used for the compression of certificates."""

please add reference to the RFC here

---

tlslite/constants.py line 591 at r1 (raw file):

    brotli = 2
    zstd = 3
    all = [1, 2, 3]

if you define all then you need to also re-declare def toRepr that that ignores it, see ECPointFormat

---

tlslite/extensions.py line 2161 at r1 (raw file):

class CompressCertificateExtension(VarListExtension):

I don't think we need to shorten it, I think it can be the full CertificateCompressionAlgorithmsExtension (from struct name) or CompressedCertificateExtension (from the extension ID name)

---

tlslite/handshakesettings.py line 372 at r1 (raw file):

    :vartype certificate_compression_send: list(str)
    :ivar certificate_compression: a list of compression algorithms that will

should be :ivar certificate_compression_send:

---

tlslite/handshakesettings.py line 378 at r1 (raw file):

    :vartype certificate_compression_receive: list(str)
    :ivar certificate_compression: a list of compression algorithms that will

should be :ivar certificate_compression_receive:

---

tlslite/handshakesettings.py line 640 at r1 (raw file):

        if other.certificate_compression_send:
            if isinstance(other.certificate_compression_send, str):

err... I don't think we want to support string as the value, if the user wants to use a specific algorithm, they can set it to a tuple

---

tlslite/handshakesettings.py line 644 at r1 (raw file):

                    other.certificate_compression_send
                ]
            elif not isinstance(other.certificate_compression_send, list):

a tuple should also be accepted... 😁
in general we just need an iterable, as long as we can iterate over it, we're good, there's no need to do an explicit check for the tape

---

tlslite/handshakesettings.py line 659 at r1 (raw file):

                    other.certificate_compression_receive
                ]
            elif not isinstance(other.certificate_compression_receive, list):

same here, no need for explicit check

---

tlslite/messages.py line 556 at r1 (raw file):

        if val is not None:
            ext = CompressCertificateExtension().create(val)
            self.addExtension(ext)

we actually don't want those types of methods, the others are here because they've been here since before tls 1.2 was supported; new code should just look through extensions list by itself

---

tlslite/messages.py line 1324 at r1 (raw file):

            ext = CompressCertificateExtension().create(val)
            self.addExtension(ext)

same here, no need to define property, .extensions are a public API

---

tlslite/messages.py line 2521 at r1 (raw file):

        elif self.compression_algo == CompressionAlgorithm.brotli:
            compressed_msg = compression_algo_impls["brotli_compress"](msg)
        else:  # self.compression_algo == CompressionAlgorithm.zstd

please change this comment into an assert

---

tlslite/messages.py line 2526 at r1 (raw file):

        return compressed_msg

    def _decompress(self, compressed_msg, exprected_length):

typo, should be expected_length

---

tlslite/messages.py line 2548 at r1 (raw file):

                    decompressed_msg = compression_algo_impls["brotli_decompress"](
                        compressed_msg)
            else:  # self.compression_algo == CompressionAlgorithm.zstd

again, please change into an assert

---

tlslite/messages.py line 2555 at r1 (raw file):

                    decompressed_msg = compression_algo_impls["zstd_decompress"](
                        compressed_msg)
        except Exception:  # pragma: no cover

this definitely should be covered 😁

---

tlslite/messages.py line 2568 at r1 (raw file):

        super(CompressedCertificate, self).create(cert_chain, context)
        self.compression_algo = compression_algo
        self._compression_cache= None

nit: please put spaces around =

---

tlslite/messages.py line 2576 at r1 (raw file):

        p.startLengthCheck(3)
        self.compression_algo = p.get(2)
        exprected_length = p.get(3)

typo, should be expected_length

---

tlslite/messages.py line 2581 at r1 (raw file):

        certificate_msg = self._decompress(compressed_msg, exprected_length)

        parser = Parser(exprected_length.to_bytes(3, 'big') + certificate_msg)

I don't think to_bytes is compatible with old pythons...

---

tlslite/messages.py line 2604 at r1 (raw file):

        return "Compressed {0}".format(
            super(CompressedCertificate, self).__repr__()
        )

nit: newline missing

---

tlslite/tlsconnection.py line 91 at r1 (raw file):

        self._pha_supported = False
        self.client_cert_compression_algo = None
        self.server_cert_compression_algo = None

documentation for those two?

also, we should be explicit that the client_cert_compression_algo will contain information about only the last CompressedCertificate message received when PHA is in use, not a specific one

---

tlslite/tlsconnection.py line 1317 at r1 (raw file):

                                       (HandshakeType.certificate_request,
                                        HandshakeType.certificate,
                                        HandshakeType.compressed_certificate),

Nope. This should be conditional, based on if we have sent the extension or not.

---

tlslite/tlsconnection.py line 1332 at r1 (raw file):

                                           (HandshakeType.certificate,
                                            HandshakeType.\
                                                compressed_certificate),

again, should be conditional

---

tlslite/tlsconnection.py line 2530 at r1 (raw file):

                ]
            else:
                algos_numbers = [1]

no, if there are no settings then we don't know, I don't think we can ask for certificate compression then... or am I missing something?

---

tlslite/tlsconnection.py line 3012 at r1 (raw file):

            for result in self._getMsg(ContentType.handshake,
                                       (HandshakeType.certificate,
                                        HandshakeType.compressed_certificate),

again, should depend on us having sent the extension or not

---

tlslite/tlsrecordlayer.py line 341 at r1 (raw file):

                                  HandshakeType.key_update,
                                  HandshakeType.certificate,
                                  HandshakeType.compressed_certificate)

this too should be conditional, but making it conditional may be rather ugly 🫤

---

tlslite/tlsrecordlayer.py line 713 at r1 (raw file):

                                                self.version)
            client_certificate.create(
                cert, cert_request.certificate_request_context)

don't we have effectively the same code both here and in tlsconnection? why not make it a private method in tlsrecordlayer, so that it can be reused in tlsconnection?

---

tlslite/utils/compression.py line 16 at r1 (raw file):

    "zstd_compress": None,
    "zstd_decompress": None,
    "zstd_accepts_limit": None

what about zlib?

---

tlslite/utils/brotlidecpy/__init__.py line 11 at r1 (raw file):

# noinspection PyUnresolvedReferences
from .decode import brotli_decompress_buffer as decompress

there really isn't a pypi package with it??

also, if we do copy it over, we should copy LICENCE file too
also include information in our LICENCE and readme that we use it

[GeorgePantelakis]

Reviewable status: 1 of 23 files reviewed, 28 unresolved discussions (waiting on @tomato42)

---

scripts/tls.py line 81 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

"Certificate Compression"?

Maybe Compression Algorithms since it is loaded or not loaded.

---

tests/tlstest.py line 2041 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

since client cert was not used here, we should also assert that the client side is None
and also add it to the client side of this test

similarly, we should add this assert to one of the tests that do use client certificates, to show that it is set for client certificates too

Done.

---

tests/tlstest.py line 2082 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

this should be HandshakeServer...

Done.

---

tlslite/constants.py line 585 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

In the RFC it's a CertificateCompressionAlgorithm...

Done.

---

tlslite/constants.py line 586 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

please add reference to the RFC here

Done.

---

tlslite/constants.py line 591 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

if you define all then you need to also re-declare def toRepr that that ignores it, see ECPointFormat

We don't need an all-variable after all. I do not use it.

---

tlslite/extensions.py line 2161 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

I don't think we need to shorten it, I think it can be the full CertificateCompressionAlgorithmsExtension (from struct name) or CompressedCertificateExtension (from the extension ID name)

Changed it to CompressedCertificateExtension

---

tlslite/handshakesettings.py line 372 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

should be :ivar certificate_compression_send:

Done.

---

tlslite/handshakesettings.py line 378 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

should be :ivar certificate_compression_receive:

Done.

---

tlslite/handshakesettings.py line 640 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

err... I don't think we want to support string as the value, if the user wants to use a specific algorithm, they can set it to a tuple

Done.

---

tlslite/handshakesettings.py line 644 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

a tuple should also be accepted... 😁
in general we just need an iterable, as long as we can iterate over it, we're good, there's no need to do an explicit check for the tape

Used hasattr(..., __iter__) to ensure that it is iterable

---

tlslite/handshakesettings.py line 659 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

same here, no need for explicit check

Done.

---

tlslite/messages.py line 556 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

we actually don't want those types of methods, the others are here because they've been here since before tls 1.2 was supported; new code should just look through extensions list by itself

Done

---

tlslite/messages.py line 1324 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

same here, no need to define property, .extensions are a public API

Done.

---

tlslite/messages.py line 2521 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

please change this comment into an assert

Done.

---

tlslite/messages.py line 2548 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

again, please change into an assert

Done.

---

tlslite/messages.py line 2555 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

this definitely should be covered 😁

Done.

---

tlslite/messages.py line 2581 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

I don't think to_bytes is compatible with old pythons...

Done.

---

tlslite/tlsconnection.py line 91 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

documentation for those two?

also, we should be explicit that the client_cert_compression_algo will contain information about only the last CompressedCertificate message received when PHA is in use, not a specific one

Done.

---

tlslite/tlsconnection.py line 1317 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

Nope. This should be conditional, based on if we have sent the extension or not.

Done.

---

tlslite/tlsconnection.py line 1332 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

again, should be conditional

Done.

---

tlslite/tlsconnection.py line 2530 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

no, if there are no settings then we don't know, I don't think we can ask for certificate compression then... or am I missing something?

Done.

---

tlslite/tlsconnection.py line 3012 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

again, should depend on us having sent the extension or not

Done.

---

tlslite/tlsrecordlayer.py line 341 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

this too should be conditional, but making it conditional may be rather ugly 🫤

Yes, it is really ugly (looping through all the cert requests) but it is conditional. So goal was achieved (I think).

---

tlslite/tlsrecordlayer.py line 713 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

don't we have effectively the same code both here and in tlsconnection? why not make it a private method in tlsrecordlayer, so that it can be reused in tlsconnection?

Done.

---

tlslite/utils/compression.py line 16 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

what about zlib?

zlib is Python buildin and it is available in all supported Python versions so we will always support it either way

---

tlslite/utils/brotlidecpy/__init__.py line 11 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

there really isn't a pypi package with it??

also, if we do copy it over, we should copy LICENCE file too
also include information in our LICENCE and readme that we use it

We can always not use it. It is not that fast either way. I only put it there because we wanted some default fallback decode code in pure python. If we want it there still I can add all that was mentioned before

[GeorgePantelakis]

Reviewable status: 1 of 23 files reviewed, 28 unresolved discussions (waiting on @tomato42)

---

-- commits line 2 at r1:

Previously, tomato42 (Hubert Kario) wrote…

"Support for compressed_certificate extension"?

Done.

[GeorgePantelakis]

@tomato42 I think it is ready for another review!

[tomato42]

Reviewed 1 of 23 files at r1, 6 of 14 files at r3, 18 of 18 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 16 unresolved discussions (waiting on @GeorgePantelakis)

---

.github/workflows/ci.yml line 222 at r4 (raw file):

            os: ubuntu-latest
            python-version: '3.12'
            opt-deps: ['brotli', 'zstd']

first, please place above the # finally test with multiple dependencies installed at the same time line
second, please add the brotli and zstd deps to the runs with all dependencies

---

.github/workflows/ci.yml line 347 at r4 (raw file):

        run: pip install brotli
      - name: Install zstandard for py3.8 and after
        if: ${{ contains(matrix.opt-deps, 'zstd') && matrix.python-version != '2.7' && matrix.python-version != '3.6' && matrix.python-version != '3.7' }}

why do we need the check for versions here if we don't have it in the matrix above?

---

.github/workflows/ci.yml line 359 at r4 (raw file):

          wget https://files.pythonhosted.org/packages/85/d5/818d0e603685c4a613d56f065a721013e942088047ff1027a632948bdae6/coverage-4.5.4.tar.gz
          wget https://files.pythonhosted.org/packages/a8/5a/5cf074e1c6681dcbb4e640113f58bed16955e7da9a6c8090b518031775e7/hypothesis-2.0.0.tar.gz
          wget https://files.pythonhosted.org/packages/f8/86/410d53faff049641f34951843245d168261512aea787a1f9f05c3fa025a0/pylint-1.7.6-py2.py3-none-any.whl

why we have this in the diff?

---

.github/workflows/ci.yml line 385 at r4 (raw file):

          wget https://files.pythonhosted.org/packages/c7/a3/c5da2a44c85bfbb6eebcfc1dde24933f8704441b98fdde6528f4831757a6/linecache2-1.0.0-py2.py3-none-any.whl
          wget https://files.pythonhosted.org/packages/bc/a9/01ffebfb562e4274b6487b4bb1ddec7ca55ec7510b22e4c51f14098443b8/chardet-3.0.4-py2.py3-none-any.whl
          wget https://files.pythonhosted.org/packages/bd/c9/6fdd990019071a4a32a5e7cb78a1d92c53851ef4f56f62a3486e6a7d8ffb/urllib3-1.23-py2.py3-none-any.whl

same here, why it's in the diff?

---

scripts/tls.py line 81 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

Maybe Compression Algorithms since it is loaded or not loaded.

there are multiple compression algorithms in TLS and HTTP, we really need to be precise here and specify we're talking about certificate compression algorithms

---

tlslite/handshakesettings.py line 644 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

Used hasattr(..., __iter__) to ensure that it is iterable

that's not pythonic, in general the code should just try to do something instead of testing if it can do something before attempting it

---

tlslite/handshakesettings.py line 659 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

Done.

same here

---

tlslite/tlsconnection.py line 66 at r4 (raw file):

    :py:class:`~.integration.tlsasyncdispatchermixin.TLSAsyncDispatcherMixIn`).

    :vartype client_cert_compression_algo: string

the type is str

---

tlslite/tlsconnection.py line 68 at r4 (raw file):

    :vartype client_cert_compression_algo: string
    :ivar client_cert_compression_algo: Set to the compression algorithm used
        for the compression of the server certificate. In the case of multiple

"server certificate"?

---

tlslite/tlsconnection.py line 73 at r4 (raw file):

        used then it is set to None.

    :vartype server_cert_compression_algo: string

the type is str

---

tlslite/tlsconnection.py line 2535 at r4 (raw file):

                algos_numbers = []
            extensions.append(CompressedCertificateExtension().create(
                algos_numbers))

this will create a malformed extension, won't it ?

---

tlslite/tlsrecordlayer.py line 341 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

Yes, it is really ugly (looping through all the cert requests) but it is conditional. So goal was achieved (I think).

yup, looks good

---

tlslite/tlsrecordlayer.py line 339 at r4 (raw file):

            elif self._cert_requests:
                cert_req_with_comp_cert_ext = False
                for _, cert_request in self._cert_requests.items():

_cert_requests.values()?

---

tlslite/utils/brotlidecpy/__init__.py line 11 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

We can always not use it. It is not that fast either way. I only put it there because we wanted some default fallback decode code in pure python. If we want it there still I can add all that was mentioned before

no, I'd rather use it, less issues with portability, but we need to follow the requirements of the LICENCE of the code we're adding

---

unit_tests/test_tlslite_handshakesettings.py line 497 at r4 (raw file):

        )

        hs.certificate_compression_send = "test"

those should be separate unit tests

---

unit_tests/test_tlslite_messages.py line 4061 at r4 (raw file):

    @unittest.skipIf(PY_VER < (3, ),
        "In Python2 zlib fails to decompress and empty message")

shouldn't it be "an"?

---

unit_tests/test_tlslite_utils_compression.py line 35 at r4 (raw file):

        extension.change_algos([1])
        algo = choose_compression_send_algo((3, 4), extension, ['zlib'])

those should be separate unit tests

[GeorgePantelakis]

Reviewable status: 20 of 28 files reviewed, 16 unresolved discussions (waiting on @tomato42)

---

.github/workflows/ci.yml line 222 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

first, please place above the # finally test with multiple dependencies installed at the same time line
second, please add the brotli and zstd deps to the runs with all dependencies

Done.

---

.github/workflows/ci.yml line 347 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

why do we need the check for versions here if we don't have it in the matrix above?

Done.

---

.github/workflows/ci.yml line 359 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

why we have this in the diff?

spaces in the end that my IDE is removing automagically

---

.github/workflows/ci.yml line 385 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

same here, why it's in the diff?

spaces in the end that my IDE is removing automagically

---

scripts/tls.py line 81 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

there are multiple compression algorithms in TLS and HTTP, we really need to be precise here and specify we're talking about certificate compression algorithms

Done.

---

tlslite/handshakesettings.py line 644 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

that's not pythonic, in general the code should just try to do something instead of testing if it can do something before attempting it

Done.

---

tlslite/handshakesettings.py line 659 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

same here

Done.

---

tlslite/tlsconnection.py line 66 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

the type is str

Done.

---

tlslite/tlsconnection.py line 68 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

"server certificate"?

Done.

---

tlslite/tlsconnection.py line 73 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

the type is str

Done.

---

tlslite/tlsconnection.py line 2535 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

this will create a malformed extension, won't it ?

Done.

---

tlslite/tlsrecordlayer.py line 339 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

_cert_requests.values()?

Done.

---

tlslite/utils/brotlidecpy/__init__.py line 11 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

no, I'd rather use it, less issues with portability, but we need to follow the requirements of the LICENCE of the code we're adding

What are the requirements? Do we need to do something else? I don't see something specific.

---

unit_tests/test_tlslite_handshakesettings.py line 497 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

those should be separate unit tests

Done.

---

unit_tests/test_tlslite_messages.py line 4061 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

shouldn't it be "an"?

Done.

---

unit_tests/test_tlslite_utils_compression.py line 35 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

those should be separate unit tests

Don't get what exactly should be separate tests.

[tomato42]

Reviewed 8 of 8 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @GeorgePantelakis)

---

tlslite/utils/brotlidecpy/__init__.py line 11 at r1 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

What are the requirements? Do we need to do something else? I don't see something specific.

ah, I was thinking of the BSD licence, this is the MIT licence, so what we have now is more than enough

---

unit_tests/test_tlslite_handshakesettings.py line 509 at r6 (raw file):

        with self.assertRaises(ValueError) as e:
            hs.validate()

this too should be in a separate unit test (in general, every .validate() call should be in separate unit test)

---

unit_tests/test_tlslite_utils_compression.py line 35 at r4 (raw file):

Previously, GeorgePantelakis (George Pantelakis) wrote…

Don't get what exactly should be separate tests.

to make sure that we get all errors when the test cases are executed, not just the first failure

[GeorgePantelakis]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tomato42)

---

unit_tests/test_tlslite_handshakesettings.py line 509 at r6 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

this too should be in a separate unit test (in general, every .validate() call should be in separate unit test)

Done.

---

unit_tests/test_tlslite_utils_compression.py line 35 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

to make sure that we get all errors when the test cases are executed, not just the first failure

Done.

[tomato42]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @GeorgePantelakis)

[tomato42]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @GeorgePantelakis)

[tomato42]

the CI failures looks relevant

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @GeorgePantelakis)

[tomato42]

Reviewed 10 of 10 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @GeorgePantelakis)