Repository: abhinavsingh/proxy.py (python)

Issue:
[MacOS] Add flag to specify custom system CA Path
Today setup `proxy.py` on a fresh Macbook (Catalina) and observed Apple has further made it harder to reliably run interceptor.  Had to hardcode `certifi` ca file to make it work

<img width="920" alt="Screen Shot 2020-03-24 at 9 59 39 PM" src="https://user-images.githubusercontent.com/126065/77451514-c7e37880-6e1a-11ea-971a-50094980c5a8.png">

Also see https://stackoverflow.com/questions/57630314/ssl-certificate-verify-failed-error-with-python3-on-macos-10-15 for some background.

Pull request:
Add flag to specify custom system CA Path
Fixes #320

This is mostly needed on MacOS where Python now don't have direct access to MacOS root certificates.

Patch:
diff --git a/README.md b/README.md
index 7984b77..9140f9d 100644
--- a/README.md
+++ b/README.md
@@ -777,14 +777,14 @@ TLS Interception
 =================
 
 By default, `proxy.py` will not decrypt `https` traffic between client and server.
-To enable TLS interception first generate CA certificates:
+To enable TLS interception first generate root CA certificates:
 
-```
-make ca-certificates
+```bash
+❯ make ca-certificates
 ```
 
 Lets also enable `CacheResponsePlugin` so that we can verify decrypted
-response from the server. Start `proxy.py` as:
+response from the server.  Start `proxy.py` as:
 
 ```bash
 ❯ proxy \
@@ -794,7 +794,16 @@ response from the server. Start `proxy.py` as:
     --ca-signing-key-file ca-signing-key.pem
 ```
 
-Verify using `curl -v -x localhost:8899 --cacert ca-cert.pem https://httpbin.org/get`
+
+> :note: **MacOS users** also need to pass explicit CA file path
+> needed for validation of peer certificates. See --ca-file flag.
+
+
+Verify TLS interception using `curl`
+
+```bash
+❯ curl -v -x localhost:8899 --cacert ca-cert.pem https://httpbin.org/get
+```
 
 ```bash
 *  issuer: C=US; ST=CA; L=SanFrancisco; O=proxy.py; OU=CA; CN=Proxy PY CA; emailAddress=proxyca@mailserver.com
@@ -1530,6 +1539,8 @@ optional arguments:
                         Default: None. Signing certificate to use for signing
                         dynamically generated HTTPS certificates. If used,
                         must also pass --ca-key-file and --ca-signing-key-file
+  --ca-file CA_FILE     Default: None. Provide path to custom CA file for peer
+                        certificate validation. Specially useful on MacOS.
   --ca-signing-key-file CA_SIGNING_KEY_FILE
                         Default: None. CA signing key to use for dynamic
                         generation of HTTPS certificates. If used, must also
diff --git a/proxy/common/constants.py b/proxy/common/constants.py
index 72efbce..a4a32bf 100644
--- a/proxy/common/constants.py
+++ b/proxy/common/constants.py
@@ -44,6 +44,7 @@ DEFAULT_CA_CERT_FILE = None
 DEFAULT_CA_KEY_FILE = None
 DEFAULT_CA_SIGNING_KEY_FILE = None
 DEFAULT_CERT_FILE = None
+DEFAULT_CA_FILE = None
 DEFAULT_CLIENT_RECVBUF_SIZE = DEFAULT_BUFFER_SIZE
 DEFAULT_DEVTOOLS_WS_PATH = b'/devtools'
 DEFAULT_DISABLE_HEADERS: List[bytes] = []
diff --git a/proxy/common/flags.py b/proxy/common/flags.py
index 30aaf6f..5b74173 100644
--- a/proxy/common/flags.py
+++ b/proxy/common/flags.py
@@ -28,7 +28,7 @@ from .utils import text_, bytes_
 from .constants import DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE, DEFAULT_LOG_FORMAT, DEFAULT_BACKLOG, DEFAULT_BASIC_AUTH
 from .constants import DEFAULT_TIMEOUT, DEFAULT_DEVTOOLS_WS_PATH, DEFAULT_DISABLE_HTTP_PROXY, DEFAULT_DISABLE_HEADERS
 from .constants import DEFAULT_ENABLE_STATIC_SERVER, DEFAULT_ENABLE_EVENTS, DEFAULT_ENABLE_DEVTOOLS
-from .constants import DEFAULT_ENABLE_WEB_SERVER, DEFAULT_THREADLESS, DEFAULT_CERT_FILE, DEFAULT_KEY_FILE
+from .constants import DEFAULT_ENABLE_WEB_SERVER, DEFAULT_THREADLESS, DEFAULT_CERT_FILE, DEFAULT_KEY_FILE, DEFAULT_CA_FILE
 from .constants import DEFAULT_CA_CERT_DIR, DEFAULT_CA_CERT_FILE, DEFAULT_CA_KEY_FILE, DEFAULT_CA_SIGNING_KEY_FILE
 from .constants import DEFAULT_PAC_FILE_URL_PATH, DEFAULT_PAC_FILE, DEFAULT_PLUGINS, DEFAULT_PID_FILE, DEFAULT_PORT
 from .constants import DEFAULT_NUM_WORKERS, DEFAULT_VERSION, DEFAULT_OPEN_FILE_LIMIT, DEFAULT_IPV6_HOSTNAME
@@ -67,6 +67,7 @@ class Flags:
             ca_key_file: Optional[str] = None,
             ca_cert_file: Optional[str] = None,
             ca_signing_key_file: Optional[str] = None,
+            ca_file: Optional[str] = None,
             num_workers: int = 0,
             hostname: Union[ipaddress.IPv4Address,
                             ipaddress.IPv6Address] = DEFAULT_IPV6_HOSTNAME,
@@ -98,6 +99,7 @@ class Flags:
         self.ca_key_file: Optional[str] = ca_key_file
         self.ca_cert_file: Optional[str] = ca_cert_file
         self.ca_signing_key_file: Optional[str] = ca_signing_key_file
+        self.ca_file = ca_file
         self.num_workers: int = num_workers if num_workers > 0 else multiprocessing.cpu_count()
         self.hostname: Union[ipaddress.IPv4Address,
                              ipaddress.IPv6Address] = hostname
@@ -223,6 +225,11 @@ class Flags:
                 opts.get(
                     'ca_signing_key_file',
                     args.ca_signing_key_file)),
+            ca_file=cast(
+                Optional[str],
+                opts.get(
+                    'ca_file',
+                    args.ca_file)),
             hostname=cast(Union[ipaddress.IPv4Address,
                                 ipaddress.IPv6Address],
                           opts.get('hostname', ipaddress.ip_address(args.hostname))),
@@ -308,6 +315,13 @@ class Flags:
             help='Default: None. Signing certificate to use for signing dynamically generated '
                  'HTTPS certificates.  If used, must also pass --ca-key-file and --ca-signing-key-file'
         )
+        parser.add_argument(
+            '--ca-file',
+            type=str,
+            default=DEFAULT_CA_FILE,
+            help='Default: None. Provide path to custom CA file for peer certificate validation. '
+                 'Specially useful on MacOS.'
+        )
         parser.add_argument(
             '--ca-signing-key-file',
             type=str,
diff --git a/proxy/http/proxy/server.py b/proxy/http/proxy/server.py
index 500aec4..7662414 100644
--- a/proxy/http/proxy/server.py
+++ b/proxy/http/proxy/server.py
@@ -89,13 +89,13 @@ class HttpProxyPlugin(HttpProtocolHandlerPlugin):
             logger.debug('Server is write ready, flushing buffer')
             try:
                 self.server.flush()
-            except OSError:
-                logger.error('OSError when flushing buffer to server')
-                return True
             except BrokenPipeError:
                 logger.error(
                     'BrokenPipeError when flushing buffer for server')
                 return True
+            except OSError:
+                logger.error('OSError when flushing buffer to server')
+                return True
         return False
 
     def read_from_descriptors(self, r: List[Union[int, HasFileno]]) -> bool:
@@ -274,13 +274,13 @@ class HttpProxyPlugin(HttpProtocolHandlerPlugin):
                     # wrap_client also flushes client data before wrapping
                     # sending to client can raise, handle expected exceptions
                     self.wrap_client()
-                except OSError:
-                    logger.error('OSError when wrapping client')
-                    return True
                 except BrokenPipeError:
                     logger.error(
                         'BrokenPipeError when wrapping client')
                     return True
+                except OSError:
+                    logger.error('OSError when wrapping client')
+                    return True
                 # Update all plugin connection reference
                 for plugin in self.plugins.values():
                     plugin.client._conn = self.client.connection
@@ -380,7 +380,7 @@ class HttpProxyPlugin(HttpProtocolHandlerPlugin):
         assert self.server is not None
         assert isinstance(self.server.connection, socket.socket)
         ctx = ssl.create_default_context(
-            ssl.Purpose.SERVER_AUTH)
+            ssl.Purpose.SERVER_AUTH, cafile=self.flags.ca_file)
         ctx.options |= ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 | ssl.OP_NO_TLSv1 | ssl.OP_NO_TLSv1_1
         self.server.connection.setblocking(True)
         self.server._conn = ctx.wrap_socket(


Test patch:
diff --git a/tests/codecov.yml b/tests/codecov.yml
new file mode 100644
index 0000000..f393de9
--- /dev/null
+++ b/tests/codecov.yml
@@ -0,0 +1,12 @@
+codecov:
+  require_ci_to_pass: yes
+  notify:
+    wait_for_ci: yes
+coverage:
+  status:
+    project:
+      default:
+        threshold: 1%
+    patch:
+      default:
+        threshold: 1%
diff --git a/tests/http/test_http_proxy_tls_interception.py b/tests/http/test_http_proxy_tls_interception.py
index ff01a7c..6f4d93e 100644
--- a/tests/http/test_http_proxy_tls_interception.py
+++ b/tests/http/test_http_proxy_tls_interception.py
@@ -69,7 +69,7 @@ class TestHttpProxyTlsInterception(unittest.TestCase):
         self.flags = Flags(
             ca_cert_file='ca-cert.pem',
             ca_key_file='ca-key.pem',
-            ca_signing_key_file='ca-signing-key.pem',
+            ca_signing_key_file='ca-signing-key.pem'
         )
         self.plugin = mock.MagicMock()
         self.proxy_plugin = mock.MagicMock()
@@ -135,7 +135,7 @@ class TestHttpProxyTlsInterception(unittest.TestCase):
         self.mock_server_conn.return_value.connection.setblocking.assert_called_with(
             False)
 
-        self.mock_ssl_context.assert_called_with(ssl.Purpose.SERVER_AUTH)
+        self.mock_ssl_context.assert_called_with(ssl.Purpose.SERVER_AUTH, cafile=None)
         # self.assertEqual(self.mock_ssl_context.return_value.options,
         # ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 | ssl.OP_NO_TLSv1 |
         # ssl.OP_NO_TLSv1_1)