Re: [NSE] SSL certificate chain and verification

[Patrik Karlsson <patrik () cqure net>]

Jesper,

Thanks for testing and your feedback!
My comments are inline;


On Sat, Jun 8, 2013 at 10:24 AM, Jesper Kückelhahn <dev.kyckel () gmail com>wrote:

> Hi Patrik,
>
> I've been testing the patch a bit, and it seems like a very nice feature
> to  add to nmap. I've listed some of the things I've noticed during my
> testing.
>
> 1. Would it make sense to implement the warning as table instead ? This
> way there could be more than one warning, such as "self signed",
>  "expired",  etc.

As far as I understand the process, described to some extent here [1],
verify will abort as soon as an error is found.
As such, there should be no more than one error returned on failure.


> 2. Currently the script returns "Certificate Warning: unable to get local
> issuer certificate" warning for certificates that have expired. Maybe this
> could be changed to "Certificate Warning: Certificate has expired" ?

I verified that I get this error only when openssl can't verify the issuer
certificate as this check is performed before the expiry.
Once the issue can be verified the certificate expired error should be
issued.


> 3. Would it be possible to check if the subject CommonName matches either
> the supplied domain name (assuming that the supplied host is not an IP
> address) or the reverse lookup ?

I will look in to this, it was on my TODO list if the patch caught any
attention.



> 4. If I'm understanding the verification correctly, it uses the local
> installation of OpenSSL to verify the certificate. If this is the case,
> then couldn't it be that the results of the verification might vary across
> different systems, depending on the version of OpenSSL and local list of
> trusted certificates ? Would it then make sense to include such a list of
> trusted certificates to nmap, so the results are consistent ? Or am I
> missing something in my understanding of OpenSSL's verification process ?

Correct. Although I'm unsure whether it would be a good idea or not to
bundle a cacert.pem file with Nmap.
The attached patch adds (in addition to the previous stuff) the ability to
add a custom cacert.pem to the nselib/data directory.
It will only use this file if it is present, otherwise it will use the
system one.
I was able to test the expiry vs. incorrect issuer error using this by
downloading the cacert.pem from the curl project [2].



> On a side note, it seems that the output is a bit off (notice the missing
> newline at ssl-cert and the indentation level):
> "
> | ssl-cert: Certificate Warning: self signed certificate
> | Subject: commonName=xxx
> | Issuer: commonName=xxx
> | Public Key type: rsa
> | Public Key bits: 1024
> | Not valid before: 2008-03-19T10:37:24+00:00
> | Not valid after:  2018-03-17T10:37:24+00:00
> "
> I checked the current svn revision, and it seems to generate the same
> output, so I don't think it's related to your patch, but I thought I'd
> mention it. It seems this is caused by the string output of the script,
> since removing ", output_str(cert)" from the return statement in "action"
> corrects this issue.

Thanks, I saw this as well, but didn't change it, I will if I end up
committing the patch.


> Regards,
>   Jesper
Thanks,
Patrik

[1] http://www.openssl.org/docs/apps/verify.html
[2] http://curl.haxx.se/ca/cacert.pem

-- 
Patrik Karlsson
http://www.cqure.net
http://twitter.com/nevdull77
http://www.linkedin.com/in/nevdull77

Attachment: ssl-cert-chain2.patch
Description:

_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/