1685589 - perma comm/mailnews/db/gloda/test/unit/test_smime_mimemsg_representation.js | test_smime_mimemsg/< - [test_smime_mimemsg/< : 64] false == true

Status: RESOLVED FIXED

(MailNews Core :: Security: S/MIME, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: mkmelin [at] iki.fi
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=326100307&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/fMBQ4Tn8Q7SZMOcOdnXcvQ/runs/0/artifacts/public/logs/live_backing.log

---

Something from https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=81e34ce5a390aa3e1e7f4c4fd9ee26eea96b8541 broke this```

Comment 1

[Magnus Melin [:mkmelin]]

Something from https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=81e34ce5a390aa3e1e7f4c4fd9ee26eea96b8541

Comment 2

[Intermittent Failures Robot]

17 failures in 587 pushes (0.029 failures/push) were associated with this bug yesterday.

Repository breakdown:

• comm-central: 17

Platform breakdown:

• macosx1014-64: 4
• linux64: 3
• macosx1014-64-shippable: 2
• windows10-64: 3
• linux64-shippable: 1
• windows7-32-shippable: 1
• windows10-64-shippable: 1
• windows7-32: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1685589&startday=2021-01-07&endday=2021-01-07&tree=all

Comment 3

[Magnus Melin [:mkmelin]]

Somehow bug 1607542 broke this test. S/MIME seems to work alright on manual testing though.

Backing out https://hg.mozilla.org/mozilla-central/rev/6b54ed81bf78c32b3360790fba6c58f6095230f0 locally fixes it. I don't see anything obvious. :keeler, any idea?

Comment 4

[Masatoshi Kimura [:emk]]

If a broken secmod.db exists in the profile folder, NSS still fails initializing even though we do not use secmod.db anymore. I think we can't stop renaming secmod.db.

Comment 5

[Masatoshi Kimura [:emk]]

Bug 1607542 stopped deleting key3.db to prevent data loss. But renaming files would not lose data. We should restore the logic to rename secmod.db.

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(In reply to Masatoshi Kimura [:emk] from comment #4)

If a broken secmod.db exists in the profile folder, NSS still fails initializing even though we do not use secmod.db anymore. I think we can't stop renaming secmod.db.

What if we fix NSS instead? There's no reason to read secmod.db if it's not going to be used.

Comment 7

[Intermittent Failures Robot]

41 failures in 2956 pushes (0.014 failures/push) were associated with this bug in the last 7 days.

This is the #18 most frequent failure this week.

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:

• comm-central: 41

Platform breakdown:

• macosx1014-64: 8
• linux64: 7
• macosx1014-64-shippable: 4
• windows10-64: 7
• linux64-shippable: 3
• windows7-32-shippable: 3
• windows10-64-shippable: 3
• windows7-32: 6

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1685589&startday=2021-01-04&endday=2021-01-10&tree=all

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Alternatively, you could probably fix this test by removing mailnews/test/data/db-tinderbox-invalid/secmod.db and references to it in the test (and potentially replacing it with a pkcs11.txt file - not sure if that's necessary).

Comment 9

[Magnus Melin [:mkmelin]]

That fixes the test. But won't it cause problems for users who have this file (for some reason)?

Comment 10

[Magnus Melin [:mkmelin]]

Sent off a try for that: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a2cfa76d3d35da33389bb27646aab2e4b1e47d32

Comment 11

[Intermittent Failures Robot]

24 failures in 515 pushes (0.047 failures/push) were associated with this bug yesterday.

Repository breakdown:

• comm-central: 24

Platform breakdown:

• linux64: 4
• windows7-32: 4
• linux64-shippable: 2
• macosx1014-64-shippable: 2
• windows10-64: 4
• macosx1014-64: 4
• windows10-64-shippable: 2
• windows7-32-shippable: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1685589&startday=2021-01-11&endday=2021-01-11&tree=all

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: bug1685589_smime_testfail.patch

I think we should take this to fix the test.

Then in another bug, secmod.db should again be renamed to something else, or NSS should not fail when it encounters one. These aren't really Thunderbird bugs though

Comment 13

[Masatoshi Kimura [:emk]]

(In reply to Magnus Melin [:mkmelin] from comment #9)

That fixes the test. But won't it cause problems for users who have this file (for some reason)?

Yes, see bug 1337950. That bug introduced this workaround. It is the reason I thought we should keep the workaround. Even if NSS should be fixed, we should keep the workaround until the NSS fix is released, IMO.

Comment 14

[Kai Engert (:KaiE:)]

Comment on attachment 9196574 [details] [diff] [review]
bug1685589_smime_testfail.patch

This change makes sense, because secmod.db isn't used with the key4/cert9 files.

NSS either uses the combination of

• key3.db + cert8.db + secmod.db
  or
• key4.db + cert9.db + pkcs11.txt

Comment 15

[Kai Engert (:KaiE:)]

I see that we had converted the test data had been converted (key3->key4, cert8->cert9) in bug 1604377.
It was a mistake to keep secmod.db at that time.

I think we don't need a pkcs11.txt file, because it's an optional file to record special NSS parameters, or references to smartcards etc.
NSS will create an empty one with default options, if it's missing. That should be fine for our testing.

Comment 16

[Kai Engert (:KaiE:)]

(In reply to Magnus Melin [:mkmelin] from comment #9)

That fixes the test. But won't it cause problems for users who have this file (for some reason)?

I don't fully understand what's going on and what caused the test to fail.
Did the presence of a secmod.db file cause NSS to conclude that an old style database is present, and that key4.db and cert9.db shouldn't be used?

In general, if a user has a secmod.db file, the user is probably migrating from an older version of the Mozilla app.
(Testing with TB 78 I see that secmod.db automatically gets renamed to something else. So if the user is using recent trunk code (moz 86+) and still has a secmod.db file, it seems to mean that the user is directly migrating from older code that didn't yet run through the code to rename secmod.db (renaming with the purpose to have it ignored by NSS init)).

So, if the user still has secmod.db, the user probably also still has key3.db and cert8.db, and IIUC NSS will see those old files, and automatically migrate them to key4/cert9.

I think that as part of such a migration, NSS should also automatically create a new pkcs11.txt file. IIUC, if key4/cert9/pkcs11 files are present, then NSS should no longer read key3/cert8/secmod

So, if the migration goes as expected, and if the user has a profile with old, not-yet-migrated NSS files, it should work. Or, if the user has already used recent versions of FF/TB, then the secmod.db will no longer be present (because it got renamed already).

The scenario that key4/cert9/secmod are the only files present seems unusual, it seems we likely wouldn't see it in practice.

However, when experimenting, I saw unexpected behavior, which I've just reported in bug 1686442. It seems incorrect that the new migration no longer creates a pkcs11.txt file.

Once that bug is addressed, I think we should do a manual test and check what happens, if key4/cert9/secmod are the only files present. I think it is an invalid state, but it should be handled gracefully.

Comment 17

[Masatoshi Kimura [:emk]]

My Firefox profile has all of secmod.db, pkcs11.txt, cert8.db, cert9.db, key3.db, and key4.db. But key4/cert9/pkcs11 are used.

Comment 18

[Magnus Melin [:mkmelin]]

Not sure at "which level" you're asking.
What caused the test to fail is bug 1607542, and AIUI from comment 4, secmod.db is no longer getting (re)moved. And for some reason having an secmod.db is causing NSS to init (related to bug 1337950)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9d6b801efcac
remove secmod.db which is not in the profile anymore (and causes NSS to fail if it's there). r=KaiE

Comment 20

[Magnus Melin [:mkmelin]]

:emk, can you clone a bug about what needs doing to fix the underlying problem? You seem to have the best insight to formulate it.

Comment 21

[Masatoshi Kimura [:emk]]

Firefox already disabled legacy DBM. Bug 1686442 will cover the problem on Thunderbird (apps that do not define NSS_DISABLE_DBM).

Comment 22

[Intermittent Failures Robot]

24 failures in 3810 pushes (0.006 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 24

Platform breakdown:

• linux64: 4
• windows7-32: 4
• linux64-shippable: 2
• macosx1014-64-shippable: 2
• windows10-64: 4
• macosx1014-64: 4
• windows10-64-shippable: 2
• windows7-32-shippable: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1685589&startday=2021-01-11&endday=2021-01-17&tree=all