rbsec / sslscan

sslscan tests SSL/TLS enabled services to discover supported cipher suites
GNU General Public License v3.0
2.29k stars 381 forks source link

Can't detect multiple certificates #208

Open agrandville opened 4 years ago

agrandville commented 4 years ago

For performance improvement some sites let client choose which signature algorithm they want to use (mainly RSA or ECDSA).

eg: www.google.com

openssl s_client -sigalgs RSA+SHA256 www.google.com:443 | openssl x509 -noout -text
....
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (2048 bit)
....

openssl s_client -sigalgs ECDSA+SHA256 www.google.com:443 | openssl x509 -noout -text
...
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
...

wouldn't it more accurate to show all certificates when --show-certificate(s) is requested ?

rbsec commented 4 years ago

Hi @agrandville,

This is an interesting idea - I've seen a few sites that have multiple certificates like this, so it would be nice to have some detection for that. However, we need to think about how we can display that (the certificate display is already very verbose, and returning multiple certificates is a breaking change to the XML).

In terms of scanning, I guess this would have to sit in the loop where we detect which signature algorithms the server supports, although this would be made more complicated by servers that claim to support arbitrary algorithms. So maybe it would have to be in a separate loop...

@jtesta what are your thoughts on this?

jtesta commented 4 years ago

I think the user should be given maximum information by default. Then if they feel like they'd rather have less, they can use the right args to cut the output down. If we need to update the XML schema, then so be it.

In terms of an implementation, perhaps we could do the signature algorithm enumeration first, build a list that's been accepted, then use it to request certificates. So if the server accepts RSA and ECDSA sig algs, later request one RSA certificate and one ECDSA certificate (just like @agrandville is doing with the openssl command line tool).

rbsec commented 4 years ago

@jtesta one potential issue with using a list of accepted algorithms is that if the server accepts the bogus one, then not further testing is carried out for them - so we'd either always have to run the full test, or have a default list to try in that case.

If the XML change is going to happen then that should be before version 2.0 gets released properly, and hopefully it won't break too much stuff.

jtesta commented 4 years ago

The XML schema can be changed quickly before we add the certificate enumeration code. In the short term, it'll only report one certificate like before, but force users to assume there may be more.

For example, add a "" block with a single "<certificate subject='blah' [...]>" entry in it (or however it works at the moment...).

-- Joseph S. Testa II Founder & Principal Security Consultant Positron Security

rbsec commented 4 years ago

That's a fair point.

Actually, looking at the current code, you can already get multiple <certificate> blocks - you get one by default, and a second one with more details if you used --show-certificate. I wonder how many parsers that breaks...

That should probably be cleaned up, and the showCertificate() and checkCertificate() functions merged together into one.

jtesta commented 4 years ago

One option is to add a comment in the XML to warn users that multiple certs are possible. Like:

It won't help with existing implementations until a human looks at the XML, but they might be forced to do that with the changes in v2.0 anyway...

rbsec commented 4 years ago

To be honest, it's not a huge deal if we change the XML a bit for 2.0 (especially given it's a bit dodgy at the moment).

I'll look at adding the <certificates> element and merging the two check functions for now, and then we can look at actually doing the multiple certificate checks down the line.

joaociocca commented 1 year ago

is this what's causing this malformation of the XML file?

image

I noticed it while trying to work sslscan's XML file with xq and getting an xq: Error running jq: ExpatError: mismatched tag: line 13, column 4.

joaociocca commented 1 year ago

ok, I figured out a way to remove those empty </certificate> thingies, sharing here in case someone stumbles on this too:

sed -ibkp ':a;N;$!ba;s#\n <certificates>\(\n \+</certificate>\)\+\n </certificates>##g'  <file>

example: image