Re: [NSE] SSL certificate chain and verification

[David Fifield <david () bamsoftware com>]

Thanks Patrik. Can you add documentation for the new result format in
nselib/nmap.luadoc, and send a new patch?

On Sat, Jun 08, 2013 at 09:24:46PM -0400, Patrik Karlsson wrote:
> On Sat, Jun 8, 2013 at 10:24 AM, Jesper Kückelhahn <dev.kyckel () gmail com>wrote:
> > 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.

Okay, but this is nontrivial and should be done as a separate patch.
Check Ncat's cert_match_dnsname and the tests in
ncat/test/test-wildcard.c. 

> > 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 think this is a bad idea. We already do it for Ncat's certificate
store, which means Ncat's certificate store is always out of date. Don't
hardcode a name like cacert.pem. Instead provide a script argument
allowing someone to use their PEM file in any location, like Ncat's
--ssl-trustfile.

> > On a side note, it seems that the output is a bit off (notice the missing
> > newline at ssl-cert and the indentation level):
>
> Thanks, I saw this as well, but didn't change it, I will if I end up
> committing the patch.

Please don't do that as part of a patch implementing an unrelated
feature. Just commit it straight to the trunk.

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