[email protected] wrote:
> Full_Name: Jan Vcelak
> Version: git master
> OS: Linux
> URL:
> ftp://ftp.openldap.org/incoming/jvcelak-20120820-nss-prefer-unlocked-slot-private-key.patch
> Submission from: (NULL) (94.113.220.43)
>
>
> With last MozNSS patches for OpenLDAP, the library explicitly opens the
> certificate database when retrieving the certificates, even if the database is
> already opened. (Requried for safe certificate lookup from a nickname.) This
> might also require a re-authentication to a slot, which holds the private key.
>
> Some application might expect that the slot with private key is already
> unlocked
> before passing the control to libldap. This got broken with the recent
> changes.
The quality of this code seems to be getting progressively worse. It seems
I've been accepting the last several patches without giving feedback though.
1) the recent patches do not adhere to the existing whitespace conventions.
Please fix this.
2) the code in this patch is unnecessarily clumsy:
+static SECKEYPrivateKey *
+tlsm_find_unlocked_key(tlsm_ctx *ctx, void *pin_arg)
+{
+ SECKEYPrivateKey *result = NULL;
+
+ PK11SlotList *slots = PK11_GetAllSlotsForCert(ctx->tc_certificate, NULL);
+ if (!slots) {
+ PRErrorCode errcode = PR_GetError();
+ Debug(LDAP_DEBUG_ANY,
+ "TLS: cannot get all slots for certificate '%s' (error %d: %s)",
+ tlsm_ctx_subject_name(ctx), errcode,
+ PR_ErrorToString(errcode, PR_LANGUAGE_I_DEFAULT));
+ return result;
+ }
+
+ PK11SlotListElement *le;
+ for (le = slots->head; le && !result; le = le->next) {
+ PK11SlotInfo *slot = le->slot;
+ if (!PK11_IsLoggedIn(slot, NULL))
+ continue;
+
+ result = PK11_FindKeyByDERCert(slot, ctx->tc_certificate, pin_arg);
+ }
+
+ PK11_FreeSlotList(slots);
+ return result;
+}
This should just be:
+ for (le = slots->head; le; le = le->next) {
+ PK11SlotInfo *slot = le->slot;
+ if (PK11_IsLoggedIn(slot, NULL)) {
+ result = PK11_FindKeyByDERCert(slot, ctx->tc_certificate,
pin_arg);
+ break;
+ }
+ }
> I'm attaching a patch which fixes it. If the certificate (and corresponding
> key)
> is held in multiple slots, libldap will take the key from an already
> authenticated slot.
>
>
> The attached file is derived from OpenLDAP Software. All of the modifications
> to
> OpenLDAP Software represented in the following patch(es) were developed by Red
> Hat. Red Hat has not assigned rights and/or interest in this work to any
> party.
> I, Jan Vcelak am authorized by Red Hat, my employer, to release this work
> under
> the following terms.
>
> Red Hat hereby place the following modifications to OpenLDAP Software (and
> only
> these modifications) into the public domain. Hence, these modifications may be
> freely used and/or redistributed for any purpose with or without attribution
> and/or other notice.
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
