Using SSLyze as a Python library instead of shelling out #151

konklone · 2017-11-01T02:00:14Z

This is a better approach than #150, which moves us to using sslyze as a Python library rather than shelling out.

It still uses the concurrent scanner, which itself does spin out multiple processes, so this may yet get us into trouble, but it is now easy for us to use the synchronous scanner if we want to, and in general have more control over the parsing experience.

The timeout library I introduced in #150 can't function with this, as the sslyze native Python calls to do its own multiprocess work interfere with the timeout's multiprocess work when signals are disabled. (When signals are enabled, the timeout library throws an error about sslyze's use of multiple processes. There's no winning.)

If this fails to fix the hanging issues, I'll switch it to use the SynchronousScanner, which will eliminate all use of multiple processes, and we'll get more granular as needed about the root cause.

konklone · 2017-11-01T02:48:49Z

Not for sure yet, but I've been watching somewhere between 30-60 minutes of scans roll by, and no defunct sslyze processes yet! They were common and quick to develop when using the existing method of shelling out. Very promising.

konklone · 2017-11-05T01:45:30Z

I added an update in #138 (comment) - this PR now has both synchronous and asynchronous modes (synchronous can be turned on with --sslyze-serial), but I'm constantly running out of memory. I believe this was happening before as well with sslyze, which was causing some processes to get killed and become zombies. In synchronous mode here, no processes get spawned, and so the kernel kills the main process entirely.

Watching the memory increase over the lifetime of a scan, it looks like a memory leak is happening, and probably was happening previously. I'm going to try to track it down before merge, even though it was likely happening before this PR as well.

python that was being used was the one from Ubuntu. I removed the Ubuntu python and corrected teh PATH so that the pyenv python is used. * I added lines to install GDB and set it up so that the py-bt, py-list, etc. commands will work when the pyenv python is used.

… sslyze-as-library

konklone · 2017-11-05T05:01:58Z

Well, after trying a bunch of things, and the most effective thing I found was to 1) use the concurrent scanner, and 2) add a network_timeout to the server connectivity call.

I've added options to try various combinations of serializing sslyze's scans, and disabling certificate info scans (since they are currently informational and not relevant to M-15-13 or BOD 18-01). A memory leak is consistently reproducible with the synchronous scanner, but not with the concurrent scanner.

It's been a wild ride, but I'm becoming comfortable with this PR, and plan to fix the build and merge it in shortly.

konklone · 2017-11-05T05:58:22Z

requirements.txt

@@ -6,6 +6,7 @@ pyyaml
 # to support sslyze scanner
 sslyze
 cryptography
+timeout-decorator


Oops, a leftover from intermediate work -- I removed this in 57b43a1.

konklone · 2017-11-05T05:58:43Z

Dockerfile

@@ -161,3 +167,5 @@ WORKDIR $SCANNER_HOME
 VOLUME /data

 ENTRYPOINT ["./scan_wrap.sh"]
+
+COPY . $SCANNER_HOME


Moved the COPY command last, since it's non-deterministic, and Docker knows this and will always rebuild from here during docker build.

konklone · 2017-11-05T05:59:30Z

Dockerfile

+ENV LANG en_US.UTF-8
+ENV LANGUAGE en_US:en
+ENV LC_ALL en_US.UTF-8
+RUN pip3 install pshtt


Locked to latest pshtt. This also required me to paste in please-just-make-it-work attempts at fixing a Unicode error that was happening because of the PyPi publicsuffixlist module's LICENSE file having Unicode characters in the author's name.

konklone · 2017-11-05T05:59:40Z

Dockerfile

-RUN cd /go/src/github.com/ssllabs/ssllabs-scan/ \
-      && git checkout stable \
-      && go install
-ENV SSLLABS_PATH /go/bin/ssllabs-scan


The tls scanner and the use of ssllabs-scan is deprecated.

konklone · 2017-11-05T06:00:10Z

Dockerfile

+RUN ln -s /opt/pyenv/sources/${PYENV_PYTHON_VERSION}-debug/Python-${PYENV_PYTHON_VERSION}/python-gdb.py /opt/pyenv/versions/${PYENV_PYTHON_VERSION}-debug/bin/python-gdb.py
+RUN apt-get -qq update && \
+    apt-get -qq --yes --no-install-recommends --no-install-suggests install gdb
+RUN echo add-auto-load-safe-path /opt/pyenv/sources/${PYENV_PYTHON_VERSION}-debug/Python-${PYENV_PYTHON_VERSION}/ >> etc/gdb/gdbinit


Thanks to @jsf9k for his work in figuring out how to create a gdb-ready version of Python that is installed via pyenv, in the Docker container.

konklone · 2017-11-05T06:01:34Z

scanners/sslyze.py

+# Supported options:
+#
+# --sslyze-serial - If set, will use a synchronous (single-threaded
+#   in-process) scanner. Defaults to false.


The --sslyze-serial option isn't documented in the README, it was just used for debugging. In the future, I may remove the SSLyze serial code path altogether, as it generates a memory leak under normal use.

konklone · 2017-11-05T06:02:05Z

scanners/sslyze.py

 ###

+# Number of seconds to wait during sslyze connection check.
+# Not much patience here, and very willing to move on.
+network_timeout = 5


This can be overridden via --network_timeout, but unless it becomes used/wanted by anyone, I don't see the need to document it.

konklone · 2017-11-05T06:02:27Z

scanners/sslyze.py

-        data['protocols']['tlsv1.2'],
+        data['protocols'].get('sslv2'), data['protocols'].get('sslv3'),
+        data['protocols'].get('tlsv1.0'), data['protocols'].get('tlsv1.1'),
+        data['protocols'].get('tlsv1.2'),


If sslyze can't connect to a hostname, all of these fields will be blank.

konklone · 2017-11-05T06:02:58Z

scanners/sslyze.py



 # Given the cert sub-obj from the sslyze JSON, use
 # the cryptography module to parse its PEM contents.
 def parse_cert(cert):
    backend = cryptography.hazmat.backends.openssl.backend
-    pem_bytes = cert['as_pem'].encode('utf-8')
+    pem_bytes = cert.public_bytes(Encoding.PEM).decode('ascii').encode('utf-8')


It's possible there's a more efficient way to do this, but I wasn't able to easily figure out one.

konklone · 2017-11-05T06:03:48Z

scanners/sslyze.py

+
+    # Default to cert info on.
+    if options.get("sslyze-no-certs", False) is False:
+        queue(CertificateInfoScanCommand())


The --sslyze-no-certs option is similarly undocumented - I used it for debugging, and may again, but I don't expect myself or anyone else to use it, and it may get yanked down the line.

konklone added 4 commits October 31, 2017 18:06

local timeout that works on sslyze, but doesn't clean up processes

7d2f49b

sane production timeout value

1ea4f91

move to using sslyze' Python API instead of its JSON output

edd2ba2

remove timeout for now

6cc8b81

konklone mentioned this pull request Nov 1, 2017

Local timeout that kills sslyze scans -- but doesn't clean up processes #150

Closed

fix flake8 errors

e9b599d

konklone mentioned this pull request Nov 2, 2017

sslyze appearing to stall out on some domains #138

Closed

konklone added 4 commits November 4, 2017 17:51

add serial mode for sslyze

9849380

server connectivity errors can happen during initialization too

14dc67e

Merge branch 'master' into sslyze-as-library

faa8502

fix flake8 errors

81c4687

konklone and others added 9 commits November 4, 2017 21:57

remove timeout code

a56ab8d

Merge branch 'sslyze-as-library' of github.com:jsf9k/domain-scan into…

6805fae

… sslyze-as-library

disable cert checking for now

18ed563

fix unicode errors when building publicsuffixlist dependency

06edd6d

adjust placement of copy in docker build

0aeb37f

always close files

d2de308

add network timeout

bd83897

opt in to cert info gathering with --sslyze-certs

b21f54c

konklone added 2 commits November 5, 2017 01:03

switch cert info to default on

0d3c1d4

fix flake8 warnings

449d80e

konklone merged commit 3b2638a into master Nov 5, 2017

konklone deleted the sslyze-as-library branch November 5, 2017 05:27

This was referenced Nov 5, 2017

Updated Dockerfile to use a newer Ubuntu base image (16.04) #152

Closed

Update sslyze and use Python integration #55

Closed

Update sslyze scanner to use 0.12 #45

Closed

konklone commented Nov 5, 2017

View reviewed changes

konklone mentioned this pull request Nov 5, 2017

sslyze calls deadlocking due to combination of threads/processes/logging #157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using SSLyze as a Python library instead of shelling out #151

Using SSLyze as a Python library instead of shelling out #151

konklone commented Nov 1, 2017

konklone commented Nov 1, 2017

konklone commented Nov 5, 2017

konklone commented Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

konklone Nov 5, 2017

Using SSLyze as a Python library instead of shelling out #151

Using SSLyze as a Python library instead of shelling out #151

Conversation

konklone commented Nov 1, 2017

konklone commented Nov 1, 2017

konklone commented Nov 5, 2017

konklone commented Nov 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment