analyze.py stopped working after changes of server-side-tls-conf.json
autopwn opened this issue · 6 comments
The script analyze.py stopped working for me a few days ago. After taking a look at the source code i guess a change of the output of https://statics.tls.security.mozilla.org/server-side-tls-conf.json is the reason. There is no more key ciphersuites in the output but openssl_ciphersuites.
Traceback (most recent call last):
File "./analyze.py", line 491, in <module>
main()
File "./analyze.py", line 476, in main
exit_status=process_results(str(data), args.level, args.json, args.nagios)
File "./analyze.py", line 364, in process_results
measured_lvl = evaluate_all(results)
File "./analyze.py", line 312, in evaluate_all
if is_old(results):
File "./analyze.py", line 124, in is_old
if conn['cipher'] not in old["ciphersuites"]:
KeyError: 'ciphersuites'
With the following changes the script works again:
index 9eac308..4e4eae9 100755
--- a/analyze.py
+++ b/analyze.py
@@ -63,7 +63,7 @@ def is_fubar(results):
pubkey_bits = int(conn['pubkey'][0])
ec_kex = re.match(r"(ECDHE|EECDH|ECDH)-", conn['cipher'])
- if conn['cipher'] not in (set(old["ciphersuites"]) | set(inter["ciphersuites"]) | set(modern["ciphersuites"])):
+ if conn['cipher'] not in (set(old["openssl_ciphersuites"]) | set(inter["openssl_ciphersuites"]) | set(modern["openssl_ciphersuites"])):
failures[lvl].append("remove cipher " + conn['cipher'])
logging.debug(conn['cipher'] + ' is in the list of fubar ciphers')
fubar = True
@@ -121,7 +121,7 @@ def is_old(results):
for conn in results['ciphersuite']:
logging.debug('testing connection %s' % conn)
# flag unwanted ciphers
- if conn['cipher'] not in old["ciphersuites"]:
+ if conn['cipher'] not in old["openssl_ciphersuites"]:
logging.debug(conn['cipher'] + ' is not in the list of old ciphers')
failures[lvl].append("remove cipher " + conn['cipher'])
isold = False
@@ -183,7 +183,7 @@ def is_intermediate(results):
all_proto = []
for conn in results['ciphersuite']:
logging.debug('testing connection %s' % conn)
- if conn['cipher'] not in inter["ciphersuites"]:
+ if conn['cipher'] not in inter["openssl_ciphersuites"]:
logging.debug(conn['cipher'] + ' is not in the list of intermediate ciphers')
failures[lvl].append("remove cipher " + conn['cipher'])
isinter = False
@@ -242,7 +242,7 @@ def is_modern(results):
all_proto = []
for conn in results['ciphersuite']:
logging.debug('testing connection %s' % conn)
- if conn['cipher'] not in modern["ciphersuites"]:
+ if conn['cipher'] not in modern["openssl_ciphersuites"]:
logging.debug(conn['cipher'] + ' is not in the list of modern ciphers')
failures[lvl].append("remove cipher " + conn['cipher'])
ismodern = False
@@ -311,17 +311,17 @@ def evaluate_all(results):
if is_old(results):
status = "old"
- if not is_ordered(results, old["ciphersuites"], "old"):
+ if not is_ordered(results, old["openssl_ciphersuites"], "old"):
status = "old with bad ordering"
if is_intermediate(results):
status = "intermediate"
- if not is_ordered(results, inter["ciphersuites"], "intermediate"):
+ if not is_ordered(results, inter["openssl_ciphersuites"], "intermediate"):
status = "intermediate with bad ordering"
if is_modern(results):
status = "modern"
- if not is_ordered(results, modern["ciphersuites"], "modern"):
+ if not is_ordered(results, modern["openssl_ciphersuites"], "modern"):
status = "modern with bad ordering"
if is_fubar(results):
could you prepare a PR with those changes?
dchandekstark has been faster ;) thank you!
Thanks for reporting, @autopwn !
It appears the ciphersuites key was actually renamed to openssl_ciphers. The openssl_ciphersuites is a separate set that I believe is used for TLS 1.3. I noticed three other issues with the latest guidelines that maybe should be in a separate issue:
- analyze.py hard-codes AES128-SHA as a required cipher for intermediate, even though it is no longer included in the latest guidelines (or the JSON)
- analyze.py hard-codes server side ordering as required, even though that is no longer the case except for the old level. The JSON now has a server_preferred_order key that is false for modern and intermediate but true for old.
- analyze.py passes True for must_match when calling has_good_pfs for each level. That means the DH and ECHD parameters must be equal to the recommendation rather than greater or equal. The current intermediate guideline defines an ecdh_param_size of 256, but the selected ciphers result in a pfs value of ECDH,B-571,570bits causing the analysis to say "consider using DHE of at least 2048bits and ECC 256bit and greater".
1. analyze.py hard-codes AES128-SHA as a required cipher for intermediate, even though it is no longer included in the latest guidelines (or the JSON)
that looks like a bug, but unrelated to the original report, please file a separate one
2. analyze.py hard-codes server side ordering as required, even though that is no longer the case except for the old level. The JSON now has a server_preferred_order key that is false for modern and intermediate but true for old.
yes, that looks like a deficiency, but again unrelated to parsing of the json file
3. analyze.py passes True for must_match when calling has_good_pfs for each level. That means the DH and ECHD parameters must be equal to the recommendation rather than greater or equal. The current intermediate guideline defines an ecdh_param_size of 256, but the selected ciphers result in a pfs value of ECDH,B-571,570bits causing the analysis to say "consider using DHE of at least 2048bits and ECC 256bit and greater".
it should be checking it the P-256 is supported, not if it is the preferred curve of the server
@dchandekstark and @autopwn thanks for the contribution, sorry it took me so long to review and merge it!