1353916 - Problem with nsIPKCS11ModuleDB.findSlotByName in TB Options->Security

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[:aceman]

When entering Options->Security pane in Thundebird, I get this error:

[Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIPKCS11ModuleDB.findSlotByName]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://messenger/content/preferences/security.js :: _masterPasswordSet :: line 117"  data: no] 1 (unknown)
_masterPasswordSet chrome://messenger/content/preferences/security.js:117:16
_initMasterPasswordUI chrome://messenger/content/preferences/security.js:101:17
init chrome://messenger/content/preferences/security.js:19:5

Could be some change in m-c.

Comment 1

[alta88]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1324071#c12

Comment 2

[:aceman]

Thanks, right. Wanna make the patch?
There are just 2 occurrences.

Comment 3

[Axel Grude [:realRaven]]

I am getting the same error when i try to check the master password status in my Addon QuickPasswords, just before the Manage Passwords window is shown (I put my code into the load event)

  let secmodDB = Components.classes["@mozilla.org/security/pkcs11moduledb;1"].getService(Ci.nsIPKCS11ModuleDB),
      slot = secmodDB.findSlotByName("");
  if (slot) {
    let status = slot.status;
    let hasMP = status != Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED &&
                status != Ci.nsIPKCS11Slot.SLOT_READY;
    if (hasMP) {
      return (slot.status == Ci.nsIPKCS11Slot.SLOT_NOT_LOGGED_IN); // locked!
    }
    else return false;  // no Masterpassword = not locked
  }  

Is there a recommended new way / workaround?

Comment 4

[Axel Grude [:realRaven]]

(In reply to Axel Grude [:realRaven] from comment #3)
> I am getting the same error when i try to check the master password status
> in my Addon QuickPasswords, just before the Manage Passwords window is shown
> (I put my code into the load event)
> 
>   let secmodDB =
> Components.classes["@mozilla.org/security/pkcs11moduledb;1"].getService(Ci.
> nsIPKCS11ModuleDB),
>       slot = secmodDB.findSlotByName("");
>   if (slot) {
>     let status = slot.status;
>     let hasMP = status != Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED &&
>                 status != Ci.nsIPKCS11Slot.SLOT_READY;
>     if (hasMP) {
>       return (slot.status == Ci.nsIPKCS11Slot.SLOT_NOT_LOGGED_IN); // locked!
>     }
>     else return false;  // no Masterpassword = not locked
>   }  
> 
> Is there a recommended new way / workaround?

Ok I coded a backwards compatible workaround based on hasPassword. Now I also need one for finding out whether the master password is set, as this code also throws:

get isMasterPasswordActive() {
  // code from browser/components/preferences/security.js - _masterPasswordSet()
  const Ci = Components.interfaces,
        Cc = Components.classes;
  let secmodDB = Cc["@mozilla.org/security/pkcs11moduledb;1"].getService(Ci.nsIPKCS11ModuleDB),
      slot = secmodDB.findSlotByName("");
  if (slot) {
    let status = slot.status,
        hasMP = status != Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED &&
                status != Ci.nsIPKCS11Slot.SLOT_READY;
    return hasMP;
  } else {
    return false;
  }
}   

How do I code this so it is forward compatible? Is it save to use
try {
  XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper", "resource://gre/modules/LoginHelper.jsm");
  return LoginHelper.isMasterPasswordSet()
}
catch (ex) {;}
... old code ?
or will it break Thunderbird / Postbox / Seamonkey?

Comment 6

[Jorg K (CEST = GMT+2)]

OK, I'll do a patch based on LoginHelper.jsm. We don't need to be backward compatible ;-)

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 1366465-master-password.patch (v1).

This works. Richard, can you please test. The sub-dialogue gets dismissed as it should, no trace of "Cu. not defined" as you mentioned in bug 1366465 comment #4.

Comment 8

[:aceman]

Comment on attachment 8869720 [details] [diff] [review]
1366465-master-password.patch (v1).

Review of attachment 8869720 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks good based on the m-c patch. r+ after paenglab run-tests it:) It seems Seamonkey does not have the offending findSlotByName() call.

::: mail/components/preferences/security.js
@@ +4,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper", "resource://gre/modules/LoginHelper.jsm");
> + 

Surplus space.

@@ +111,5 @@
>  
>    /**
>     * Returns true if the user has a master password set and false otherwise.
>     */
>    _masterPasswordSet: function ()

Do we now need this function?

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1366465-master-password.patch (v2).

OK, ripped out more.

Comment 10

[:aceman]

Comment on attachment 8869722 [details] [diff] [review]
1366465-master-password.patch (v2).

Review of attachment 8869722 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

::: im/content/preferences/privacy.js
@@ -90,5 @@
> -      var hasMP = status != Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED &&
> -                  status != Ci.nsIPKCS11Slot.SLOT_READY;
> -      return hasMP;
> -    } else {
> -      // XXX I have no bloody idea what this means

Nice that we can remove this :-P

Comment 11

[Richard Marti (:Paenglab)]

Comment on attachment 8869722 [details] [diff] [review]
1366465-master-password.patch (v2).

Works, also closing the subdialog. Thank you Jörg.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1366465-master-password.patch (v3).

Well, the "Cu. not defined" error isn't there any more because there is no error. But if there were an error, we'd get this useless message instead of the error, so let's fix this now as well.

Sorry, 3rd review for this trivial thing.

Comment 13

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/4ae874d44245158a802e05253b0d353b1fd98164

Comment 14

[Jorg K (CEST = GMT+2)]

Comment on attachment 8869739 [details] [diff] [review]
1366465-master-password.patch (v3).

Looks like this needs uplift since it's reported for TB 54 beta in bug 1366465.

Comment 15

[Jorg K (CEST = GMT+2)]

Beta (TB 54):
https://hg.mozilla.org/releases/comm-beta/rev/8d61b7ad73ef1d1c4fbe9266357ef2891c53441f