Project

General

Profile

Bug #15771

OpenPGP key fetch tests started failing

Added by intrigeri 10 months ago. Updated 10 months ago.

Status:
Resolved
Priority:
Elevated
Assignee:
-
Category:
Test suite
Target version:
Start date:
08/08/2018
Due date:
% Done:

100%

Estimated time:
0.25 h
Feature Branch:
test/15771-use-reliable-gnupg-key+force-all-tests
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

… I believe that's because they use anonym's key, which expired recently.


Related issues

Blocks Tails - Feature #13241: Core work: Test suite maintenance Rejected 06/29/2017

Associated revisions

Revision 68fa3d9d (diff)
Added by intrigeri 10 months ago

Test suite: use hopefully more reliable public GnuPG key (refs: #15771)

So far we were using anonym's key, on the ground that he would maintain
it and worst case, if he fails to keep it up-to-date, notice if automated tests
start failing. This did not work: his key has expired a couple days ago.

So let's switch to dkg's key and hope he'll maintain it better :)

While I'm at it, let's use the full fingerprint instead of a short ID when
feasible (dkg actually has published another key with the same short ID, most
likely for demonstration purposes). But Seahorse does not support fetching keys
by fingerprint so the corresponding steps keep using the short ID and I'm
updating the expected picture to match the non-revoked one of these two keys.

Revision cd2932ec (diff)
Added by intrigeri 10 months ago

Test suite: make the "Syncing OpenPGP keys using Seahorse" tests more robust wrt. new subkeys and other kinds of self-certification (refs: #15771).

Otherwise it will break next time dkg adds a subkey to his key.

Revision 2011c503 (diff)
Added by intrigeri 10 months ago

Test suite: update another hard-coded GnuPG signature count (refs: #15771).

dkg's key currently has 10 self-signatures so without this change,
this check would always pass even though Seahorse has not synchronized
the keys yet, and as a result that step would sometimes erroneously pass:

I synchronize keys in Seahorse

… which in turn sometimes causes the next one to fail:

the key "0EE5BE979282D80B9F7540F1CCD2ED94D21739E9" has more than 42 signatures

… because it was run too early.

Revision 1a6f57b1
Added by intrigeri 10 months ago

Merge branch 'test/15771-use-reliable-gnupg-key+force-all-tests' into devel (Fix-committed: #15771)

History

#1 Updated by intrigeri 10 months ago

#2 Updated by intrigeri 10 months ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10
  • Feature Branch set to test/15771-use-reliable-gnupg-key+force-all-tests

#3 Updated by intrigeri 10 months ago

  • Assignee changed from intrigeri to segfault
  • % Done changed from 10 to 50
  • Estimated time set to 0.25 h
  • QA Check set to Ready for QA

torified_gnupg.feature passes again. Could you do a quick code review to ensure I did not do anything stupid or dangerous?

#4 Updated by segfault 10 months ago

  • Assignee changed from segfault to intrigeri
  • QA Check changed from Ready for QA to Dev Needed

Code review passes, except for one thing:

IIUC, the two tests containing `But the key "0EE5BE979282D80B9F7540F1CCD2ED94D21739E9" has only 10 signatures` will not run if the key does not have exactly 10 self-signatures. While this is currently the case, it could (and probably will) change in the future. This was also the case before your change, but then at least the key was under our (anonym's) control.

Possible solution: Add a "less then" parameter, and use it with a high enough number (50?) that won't be reached by accident. I suppose we can trust dkg to not make our tests buggy on purpose by adding 50 self-signatures to his key :)

#5 Updated by intrigeri 10 months ago

IIUC, the two tests containing `But the key "0EE5BE979282D80B9F7540F1CCD2ED94D21739E9" has only 10 signatures` will not run if the key does not have exactly 10 self-signatures. While this is currently the case, it could (and probably will) change in the future.

Indeed, good catch.

This was also the case before your change, but then at least the key was under our (anonym's) control.

… which did not help much, otherwise I would not have had to file this ticket in the first place.

Possible solution: Add a "less then" parameter, and use it with a high enough number (50?) that won't be reached by accident. I suppose we can trust dkg to not make our tests buggy on purpose by adding 50 self-signatures to his key :)

Good idea. I've implemented this, now waiting for our (very overloaded today) Jenkins CI to validate it. I'm quite confident it will work (famous last words) so you might want to review it already: commit:8f3f506446039ac9e4ed2c7083d1e36acd64cd50.

#6 Updated by intrigeri 10 months ago

  • Assignee changed from intrigeri to segfault
  • QA Check changed from Dev Needed to Ready for QA

#7 Updated by segfault 10 months ago

  • Assignee changed from segfault to intrigeri
  • QA Check changed from Ready for QA to Pass

intrigeri wrote:

This was also the case before your change, but then at least the key was under our (anonym's) control.

… which did not help much, otherwise I would not have had to file this ticket in the first place.

It doesn't seem like that was the problem, the key still has 2 self-signed signatures.

Possible solution: Add a "less then" parameter, and use it with a high enough number (50?) that won't be reached by accident. I suppose we can trust dkg to not make our tests buggy on purpose by adding 50 self-signatures to his key :)

Good idea. I've implemented this, now waiting for our (very overloaded today) Jenkins CI to validate it. I'm quite confident it will work (famous last words) so you might want to review it already: commit:8f3f506446039ac9e4ed2c7083d1e36acd64cd50.

Code review passes

#8 Updated by intrigeri 10 months ago

  • Status changed from In Progress to Fix committed

… which did not help much, otherwise I would not have had to file this ticket in the first place.

It doesn't seem like that was the problem, the key still has 2 self-signed signatures.

I believe you're mistaken and your using "still" word explains why:

  • If you count invalid (expired) self-sigs, anonym's key has had way more than 2 self-sigs for years; the test with the hard-coded "2" used to work only because this test we use --keyserver-options import-clean, so expired self-sigs are not imported.
  • If you don't count invalid (expired) self-sigs, before his key expired, he indeed had 2 valid self-sigs; then after it expired, it had 0, which made tests fail and thus I've filed this ticket; and finally, since he updated his key, as of today the key has 2 valid self-sigs again.

Code review passes

Thanks, merged!

#9 Updated by intrigeri 10 months ago

  • Assignee deleted (intrigeri)
  • % Done changed from 50 to 100

#10 Updated by intrigeri 10 months ago

  • Status changed from Fix committed to In Progress

#11 Updated by intrigeri 10 months ago

  • Status changed from In Progress to Fix committed

#12 Updated by intrigeri 10 months ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF