Repository: certbot/certbot

Issue:
HTTP responses logged as hard-to-read bytes repr in Python 3
If you're having trouble using Certbot and aren't sure you've found a bug or
request for a new feature, please first try asking for help at
https://community.letsencrypt.org/. There is a much larger community there of
people familiar with the project who will be able to more quickly answer your
questions.

## My operating system is (include version):
Ubuntu 16.04 (x86-64)

## I installed Certbot with (certbot-auto, OS package manager, pip, etc):
certbot-auto (Python 2) and the PPA (0.22.2, Python 3) on different systems.

## I ran this command and it produced this output:


## Certbot's behavior differed from what I expected because:
Under Python 2, Certbot logs HTTP responses as easy-to-read text, e.g.:

    2018-05-02 13:41:58,328:DEBUG:acme.client:Received response:
    HTTP 200
    Server: nginx
    Content-Type: application/json
    Content-Length: 724
    X-Frame-Options: DENY
    Strict-Transport-Security: max-age=604800
    Expires: Wed, 02 May 2018 13:41:58 GMT
    Cache-Control: max-age=0, no-cache, no-store
    Pragma: no-cache
    Date: Wed, 02 May 2018 13:41:58 GMT
    Connection: keep-alive

    {
      "6ELa2lV28v0": "https://community.letsencrypt.org/t/adding-random-entries-to-the-directory/33417",
      "keyChange": "https://acme-staging-v02.api.letsencrypt.org/acme/key-change",
      "meta": {
        "caaIdentities": [
          "letsencrypt.org"
        ],
        "termsOfService": "https://letsencrypt.org/documents/LE-SA-v1.2-November-15-2017.pdf",
        "website": "https://letsencrypt.org/docs/staging-environment/"
      },
      "newAccount": "https://acme-staging-v02.api.letsencrypt.org/acme/new-acct",
      "newNonce": "https://acme-staging-v02.api.letsencrypt.org/acme/new-nonce",
      "newOrder": "https://acme-staging-v02.api.letsencrypt.org/acme/new-order",
      "revokeCert": "https://acme-staging-v02.api.letsencrypt.org/acme/revoke-cert"
    }

Under Python 3, it logs the `repr()` of the bytes:

    2018-05-02 13:44:24,936:DEBUG:acme.client:Received response:
    HTTP 200
    Server: nginx
    Content-Type: application/json
    Content-Length: 724
    X-Frame-Options: DENY
    Strict-Transport-Security: max-age=604800
    Expires: Wed, 02 May 2018 13:44:24 GMT
    Cache-Control: max-age=0, no-cache, no-store
    Pragma: no-cache
    Date: Wed, 02 May 2018 13:44:24 GMT
    Connection: keep-alive
    
    b'{\n  "keyChange": "https://acme-staging-v02.api.letsencrypt.org/acme/key-change",\n  "meta": {\n    "caaIdentities": [\n      "letsencrypt.org"\n    ],\n    "termsOfService": "https://letsencrypt.org/documents/LE-SA-v1.2-November-15-2017.pdf",\n    "website": "https://letsencrypt.org/docs/staging-environment/"\n  },\n  "newAccount": "https://acme-staging-v02.api.letsencrypt.org/acme/new-acct",\n  "newNonce": "https://acme-staging-v02.api.letsencrypt.org/acme/new-nonce",\n  "newOrder": "https://acme-staging-v02.api.letsencrypt.org/acme/new-order",\n  "revokeCert": "https://acme-staging-v02.api.letsencrypt.org/acme/revoke-cert",\n  "uog1GW6DtvQ": "https://community.letsencrypt.org/t/adding-random-entries-to-the-directory/33417"\n}'

It makes log files more difficult to read.

Ideally Certbot would try to decode the HTTP response and log it normally. (Or perhaps dump the raw bytes straight to the log file.)

This affects JSON responses and certificates, but a certificate's worth of base64 isn't human readable anyway.

## Here is a Certbot log showing the issue (if available):
###### Logs are stored in `/var/log/letsencrypt` by default. Feel free to redact domains, e-mail and IP addresses as you see fit.

## Here is the relevant nginx server block or Apache virtualhost for the domain I am configuring:

Discussion:

Patch:
diff --git a/acme/acme/client.py b/acme/acme/client.py
index 7807f0ece..bdc07fb1c 100644
--- a/acme/acme/client.py
+++ b/acme/acme/client.py
@@ -12,7 +12,9 @@ from six.moves import http_client  # pylint: disable=import-error
 import josepy as jose
 import OpenSSL
 import re
+from requests_toolbelt.adapters.source import SourceAddressAdapter
 import requests
+from requests.adapters import HTTPAdapter
 import sys
 
 from acme import crypto_util
