Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Hans Leidekker
On Tuesday 21 October 2008 19:06:20 Juan Lang wrote: But you don't check whether those conditions are true, and you march ahead and install the certificate into the root store whether or not they are true. I'm sorry, but the code is just not correct. Please write some test cases. It's a

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Marcus Meissner
On Wed, Oct 22, 2008 at 10:41:00AM +0200, Hans Leidekker wrote: On Tuesday 21 October 2008 19:06:20 Juan Lang wrote: But you don't check whether those conditions are true, and you march ahead and install the certificate into the root store whether or not they are true. I'm sorry, but the

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Hans Leidekker
On Wednesday 22 October 2008 10:47:25 Marcus Meissner wrote: It's a stub of course, so it doesn't always do the right thing. We have many of these in Wine, and that's OK as long as you are warned about the shortcomings. If I'm right about typical usage of this function it will do the

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Juan Lang
If I'm right about typical usage of this function it will do the right thing more often than not, which is pretty good for a stub. I don't think that's typical usage at all: typical usage presents a UI. It's called from elsewhere in cryptui, so it's under the control of the user how

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Hans Leidekker
On Wednesday 22 October 2008 16:37:16 you wrote: I don't think that's typical usage at all: typical usage presents a UI. It's called from elsewhere in cryptui, so it's under the control Sure, but the app may present its own UI like Outlook does, and call this function with CRYPTUI_WIZ_NO_UI

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-22 Thread Juan Lang
Yes, so those users may benefit from the stub as well. And I do print a FIXME. This is nothing new, we've been ignoring invalid certificates in wininet for years where we should stop and show a UI. When I tested with native cryptui and imported a cert, it didn't pick the root store. So I'm

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-21 Thread Hans Leidekker
On Monday 20 October 2008 23:21:44 Juan Lang wrote: You haven't convinced me that Windows does indeed import the certificate to the root store in all cases. Making the root store I don't think I said that. I put a fixme in the code that explicitly warns that the store should be determined

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-21 Thread Juan Lang
You haven't convinced me that Windows does indeed import the certificate to the root store in all cases. Making the root store I don't think I said that. I put a fixme in the code that explicitly warns that the store should be determined dynamically. No, but that's what the code does. What

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-21 Thread Hans Leidekker
On Tuesday 21 October 2008 16:46:51 Juan Lang wrote: I don't think I said that. I put a fixme in the code that explicitly warns that the store should be determined dynamically. No, but that's what the code does. What bothers me is that your implementation is correct in only an extremely

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-21 Thread Juan Lang
Like I said, it's exactly the set of conditions that happens to satisfy Outlook. The typical scenario is that you can't connect to a secure server because of an invalid certificate and then forcibly import the certificate. The invalid certificates I tried on Windows where added to the root

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Juan Lang
Hi Hans, I know this patch already got committed. +BOOL WINAPI CryptUIWizImport(DWORD dwFlags, HWND hwndParent, LPCWSTR pwszWizardTitle, + PCCRYPTUI_WIZ_IMPORT_SRC_INFO pImportSrc, HCERTSTORE hDestCertStore) +{ +static const WCHAR Root[] = {'R','o','o','t',0};

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Hans Leidekker
On Monday 20 October 2008 21:48:37 Juan Lang wrote: +/* FIXME: verify certificate and determine store name dynamically */ +if (!(store = CertOpenStore(CERT_STORE_PROV_SYSTEM_W, 0, 0, CERT_SYSTEM_STORE_CURRENT_USER, Root))) +{ +WARN(unable to open certificate store\n); +

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Juan Lang
It's my limited manual testing with a self-signed root CA certificate that turned this up on Windows. The certificate is still there after Outook is closed. In Windows? Sure. In Wine? I can't see how that would be the case. (In fact it turns up a crash here for me.) --Juan

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Hans Leidekker
On Monday 20 October 2008 22:51:15 Juan Lang wrote: It's my limited manual testing with a self-signed root CA certificate that turned this up on Windows. The certificate is still there after Outook is closed. In Windows? Sure. In Wine? I can't see how that would be the case. (In fact

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Juan Lang
It persists in Windows, yes. Haven't tested Wine, where do you see a crash? In crypt32. I wrote a quick test program that does what your patch does, and it crashes adding the certificate to the root store. I'll send a patch shortly that'll avoid the crash. Nevertheless, this won't do what you

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Hans Leidekker
On Monday 20 October 2008 23:06:35 Juan Lang wrote: It persists in Windows, yes. Haven't tested Wine, where do you see a crash? In crypt32. I wrote a quick test program that does what your patch does, and it crashes adding the certificate to the root store. I'll send a patch shortly

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 Thread Juan Lang
It may not persist but I could import the certificate fine on Wine. Is there an alternative for the root store? What's involved in making the root store read-write? You haven't convinced me that Windows does indeed import the certificate to the root store in all cases. Making the root store