Re: [Bug-wget] Windows cert store support
On Fri, Dec 11, 2015 at 01:22:48PM +0200, Eli Zaretskii wrote: > > Date: Thu, 10 Dec 2015 01:12:37 +0100 > > From: Ángel González > > Cc: bug-wget > > > > On 09/12/15 03:06, Random Coder wrote: > > > I'm not sure if the wget maintainers would be interested, but I've > > > been carrying this patch around in my private builds of wget for a > > > while. It allows wget to load SSL certs from the default Windows cert > > > store. > > > > > > The patch itself is fairly straightforward, but as it changes the > > > default SSL behavior, and no care was taken to follow coding convents > > > when I wrote it, so it's probably not ready for inclusion in the > > > codebase. Still, if it's useful, feel free to use it for ideas. > > Wow, supporting the OS store would certainly be very cool. > > > > I would probably move it to windows.c and attempt to make it also work > > in gnutls, but in general it looks good. > > Wget compiled with GnuTLS already supports this feature: it calls > gnutls_certificate_set_x509_system_trust when the GnuTLS library > supports that. gnutls_certificate_set_x509_system_trust does > internally what the proposed patch does. > > So I think this code should indeed go only to openssl.c, as gnutls.c > already has its equivalent. > AFAIK OpenSSL source contains crypto engine that delegates all operations to Windows native cryptographical subsystem. It's only matter of default configuration. -- Petr signature.asc Description: PGP signature
Re: [Bug-wget] Windows cert store support
> Date: Thu, 10 Dec 2015 01:12:37 +0100
> From: Ángel González
> Cc: bug-wget
>
> On 09/12/15 03:06, Random Coder wrote:
> > I'm not sure if the wget maintainers would be interested, but I've
> > been carrying this patch around in my private builds of wget for a
> > while. It allows wget to load SSL certs from the default Windows cert
> > store.
> >
> > The patch itself is fairly straightforward, but as it changes the
> > default SSL behavior, and no care was taken to follow coding convents
> > when I wrote it, so it's probably not ready for inclusion in the
> > codebase. Still, if it's useful, feel free to use it for ideas.
> Wow, supporting the OS store would certainly be very cool.
>
> I would probably move it to windows.c and attempt to make it also work
> in gnutls, but in general it looks good.
Wget compiled with GnuTLS already supports this feature: it calls
gnutls_certificate_set_x509_system_trust when the GnuTLS library
supports that. gnutls_certificate_set_x509_system_trust does
internally what the proposed patch does.
So I think this code should indeed go only to openssl.c, as gnutls.c
already has its equivalent.
One other comment I have about the patch is that it's inconsistent
with what gnutls.c does:
if (!opt.ca_directory)
ncerts = gnutls_certificate_set_x509_system_trust (credentials);
/* If GnuTLS version is too old or CA loading failed, fallback to old
behaviour.
* Also use old behaviour if the CA directory is user-provided. */
if (ncerts <= 0)
{
IOW, condition the attempt to load the system certs on
opt.ca_directory, and fall back to the certs from files if that fails.
Thanks.
Re: [Bug-wget] Windows cert store support
On Thu, Dec 10, 2015 at 2:13 AM, Gisle Vanem wrote:
> it would be nice to know if it succeeded because of WinCrypt or
> OpenSSL.
It succeeded because of both. WinCrypt to load the cert, and OpenSSL
to verify it. With my patch, you can't actually provide certs from
both an OpenSSL store and a Windows store. I suppose I could add some
optional information message when WinCrypt is used. Is there
precedent for such a message?
> How does this prevent an expired Cert to be used?
> I see in the 'CERT_INFO' structure a 'NotAfter' member. But this
> struct seems to support for WINAPI_PARTITION_APP only :-(
> I assume this could be used to check expired certificates.
The certificate itself contains that information encoded in the
pbCertEncoded data blob. As a quick verification/example, I added the
following bit of code to the loop in my patch that loads the certs.
/* Before the loop */
int pickACert = 0;
/* ... */
/* after the d2i_X509 call */
if (pickACert++ == 42) {
char* certAsString = X509_to_PEM(cert);
FILE* f=fopen("test.x509.pem","wb");
fwrite(certAsString,strlen(certAsString),1,f);
fclose(f);
}
(I used the X509_to_PEM helper function from this StackOverflow
answer: http://stackoverflow.com/a/23137774 )
That code simply takes the x509 certificate after OpenSSL has parsed
it, and writes it out into a file.
Then, opening the cert in openssl using this command to view it in a
human readable format:
openssl x509 -in test.x509.pem -text -noout
Along with the rest of the information in the output is this little
tidbit showing the random cert I picked is expired and OpenSSL should
ignore it:
Validity
Not Before: Apr 9 00:00:00 1996 GMT
Not After : Jan 7 23:59:59 2004 GMT
Re: [Bug-wget] Windows cert store support
Random Coder wrote:
> I'm not sure if the wget maintainers would be interested, but I've
> been carrying this patch around in my private builds of wget for a
> while. It allows wget to load SSL certs from the default Windows cert
> store.
I've applied your patch. It seems to work fine. Nice!
But in a message like:
X509 certificate successfully verified and matches host
www.ssllabs.com
it would be nice to know if it succeeded because of WinCrypt or
OpenSSL.
> + /* Loop through all the certs in the Windows cert store */
> + for ( pCertCtx = Local_CertEnumCertificatesInStore(hStore, NULL);
> + pCertCtx != NULL;
> + pCertCtx = Local_CertEnumCertificatesInStore(hStore, pCertCtx) )
> + {
> +if (!((pCertCtx->dwCertEncodingType & PKCS_7_ASN_ENCODING) ==
> PKCS_7_ASN_ENCODING))
> +{
> + /* Add all certs we find to OpenSSL's store */
How does this prevent an expired Cert to be used?
I see in the 'CERT_INFO' structure a 'NotAfter' member. But this
struct seems to support for WINAPI_PARTITION_APP only :-(
I assume this could be used to check expired certificates.
--
--gv
Re: [Bug-wget] Windows cert store support
On Wed, Dec 9, 2015 at 4:12 PM, Ángel González wrote: > I would probably move it to windows.c and attempt to make it also work in > gnutls, but in general it looks good. Fair enough. I'll fix up the patch in the coming weeks. If anyone else wants a stab before me, feel free!
Re: [Bug-wget] Windows cert store support
On 09/12/15 03:06, Random Coder wrote: I'm not sure if the wget maintainers would be interested, but I've been carrying this patch around in my private builds of wget for a while. It allows wget to load SSL certs from the default Windows cert store. The patch itself is fairly straightforward, but as it changes the default SSL behavior, and no care was taken to follow coding convents when I wrote it, so it's probably not ready for inclusion in the codebase. Still, if it's useful, feel free to use it for ideas. Wow, supporting the OS store would certainly be very cool. I would probably move it to windows.c and attempt to make it also work in gnutls, but in general it looks good. Thanks!
[Bug-wget] Windows cert store support
I'm not sure if the wget maintainers would be interested, but I've been carrying this patch around in my private builds of wget for a while. It allows wget to load SSL certs from the default Windows cert store. The patch itself is fairly straightforward, but as it changes the default SSL behavior, and no care was taken to follow coding convents when I wrote it, so it's probably not ready for inclusion in the codebase. Still, if it's useful, feel free to use it for ideas. openssl.patch Description: Binary data
