After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 119090 - Certificate manager implementation
Certificate manager implementation
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: [obsolete] Backend:Mozilla
unspecified
Other All
: Normal enhancement
: Future
Assigned To: Epiphany Maintainers
Marco Pesenti Gritti
Depends on:
Blocks:
 
 
Reported: 2003-08-04 15:47 UTC by Robert Marcano
Modified: 2008-04-01 21:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for epiphany 0.8.1 (83.24 KB, patch)
2003-08-04 15:49 UTC, Robert Marcano
none Details | Review
updated patch against CVS 20030811 (85.37 KB, patch)
2003-08-11 12:57 UTC, Robert Marcano
needs-work Details | Review
Updated patch based on the recomendations, (not yet corrected the x509cert constructor problem) (71.89 KB, patch)
2003-08-18 12:10 UTC, Robert Marcano
none Details | Review
Glade file (17.28 KB, text/xml)
2003-08-18 12:12 UTC, Robert Marcano
  Details
Upgraded patch to CVS HEAD 1.1 (75.22 KB, patch)
2003-09-22 17:19 UTC, Robert Marcano
needs-work Details | Review
Dialog screeshot (15.91 KB, image/png)
2003-10-01 13:17 UTC, Robert Marcano
  Details
Proposed PDM integrated UI (22.39 KB, image/png)
2003-10-27 13:20 UTC, Robert Marcano
  Details
Proposed PDM integrated UI (manage CA button) (12.93 KB, image/png)
2003-10-27 13:21 UTC, Robert Marcano
  Details
Certificates used for testing (all passwords for the files are 'test') (5.01 KB, application/octet-stream)
2003-10-27 13:30 UTC, Robert Marcano
  Details
Updated patch for CVS - 2005-10-29 (81.39 KB, patch)
2005-10-29 13:14 UTC, Crispin Flowerday (not receiving bugmail)
needs-work Details | Review
Another updated patch for CVS - 2005-10-29 (69.67 KB, patch)
2005-10-29 15:36 UTC, Crispin Flowerday (not receiving bugmail)
none Details | Review

Description Robert Marcano 2003-08-04 15:47:49 UTC
Currently this ui only is able to show the list of installed certificates,
but I want some feedback about the design of this solution. you can call
the certificate manager from the edit menu.

I defined two new gtype interfaces EphyEmbedCertsManager and
EphyEmbedX509cert on embed directory. Implemented those interfaces on
embed/mozilla directory and added a new method to the interface
EphyEmbedSingle called get_certs_manager(...)
 
The certificate manager dialog looks like the mozilla xul certificate
manager with a better layout (I think) without the "Other's People"
notebook page (Epiphany is not an email client ;-) )
Comment 1 Robert Marcano 2003-08-04 15:49:13 UTC
Created attachment 18899 [details] [review]
patch for epiphany 0.8.1
Comment 2 Marco Pesenti Gritti 2003-08-04 15:53:41 UTC
Dont laugh at me but ... could you provide a small explanation of what
a cert manager is and for what it's used ?
The code approach looks fine, but I guess we want to deal with
interface design first.
Comment 3 Robert Marcano 2003-08-04 16:06:44 UTC
jajaja. I'm laughing at me because I didn't explain what a certManager
is ;-)

The reponsabilities of the certificate manager are:

Retrieve the certificates from the mozilla psm by type (personal,
server, CA) (this is implemented).
Import / Export certificates (not yet implemented)
Remove certificates (not yet implemented)
Comment 4 Dave Bordoley [Not Reading Bug Mail] 2003-08-04 16:51:12 UTC
Typical questions:

1. What is the use case?
2. Could something like this be done using the extensions mechanism? 

ps. I do think we need to provide a way for extensions to add menu 
items and perhaps even add a tools menu for this (though it wouldn't 
be there if no tools are installed).
Comment 5 Robert Marcano 2003-08-04 17:30:35 UTC
I'm not sure if this enhancement must be an extension/plugin. I think
that SSL certificate management is an integral part of any modern
browser. There is already a bug related to the need to create a
certificate viewer (Bug #115948), and if the certificate managment is
implemented as a plugin, the bug #115948 will reimplement many of the
code already created (when finished) for this bug
Comment 6 Marco Pesenti Gritti 2003-08-04 17:33:39 UTC
Can you elaborate on the use case, giving example maybe ? (Note that
we arent saying it's not useful, but just we dont know what its use it)
Comment 7 Robert Marcano 2003-08-04 17:36:27 UTC
bordoley@msu.edu:

> 1. What is the use case?

I started to work in this patch becuase i needed to use an SSL
certificate that my bank gave to me. So I exported it from mozilla and
found no way to import it to epiphany. Maybe in adition to a
certificate manager like Mozilla we can make epiphany import the
certificate executing the action File->Open (this is just an idea ;-) )
Comment 8 Marco Pesenti Gritti 2003-08-04 17:39:24 UTC
Dave, this looks like necessary evil to me. We cant stop our user to
access their bank services or require them to add an extension for it.
Comment 9 Dave Bordoley [Not Reading Bug Mail] 2003-08-04 17:44:38 UTC
Marco:

Its fine with me if its necessary. I like you just had no use why you
would need this :P. Maybe this should be integrated into the PDM?
Comment 10 Robert Marcano 2003-08-04 18:19:08 UTC
Reasons to integrate with PDM:

- Less dialogs

any other?

Reasons to not integrate with PDM:

- a user trying get more privacy will erase the data stored (cookies,
passwords) can remove the certificates (CA if no personal certificates
are installed) assuming that it is data collected by the browser
- Basic options in the PDM dialog

I'm open to make the changes to integrate it to PDM if people agrees :-)
Comment 11 Marco Pesenti Gritti 2003-08-08 11:00:35 UTC
I'd say to go ahead with separate ui for now. If the code is good
enough I guess we can easily integrate later if necessary.
I'll try to review the patch more closely in the weekend, I'm very
busy with 1.0 bugs :/
Can you update it to cvs btw, I think chpe said it was not applying.
Comment 12 Robert Marcano 2003-08-11 12:57:44 UTC
Created attachment 19107 [details] [review]
updated patch against CVS 20030811
Comment 13 Robert Marcano 2003-08-11 13:02:26 UTC
The updated patch now has the ability to remove certificates, I have
not tested it with imported certificates because i have yet to
implement the import code. The CA certificates provided by Mozilla
apparently can only be removed by root (not a bad idea) but when a non
root user removes it mozilla PSM does not signal an error, but neither
removes it (The same behaviour happens when using plain mozilla)
Comment 14 Robert Marcano 2003-08-11 13:06:33 UTC
On the certificate manager ui (certs-manager.glade), can be seen that
each notebook page has an import button, but the import operation is
not related to any of the pages, it is a global action of the dialog;
my question is: If I place that button as a dialog button (like
"Close" and "Help") will this change be compliant with the GNOME HIG?
Comment 15 Marco Pesenti Gritti 2003-08-13 10:57:06 UTC
Some minor stylistic comments:

- Funcs should be func () not func(). The same for things like
EPHY_EMBED_CERTS_MANAGER(mes->priv->certs_manager);
- Please do not put definitions at the top for all funcs but order
them so that it's not necessary (we make an exception for
init/finalize usually)
- Somewhere aligmenents seem screwed looking at the patch, but maybe
that's just the patch
- ephy_embed_certs_manager_get_authorities_cert_list. I wonder if we
could think to shorter names here, no good ideas atm though.

+	  N_("Manager Certificates"),

Manage ?

G_CONST_RETURN gchararray (*get_title) (EphyEmbedX509cert *cert);

We should ever use gresult in embed stuff as return value.

+ 	if (NS_FAILED(rv))
+ 		return G_FAILED;

These should go on the same line

+ 		// WORKAROUND the implementation of the certDB XPCOM component,
each string
+ 		// returned by FindCertNicknames is a compound string with the
following

/* */ for commments. I'd add a reference to the bug there too.

