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-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
> 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 frequent

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

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,

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-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 roo

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 extre

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.

Re: cryptui: Add a partial implementation of CryptUIWizImport.

2008-10-20 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 dy

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

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 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 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. >

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 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
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}; (sni