Closed Bug 1353916 Opened 4 years ago Closed 4 years ago

Problem with nsIPKCS11ModuleDB.findSlotByName in TB Options->Security

Categories

(Thunderbird :: Preferences, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(thunderbird54 fixed, thunderbird55 fixed)

RESOLVED FIXED
Thunderbird 55.0
Tracking Status
thunderbird54 --- fixed
thunderbird55 --- fixed

People

(Reporter: aceman, Assigned: jorgk-bmo)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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.
Thanks, right. Wanna make the patch?
There are just 2 occurrences.
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?
(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?
Blocks: 1360944
Duplicate of this bug: 1366465
OK, I'll do a patch based on LoginHelper.jsm. We don't need to be backward compatible ;-)
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Flags: needinfo?(richard.marti)
Attachment #8869720 - Flags: review?(acelists)
Attachment #8869720 - Flags: feedback?(richard.marti)
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?
Attachment #8869720 - Flags: review?(acelists) → review+
OK, ripped out more.
Attachment #8869720 - Attachment is obsolete: true
Attachment #8869720 - Flags: feedback?(richard.marti)
Attachment #8869722 - Flags: review?(acelists)
Attachment #8869722 - Flags: feedback?(richard.marti)
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
Attachment #8869722 - Flags: review?(acelists) → review+
Comment on attachment 8869722 [details] [diff] [review]
1366465-master-password.patch (v2).

Works, also closing the subdialog. Thank you Jörg.
Flags: needinfo?(richard.marti)
Attachment #8869722 - Flags: feedback?(richard.marti) → feedback+
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.
Attachment #8869722 - Attachment is obsolete: true
Attachment #8869739 - Flags: review?(acelists)
Attachment #8869739 - Flags: review?(acelists) → review+
https://hg.mozilla.org/comm-central/rev/4ae874d44245158a802e05253b0d353b1fd98164
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
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.
Attachment #8869739 - Flags: approval-comm-beta?
Attachment #8869739 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.