+ /* This constructor is only intended to be used from C++ code */
+ MozillaEmbedX509cert *
+ mozilla_embed_x509cert_new (nsCOMPtr<nsIX509Cert> &moz_cert)
+ {
+         MozillaEmbedX509cert *cert;
+ 
+         cert = MOZILLA_EMBED_X509CERT (g_object_new
(MOZILLA_EMBED_X509CERT_TYPE, NULL));
+ 	nsAutoString nickname_string;
+ 	moz_cert->GetNickname(nickname_string);
+ 	cert->priv->nickname = ToNewUTF8String(nickname_string);
+ 	cert->priv->moz_cert = moz_cert;
+ 
+         return cert;
+ }

Priv stuff should not be set by _new functions.
I'd initialize nickname from _init and add a gobject property for
moz_cert.
Comment 16 Marco Pesenti Gritti 2003-08-13 10:58:15 UTC
Dave, it would be cool if you could have a look to the glade file and
comment about it (and about the HIG problem).
Chpe, I'd appreciate if you could do a review too.
Comment 17 Christian Persch 2003-08-13 15:53:13 UTC
Only some nite not already mentioned in Marco's review, look good overall:

The certs manager is a modal dialogue, daveb will kill you for that ;)
-- no need to be even window-modal, just make it a singleton, like bm and
history windows. (?)

--- epiphany/embed/ephy-embed-certs-manager.h	2003-08-11
08:23:38.000000000 -0400
+ #define IS_EPHY_EMBED_CERTS_MANAGER(obj)         
(G_TYPE_CHECK_INSTANCE_TYPE ((obj), EPHY_EMBED_CERTS_MANAGER_TYPE))
+ #define IS_EPHY_EMBED_CERTS_MANAGER_CLASS(klass) 
(G_TYPE_CHECK_CLASS_TYPE ((klass), EPHY_EMBED_CERTS_MANAGER_TYPE))

(and in some other places):

I know there are still some files which do it like this, but for new code
use EPHY_IS_... instead of IS_EPHY_...

--- epiphany/embed/mozilla/mozilla-embed-certs-manager.cpp	2003-08-11
08:23:38.000000000 -0400
+ 		list = g_slist_append(list, cert);

Use g_slist_prepend, and if the list order _really_ matters,
g_slist_reverse() it
later:
+ 	*l = g_slist_reverse (list);

--- epiphany/embed/mozilla/mozilla-embed-x509cert.cpp	2003-08-11
08:23:38.000000000 -0400
+         gchararray nickname;
(and everywhere else too):

Use char* like we do everywhere.

Comment 18 Dave Bordoley [Not Reading Bug Mail] 2003-08-13 16:15:34 UTC
can you provide a copy of the glade file so that i don't have to apply
the patch to my tree and i can review the ui. And yes modal anything
is banned. :)
Comment 19 Dave Bordoley [Not Reading Bug Mail] 2003-08-13 16:32:57 UTC
Also I'm a little uncomfortable putting this in the edit menu. I think
it mighte be worth it to bite the bullet and add a tools menu which
could include this and the pdm. 

Also sort of unrelated, can plugins add menu items?
Comment 20 Marco Pesenti Gritti 2003-08-13 16:54:22 UTC
Pleeease no, Tools menu sucks badly and we have already too many
toplevel menus (hard to scan).
Personally I dont any problem with:

--------
Certificates
Personal Data
---------
Toolbars
Preferences

Or maybe it should be available in prefs ?

(plugins can merge menu in theory, they need a patch atm)
Comment 21 Robert Marcano 2003-08-13 20:57:12 UTC
I will work on updating the code to the conventions on a few days.

About the modal dialog thing, i knew that looks ugly, that is is the
reason why i placed this comment in the code ( ;-) )

+ 	/* WORKAROUND the dialog is made modal until is implemented
+ 	   a way to move to front the current open certificate manager */
+ 	ephy_dialog_set_modal(EPHY_DIALOG(dialog), TRUE);
Comment 22 Dave Bordoley [Not Reading Bug Mail] 2003-08-14 02:02:45 UTC
Re: Tools menu
-------------
I see no real problem with the tools menu (especially since this 
seems to be a tool?) I'd rather have more toplevel that are clear 
with fewer items, rather than just throwing stuff into existing menus 
for the sake of fewer toplevels. But thats just my opinion, maybe i 
should shoot off a quick email to calum and seth about this to get 
their input (Actually if someone else could do this that would be 
nice, i'm really busy with moving and stuff for the next three weeks.)

re: Modal
--------------
make it transient to the parent window??
Comment 23 Marco Pesenti Gritti 2003-08-14 10:30:57 UTC
>I see no real problem with the tools menu (especially since this 
>seems to be a tool?)

It's really hard to say what is a tool and what isnt. Bookmarks,
history, preferences, toolbar editor are all tools from my point of
view (confirmed by the position of prefs in IE and firebird). I think
the term is too generic to be a useful categorization.

>I'd rather have more toplevel that are clear 
>with fewer items, rather than just throwing stuff into existing menus 
>for the sake of fewer toplevels.

I just dont see the need of a new toplevel considering that it would
have two items and that Edit menu would have only 11 items, less then
the View menu (the HIG allow a max of 15).
I think it would just make scanning the toplevel menus less efficient
without any real simplification gain in the drop down.

>But thats just my opinion, maybe i 
>should shoot off a quick email to calum and seth about this to get 
>their input (Actually if someone else could do this that would be 
>nice, i'm really busy with moving and stuff for the next three weeks.)

Well it's not urgent ... all post 1.0.
Comment 24 Robert Marcano 2003-08-18 12:10:32 UTC
Created attachment 19303 [details] [review]
Updated patch based on the recomendations, (not yet corrected the x509cert constructor problem)
Comment 25 Robert Marcano 2003-08-18 12:12:21 UTC
Created attachment 19304 [details]
Glade file
Comment 26 Marco Pesenti Gritti 2003-09-01 13:22:15 UTC
Sorry for the spam. Reassigning bugs with a target to our next milestone.
Comment 27 Christian Persch 2003-09-19 13:11:50 UTC
Hi Robert, what's the status on this?

Marco, I think we can take this now and improve it from there, what do
you think?

Also, just FYI so there's no needless duplicate work, crispin is
working on something which overlaps a little, the cert dialogues
[http://patches.theflowerdays.com/cert/].
Comment 28 Robert Marcano 2003-09-19 14:03:06 UTC
I'm migrating the implementation to the CVS HEAD, this needed that i
updated the required libraries (GTK, libxml, etc..). Also I'm learning
how to implement a new XPCOM Service. This new class is needed because
Mozilla NSS prompts for the required password used to locally protect
the certifates imported. I think than I will have a new patch ready at
the end of this weekend
Comment 29 Crispin Flowerday (not receiving bugmail) 2003-09-19 15:25:15 UTC
I think I already have the code for the dialog boxes. Assuming you are
talking about the nsICertificateDialogs service. I am porting that to
Gtk, the only one I have left to do is the ViewCert dialog. I can make
that available if you want to look at it.

I hope to get the ViewCert ported this weekend, so for the moment you
may want to just call the approriate mozilla functions, and when I
have these dialogs finished they should be automatically used.
Comment 30 Robert Marcano 2003-09-19 17:34:54 UTC
Yes... this is the XPCOM interface, are you implementing dialog for
SetPKCS12FilePassword and GetPKCS12FilePassword methods?

There is another dialog that mozilla shows the first time that a
certificate is imported (Master Password, see Mozilla
preferences->Privacy & Security->Master Password->Change Password) and
i think that i need to implement it. My current code crash when
importing certificates, i think that the mozilla default dialog for
this password is causing it
Comment 31 Crispin Flowerday (not receiving bugmail) 2003-09-19 18:15:28 UTC
Indeed, it is those that I am implementing:
http://patches.theflowerdays.com/cert/

Your mozilla is crashing probably because of the bug in mozilla 1.4
(the GetGtkDomWindow() crash or whatever it is called). IIRC the
master password ones use the standard PromptService api which is
already gtk'ified.
Comment 32 Robert Marcano 2003-09-19 19:42:59 UTC
This dialog that you talk about is the standard window.alert() one.
This dialog is called when the user needs to enter the password in
order to have access to a installed certificate when a website request
it. The dialog than I'm talking about is used to set the master
password for the first time (it request the password two times and
verifies that both values are equals, and shows a progress bar with an
indication about the quality of the password) when no certificate has
been imported
Comment 33 Crispin Flowerday (not receiving bugmail) 2003-09-19 19:58:11 UTC
For interested people, my patch for the various alerts is at:

http://patches.theflowerdays.com/diff.cgi/galeon_cert_dialogs.patch

This is very much work in progress, but at least you will be able to
see the dialogs in action :) (the patch is against galeon, but the
code for epiphany is identical).
Comment 34 Robert Marcano 2003-09-22 17:19:46 UTC
Created attachment 20186 [details] [review]
Upgraded patch to CVS HEAD 1.1
Comment 35 Robert Marcano 2003-09-22 17:29:21 UTC
The updated patch is able to import PKCS12 packaged certificates, (is
required to reopen the dialog to see them, for now)

an small line is disabled on ephy-window.c
-	g_object_unref (window->ui_merge);
+	//	g_object_unref (window->ui_merge);


This unref is making epiphany crash when any window or tab is closed
(This a ugly hack in order to be able to test my code ;-) because the
xul master password dialog make epiphany crash)

REQUIREMENTS:
Mozilla 1.5b or 1.4 branch with patch
http://bugzilla.mozilla.org/show_bug.cgi?id=211834
TODO:
Implement a call to the View certificate dialog
Implement Properties dialog.
Any GNOME HIG change required ;-)
NLS
Comment 36 Marco Pesenti Gritti 2003-09-22 23:16:17 UTC
I finally looked to the glade file. What's the difference between
import/export, they apply one globally and one per tab ?

Dave. They are 3 tabs I guess we cant integrate it with the pdm.
Mozilla has it in the prefs dialog but I'm not sure if that make sense
(also why the pdm would be in the menu and this in prefs ...).
Do you think it should be a menu ? I see a bit of a conflict with
current naming of Pdm, isnt this personal data too ?
Basically I think the whole thing will be a pain :/
What about if we write a short document about the usage and the way
this stuff works, it helped in the prefs case. I'm not even sure to
understand it fully myself :/ Then maybe we can get Seth to look at it.
Comment 37 Marco Pesenti Gritti 2003-09-23 09:47:04 UTC
Robert, mozilla doesnt appear to have an export button at all, has it
been introduced recently ?
Comment 38 Robert Marcano 2003-09-23 12:01:06 UTC
Yes, it has one, it is called "backup", I used the "export" name in
order to identify to diferent oposite Import and Export, Import and
Backup dosn't make sene to me.

> I finally looked to the glade file. What's the difference between
> import/export, they apply one globally and one per tab ?

Mozilla Certificate Database XPCOM component has thow import options:
import certifcates based on their type (the tab selected), and it can
import any certificate from an PKCS12 file without testing their
types, this latest is the method that i'm using (at least based on my
experience, i never needed to import select which certificate type i
need to import, I use the export/backup option of the source software
to do that, for example Mozilla Seamonkey Backup certificates button).

The export button will export the selected certificates from the
current selected tab, I think that it is not easy to understand that
something that is not being shown (certificates selected in another
tab) is being exported too. Maybe instead of using three export
buttons, one per tab, i could use only one like import, but the
placement of the import/export buttons may change it this code is
intergrated with the PDM dialog
Comment 39 Robert Marcano 2003-09-23 12:03:37 UTC
o my god, i need to check the spelling before posting messages :$. I
correct the first paragraph:

Yes, it has one, it is called "backup", I used the "export" name in
order to identify two oposite actions: Import and Export, Import and
Backup doesn't make sence to me.
Comment 40 Robert Marcano 2003-10-01 13:17:37 UTC
Created attachment 20406 [details]
Dialog screeshot
Comment 41 Robert Marcano 2003-10-27 13:20:51 UTC
Created attachment 20968 [details]
Proposed PDM integrated UI
Comment 42 Robert Marcano 2003-10-27 13:21:44 UTC
Created attachment 20969 [details]
Proposed PDM integrated UI (manage CA button)
Comment 43 Robert Marcano 2003-10-27 13:30:34 UTC
Created attachment 20970 [details]
Certificates used for testing (all passwords for the files are 'test')
Comment 44 Marco Pesenti Gritti 2003-11-03 15:38:49 UTC
I like the idea of putting CA on a separate level, that makes the
thing a bit more clear IHMO. Though I'm not very convinced about
grouping the two types of certs in a tab, with an optionmenu to choose
them.

Even if I'm convinced we need to put this in epiphany core at some
point, there are so many problems to deal with ... We have identified
many of them, which is an important step, but it's hard to find good
solutions. Maybe it make sense to start implementing it like a plugin,
let it mature and then gradually integrate it in epiphany core. I
think we need to start experimenting with it now that we have
identified user tasks and problems in mozilla design. I'm not
convinced we will get it perfectly right the first time ...

Robert/Dave/spark/chpe, what do you think ? Does a plan like this make
sense ? Alternative proposals ?
Comment 45 Marco Pesenti Gritti 2003-11-03 15:40:31 UTC
Xan should be involved in this decision too.
Comment 46 Robert Marcano 2003-11-03 15:56:28 UTC
I'm working on a document about a proposal to separate the certificate
database management from epiphany to an external module, that can be
shared by any gnome application

The basic idea is to implement a GObjects based API to this
certificate database and implement a new PKCS#11 module for
epiphany/mozilla 

See
http://developer.netscape.com/docs/manuals/security/pkcs/index.htm
ftp://ftp.rsa.com/pub/pkcs/pkcs-11/pkcs11v2.doc

Implementing it as a plugin is not a bad idea either
Comment 47 Marco Pesenti Gritti 2003-11-03 16:02:59 UTC
That sounds like a cool idea. I think I've seen secure certificates ui
for at least evolution and mono. It would make a lot of sense to share
this at desktop level, one more point we gain against mozilla :)
Comment 48 Christian Persch 2003-11-03 17:18:13 UTC
We should start a discussion about that GNOME-wide cert infrastructure
on a ML [d-d-l ?] soon, before those other apps have invested too much
in their own solution.
Comment 49 Dave Bordoley [Not Reading Bug Mail] 2003-11-03 18:50:30 UTC
I don't use certs, nor does anyone i know, so i guess i don't really
have an opinion
Comment 50 Xan Lopez 2003-11-03 21:28:32 UTC
I think giving this a first try as a separated entity is a great idea
too. If we finally merge with ephy core if will serve as a good
testbed, and if it finally makes sense to share with the desktop we'll
be on the right track already.
Comment 51 Christian Persch 2003-11-07 14:58:33 UTC
Now that we've decided on our strategy, I've decided to review the
patch a little more thouroughly, but only the backend side of thing
[i.e. anything that goes in embed/] because this needs to be in
epiphany proper even when the manager UI will be an extension.

From attachment id=20186:
+++ epiphany-new/embed/ephy-embed-certs-manager.c	2003-09-21
14:20:39.000000000 -0400
I think this should be renamed. ephy-certificate-manager.[ch],
EphyCertificateManager, EPHY_CERTIFICATE_MANAGER,
ephy_certificate_manager etc; same for mozilla side of things.

+struct EphyEmbedCertsManagerPrivate
+{
+	gpointer dummy;
+};
Unused, remove.

+gresult
+ephy_embed_certs_manager_get_certificates (EphyEmbedCertsManager
*manager,
+					   GSList **l,
+					   EphyEmbedX509certType type)
I've recently gotten rid of gresult; instead of out-parameters we can
now use normal return values, i.e
GList *
ephy_certificate_manager_get_certificates (EphyCertificateManager
*manager, EphyX509CertificateType type);

and same everywhere else. If you want to indicate failure and no
obvious return value can represent them (e.g. NULL), use gboolean
return, or an GError out-param.

+gresult
+ephy_embed_single_get_certs_manager (EphyEmbedSingle *shell,

..._get_certificate_manager

+++ epiphany-new/embed/ephy-embed-x509cert.c	2003-09-21
23:04:52.000000000 -0400
Rename ephy-x509-certificate.c, EphyX509Certificate,
EPHY_X509_CERTIFICATE, ephy_x509_certificate etc, same for mozilla
side of things.

+struct EphyEmbedX509certPrivate
+{
+	gpointer dummy;
+};
Unused, remove.
+typedef enum
+{
+	PERSONAL_CERTIFICATE,
+	SERVER_CERTIFICATE,
+	CA_CERTIFICATE
+} EphyEmbedX509certType;

Maybe EPHY_CERTIFICATE_PERSONAL etc.
And EphyX509CertificateType.

+struct MozillaEmbedCertsManagerPrivate
+{
+};

Unused, remove; or do you plan to add sth there?

+static void
+mozilla_embed_certs_manager_init (MozillaEmbedCertsManager *manager)
+{
+	manager->priv = g_new0 (MozillaEmbedCertsManagerPrivate, 1);
+}
If you decided to keep ...Private, use GObject instance-private data
here; see any ephy object impl for how it's done.

+
+	g_return_if_fail (object != NULL);
+	g_return_if_fail (MOZILLA_IS_EMBED_CERTS_MANAGER (object));
+
+	manager = MOZILLA_EMBED_CERTS_MANAGER (object);
+	
+	g_return_if_fail (manager->priv != NULL);
Unnecessary, gobject type system guarantees _finalize is only called
for objects of the correct type.

+	g_free (manager->priv);
This is unnecessary both if you removed Private and if you kept
Private with gobject instance-private data.

+enum
+{
+	PROP_MOZILLA_CERT = 1,
+	PROP_END
+};
Normally we use
enum
{
        PROP_0,
        PROP_MOZILLA_CERT
};
etc.

+nsCOMPtr<nsIX509Cert>
+mozilla_embed_x509cert_get_moz_cert  (MozillaEmbedX509cert *cert)
+{
+	return cert->priv->mozilla_cert;
+}
I'm not an XPCOM expert, but the XPCOM tutorial recommends against
this, I think it will leak. Should probably be
already_AddRefed<nsIX509Cert>
mozilla_x506_certificate_get_moz_cert (...

---

What's left to do:
I think there need to be functions to access the GtkNSSDialogs, for
example
ephy_x509_certificate_view (....)
which would show the cert viewer for this certificate.
Comment 52 Robert Marcano 2003-11-18 18:46:49 UTC
I have taken notes of the required changes, I am resuming my work on
this bug now (i had much work these days not near my computer , I need
a laptop '-( ) 

If the idea to create a PKCS#11 module for mozilla to connect it to an
external certificate database works as I think (I hope), no code
changes will be needed to epiphany apart from loading the PKCS#11
module, so all this code (if used) will be outside epiphany
Comment 53 Christian Persch 2003-12-14 10:42:28 UTC
Hi Robert,

have you been able to work on the patch since? Will it be ready in
time for feature freeze on Jan. 12 ?

Thanks
Comment 54 Robert Marcano 2003-12-15 13:19:41 UTC
Sure, I am working on it but currently I don't have much free time
util this friday 19, I hope to have an usable version around
01/12/2004 (I'm returning to work the 01/15/2004)
Comment 55 Christian Persch 2004-10-13 10:55:13 UTC
Mass reassigning of Epiphany bugs to epiphany-maint@b.g.o
Comment 56 Crispin Flowerday (not receiving bugmail) 2005-10-29 13:14:31 UTC
Created attachment 54044 [details] [review]
Updated patch for CVS - 2005-10-29

This is an updated patch to work with CVS head as of today, I'm made some
changes to make the certificate manager an interface on the embed-single
object, partially updated to use embed strings, and converted to use file
chooser.
Comment 57 Crispin Flowerday (not receiving bugmail) 2005-10-29 15:36:14 UTC
Created attachment 54048 [details] [review]
Another updated patch for CVS - 2005-10-29

The backend is now a lot cleaner, and I believe it is free of memory leaks, and
reference counting is more intuitive.

Still to do:
 - Resplit out the cert manager iface from the embed-single, I now don't like
this approach as the iface will grow
 - Design a user interface

The second point is the real blocker, as that will impact what functions need
adding to the interface, and where the frontend code lives.
Comment 58 Christian Persch 2005-12-11 14:39:28 UTC
I committed this to cvs disabled by default (enable with
--enable-certificate-manager configure switch).

Thanks to Robert and Crispin for working on this!