@@ -857,9 +859,12 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     :param bool verify_ssl: Whether to verify certificates on SSL connections.
     :param str user_agent: String to send as User-Agent header.
     :param float timeout: Timeout for requests.
+    :param source_address: Optional source address to bind to when making requests.
+    :type source_address: str or tuple(str, int)
     """
     def __init__(self, key, account=None, alg=jose.RS256, verify_ssl=True,
-                 user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT):
+                 user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT,
+                 source_address=None):
         # pylint: disable=too-many-arguments
         self.key = key
         self.account = account
@@ -869,6 +874,13 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         self.user_agent = user_agent
         self.session = requests.Session()
         self._default_timeout = timeout
+        adapter = HTTPAdapter()
+
+        if source_address is not None:
+            adapter = SourceAddressAdapter(source_address)
+
+        self.session.mount("http://", adapter)
+        self.session.mount("https://", adapter)
 
     def __del__(self):
         # Try to close the session, but don't show exceptions to the
@@ -1018,7 +1030,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         if response.headers.get("Content-Type") == DER_CONTENT_TYPE:
             debug_content = base64.b64encode(response.content)
         else:
-            debug_content = response.content
+            debug_content = response.content.decode("utf-8")
         logger.debug('Received response:\nHTTP %d\n%s\n\n%s',
                      response.status_code,
                      "\n".join(["{0}: {1}".format(k, v)
diff --git a/acme/setup.py b/acme/setup.py
index 72ab5919b..e91c36b3d 100644
--- a/acme/setup.py
+++ b/acme/setup.py
@@ -19,6 +19,7 @@ install_requires = [
     'pyrfc3339',
     'pytz',
     'requests[security]>=2.4.1',  # security extras added in 2.4.1
+    'requests-toolbelt>=0.3.0',
     'setuptools',
     'six>=1.9.0',  # needed for python_2_unicode_compatible
 ]
diff --git a/certbot/main.py b/certbot/main.py
index a041b998f..0ae5b9d7a 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -324,7 +324,7 @@ def _find_lineage_for_domains_and_certname(config, domains, certname):
                 return "newcert", None
             else:
                 raise errors.ConfigurationError("No certificate with name {0} found. "
-                    "Use -d to specify domains, or run certbot --certificates to see "
+                    "Use -d to specify domains, or run certbot certificates to see "
                     "possible certificate names.".format(certname))
 
 def _get_added_removed(after, before):
diff --git a/docs/using.rst b/docs/using.rst
index 272f5ac6e..40d8f8452 100644
--- a/docs/using.rst
+++ b/docs/using.rst
@@ -609,7 +609,7 @@ commands into your individual environment.
 .. note:: ``certbot renew`` exit status will only be 1 if a renewal attempt failed.
   This means ``certbot renew`` exit status will be 0 if no certificate needs to be updated.
   If you write a custom script and expect to run a command only after a certificate was actually renewed
-  you will need to use the ``--post-hook`` since the exit status will be 0 both on successful renewal
+  you will need to use the ``--deploy-hook`` since the exit status will be 0 both on successful renewal
   and when renewal is not necessary.
 
 .. _renewal-config-file:


Test patch:
diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py
index c17b83210..f3018ed81 100644
--- a/acme/acme/client_test.py
+++ b/acme/acme/client_test.py
@@ -1129,6 +1129,31 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase):
         self.assertRaises(requests.exceptions.RequestException,
                           self.net.post, 'uri', obj=self.obj)
 
+class ClientNetworkSourceAddressBindingTest(unittest.TestCase):
+    """Tests that if ClientNetwork has a source IP set manually, the underlying library has
+    used the provided source address."""
+
+    def setUp(self):
+        self.source_address = "8.8.8.8"
+
+    def test_source_address_set(self):
+        from acme.client import ClientNetwork
+        net = ClientNetwork(key=None, alg=None, source_address=self.source_address)
+        for adapter in net.session.adapters.values():
+            self.assertTrue(self.source_address in adapter.source_address)
+
+    def test_behavior_assumption(self):
+        """This is a test that guardrails the HTTPAdapter behavior so that if the default for
+        a Session() changes, the assumptions here aren't violated silently."""
+        from acme.client import ClientNetwork
+        # Source address not specified, so the default adapter type should be bound -- this
+        # test should fail if the default adapter type is changed by requests
+        net = ClientNetwork(key=None, alg=None)
+        session = requests.Session()
+        for scheme in session.adapters.keys():
+            client_network_adapter = net.session.adapters.get(scheme)
+            default_adapter = session.adapters.get(scheme)
+            self.assertEqual(client_network_adapter.__class__, default_adapter.__class__)
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover