bug in libp11 error strings handling

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

bug in libp11 error strings handling

Ludovic Rousseau
Hi,

I noticed a bug in libp11. The problem is that PKCS11_CTX_free() (from
p11_load.c) calls
ERR_free_strings() but does not reset the static flag "init" in
ERR_load_PKCS11_strings() (from p11_err.c).

So if you use a new PKCS11_CTX_new(), PKCS11_CTX_load(),
PKCS11_CTX_unload(), PKCS11_CTX_free() cycle the error messages are
not reloaded. You get "error:2A00E101:FIPS
routines:PKCS11_init_pin:User not logged in" the first time and then
"error:2A00E101:lib(42):func(14):reason(257)" for the next round.

I propose the attached patch.

Note that I don't know why the ERR_load_unload_PKCS11_strings()
function should be in libp11.h and not be an internal only function.

Bye,

--
 Dr. Ludovic Rousseau
 For private mail use [hidden email] and not "big brother" Google

_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: bug in libp11 error strings handling

Nils Larsch
Ludovic Rousseau wrote:

> Hi,
>
> I noticed a bug in libp11. The problem is that PKCS11_CTX_free() (from
> p11_load.c) calls
> ERR_free_strings() but does not reset the static flag "init" in
> ERR_load_PKCS11_strings() (from p11_err.c).
>
> So if you use a new PKCS11_CTX_new(), PKCS11_CTX_load(),
> PKCS11_CTX_unload(), PKCS11_CTX_free() cycle the error messages are
> not reloaded. You get "error:2A00E101:FIPS
> routines:PKCS11_init_pin:User not logged in" the first time and then
> "error:2A00E101:lib(42):func(14):reason(257)" for the next round.

agree

> I propose the attached patch.

when I think again about this: is it really a good idea to unload the
error string in PKCS11_CTX_free ? If the application using this
library is using openssl for something else it would free the error
queue for this as well (as the openssl error stuff is thread specific),
hence it might be better if the libp11 user must call ERR_free_strings
on his own ...

Nils
_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: bug in libp11 error strings handling

Ludovic Rousseau
On 10/08/05, Nils Larsch <[hidden email]> wrote:
> > I propose the attached patch.
>
> when I think again about this: is it really a good idea to unload the
> error string in PKCS11_CTX_free ? If the application using this
> library is using openssl for something else it would free the error
> queue for this as well (as the openssl error stuff is thread specific),
> hence it might be better if the libp11 user must call ERR_free_strings
> on his own ...

I agree. Since it is not possible (AFAIK) to unload only the libp11
strings the library should not unload any strings.

libp11 should not free the error queue either since it may contain non
libp11 errors.

So the proposed patch is now:
Index: p11_load.c
===================================================================
--- p11_load.c  (revision 2487)
+++ p11_load.c  (working copy)
@@ -137,8 +137,6 @@ void PKCS11_CTX_unload(PKCS11_CTX * ctx)
  */
 void PKCS11_CTX_free(PKCS11_CTX * ctx)
 {
-       ERR_free_strings();
-       ERR_remove_state(0);
        OPENSSL_free(ctx->manufacturer);
        OPENSSL_free(ctx->description);
        OPENSSL_free(ctx->_private);

Any other objection?

--
 Dr. Ludovic Rousseau
 For private mail use [hidden email] and not "big brother" Google
_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel