New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSOn output is broken by the JSON footer in combination with mass mode #925

Merged
merged 2 commits into from Dec 7, 2017

Conversation

Projects
None yet
2 participants
@seccubus
Contributor

seccubus commented Dec 6, 2017

The comma was missing in front of the last finding that is inserted as part of the JSOn footer. THis corrects it and once merged my unit tests should work again.

@drwetter

This comment has been minimized.

Show comment
Hide comment
@drwetter

drwetter Dec 6, 2017

Owner

Hi Frank,

thx!

Not sure which bug you're talking about (needed to guess a bit). Did you mean to address this one?

./testssl.sh -p --jsonfile=tmp --parallel --file mass

If yes, that is still present:

tail -10 tmp
             "finding"      : "HTTP2/ALPN : not offered"
          }
         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]

If not: The cmd line for the bug you're trying to address would be great.

Cheers, Dirk

Owner

drwetter commented Dec 6, 2017

Hi Frank,

thx!

Not sure which bug you're talking about (needed to guess a bit). Did you mean to address this one?

./testssl.sh -p --jsonfile=tmp --parallel --file mass

If yes, that is still present:

tail -10 tmp
             "finding"      : "HTTP2/ALPN : not offered"
          }
         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]

If not: The cmd line for the bug you're trying to address would be great.

Cheers, Dirk

@seccubus

This comment has been minimized.

Show comment
Hide comment
@seccubus

seccubus Dec 6, 2017

Contributor

I see I did not fix it right. Let me look again. Battery is nearly empty now.

Contributor

seccubus commented Dec 6, 2017

I see I did not fix it right. Let me look again. Battery is nearly empty now.

@seccubus

This comment has been minimized.

Show comment
Hide comment
@seccubus

seccubus Dec 6, 2017

Contributor

I've checked again and it actual working as expected for me.

My command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851

My output:

tail -12 /tmp/seccubus.testssl.666.json
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
,         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
Contributor

seccubus commented Dec 6, 2017

I've checked again and it actual working as expected for me.

My command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851

My output:

tail -12 /tmp/seccubus.testssl.666.json
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
,         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
@seccubus

This comment has been minimized.

Show comment
Hide comment
@seccubus

seccubus Dec 6, 2017

Contributor

With the --parallel option the problem remains.

Command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851 --parallel

Output:

              "ip"           : "seccubus.com/178.237.34.227",
              "port"         : "443",
              "severity"     : "OK",
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
Contributor

seccubus commented Dec 6, 2017

With the --parallel option the problem remains.

Command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851 --parallel

Output:

              "ip"           : "seccubus.com/178.237.34.227",
              "port"         : "443",
              "severity"     : "OK",
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
@seccubus

This comment has been minimized.

Show comment
Hide comment
@seccubus

seccubus Dec 6, 2017

Contributor

This seems to fix it for --parallel too.

Command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851 --parallel

Output:

              "ip"           : "seccubus.com/178.237.34.227",
              "port"         : "443",
              "severity"     : "OK",
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
,         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
Contributor

seccubus commented Dec 6, 2017

This seems to fix it for --parallel too.

Command line:

./testssl.sh  --color 0 -p -S -P -h -U --fast --jsonfile=/tmp/seccubus.testssl.666.json --file /tmp/seccubus.hosts.98851 --parallel

Output:

              "ip"           : "seccubus.com/178.237.34.227",
              "port"         : "443",
              "severity"     : "OK",
              "cve"          : "CVE-2013-2566, CVE-2015-2808",
              "cwe"          : "CWE-310",
              "finding"      : "RC4: not vulnerable"
          }
,         {
              "id"           : "scanTime",
              "ip"           : "/",
              "port"         : "443",
              "severity"     : "INFO",
              "finding"      : "0"
          }
]
@drwetter

This comment has been minimized.

Show comment
Hide comment
@drwetter

drwetter Dec 7, 2017

Owner

Ok, thanks!

Let me have a look at the root cause. I added a while back the last section to track the performance better. I plan another section with version info. Your fix should still work then.

Owner

drwetter commented Dec 7, 2017

Ok, thanks!

Let me have a look at the root cause. I added a while back the last section to track the performance better. I plan another section with version info. Your fix should still work then.

@seccubus

This comment has been minimized.

Show comment
Hide comment
@seccubus

seccubus Dec 7, 2017

Contributor

Root cause is that in mass mode, the main script does did not produce any findings itself, so the comma would be skipped when it added it's first finding.
On line 14537 I'm setting FIRST_FINDING to false if we have imported results from another scan and that should work fine.
You did a similar thing on line 14559 with the FIRST_JSON_OUTPUT variable, but did not set FIRST_FINDING to false as well which I now did at line 14560.

Please, please, please, can you merge quickly as our production setup is broken now too?

Contributor

seccubus commented Dec 7, 2017

Root cause is that in mass mode, the main script does did not produce any findings itself, so the comma would be skipped when it added it's first finding.
On line 14537 I'm setting FIRST_FINDING to false if we have imported results from another scan and that should work fine.
You did a similar thing on line 14559 with the FIRST_JSON_OUTPUT variable, but did not set FIRST_FINDING to false as well which I now did at line 14560.

Please, please, please, can you merge quickly as our production setup is broken now too?

@drwetter drwetter merged commit 7e62dc3 into drwetter:2.9dev Dec 7, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@drwetter

This comment has been minimized.

Show comment
Hide comment
@drwetter

drwetter Dec 7, 2017

Owner

for the time being at least, sure.

Root cause was an earlier commit where I added this object :) It worked in modes where outputs went to different files.

Owner

drwetter commented Dec 7, 2017

for the time being at least, sure.

Root cause was an earlier commit where I added this object :) It worked in modes where outputs went to different files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment