Page MenuHome GnuPG

arch-specific (32 bit) failures in AddExistingSubkeyJobTest::testAddExistingSubkeyWithExpiration
Closed, ResolvedPublic

Description

When jumping from 1.16.0 to 1.18.0 we are seeing a new test failure that is specific to QT and, more importantly, arch specific to 32 bit (i586, armv7l).

Build in our build system. Seems to have been introduced by https://dev.gnupg.org/rM4d913a8aa5dad1327bed5987dada89e9d7c5d292

[  203s] ********* Start testing of AddExistingSubkeyJobTest *********
[  203s] Config: Using QtTest library 5.15.5, Qt 5.15.5 (i386-little_endian-ilp32 shared (dynamic) release build; by GCC 12.1.0), unknown unknown
[  203s] PASS   : AddExistingSubkeyJobTest::initTestCase()
[  203s] PASS   : AddExistingSubkeyJobTest::testAddExistingSubkeyAsync()
[  203s] PASS   : AddExistingSubkeyJobTest::testAddExistingSubkeySync()
[  203s] FAIL!  : AddExistingSubkeyJobTest::testAddExistingSubkeyWithExpiration() 'result.code() == GPG_ERR_NO_ERROR' returned FALSE. ()
[  203s]    Loc: [t-addexistingsubkey.cpp(216)]
[  203s] PASS   : AddExistingSubkeyJobTest::cleanupTestCase()
[  203s] Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 2820ms
[  203s] ********* Finished testing of AddExistingSubkeyJobTest *********
[  203s] FAIL: t-addexistingsubkey

Likely cause is expiry after 2038...

ssb   cv25519 2022-01-13 [E] [expires: 2100-01-01]

Similar to https://dev.gnupg.org/T5522

Event Timeline

There is a reason that we switched to ISO Date strings in large parts of GnuPG ;-)

ikloecker added a project: Restricted Project.
ikloecker moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
ikloecker triaged this task as Normal priority.Aug 17 2022, 6:50 PM

WIP with those three patches:

[  276s] ********* Start testing of AddExistingSubkeyJobTest *********
[  276s] Config: Using QtTest library 5.15.5, Qt 5.15.5 (i386-little_endian-ilp32 shared (dynamic) release build; by GCC 12.1.0), unknown unknown
[  276s] PASS   : AddExistingSubkeyJobTest::initTestCase()
[  276s] PASS   : AddExistingSubkeyJobTest::testAddExistingSubkeyAsync()
[  276s] PASS   : AddExistingSubkeyJobTest::testAddExistingSubkeySync()
[  276s] FAIL!  : AddExistingSubkeyJobTest::testAddExistingSubkeyWithExpiration() Compared values are not the same
[  276s]    Actual   (result.code())                     : 1
[  276s]    Expected (static_cast<int>(GPG_ERR_NO_ERROR)): 0
[  276s]    Loc: [t-addexistingsubkey.cpp(216)]
[  276s] PASS   : AddExistingSubkeyJobTest::cleanupTestCase()
[  276s] Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 4076ms
[  276s] ********* Finished testing of AddExistingSubkeyJobTest *********
[  276s] FAIL: t-addexistingsubkey

Hmm. Please run the test with

GPGMEPP_INTERACTOR_DEBUG=stderr GPGME_DEBUG=8 TESTS="initial.test t-addexistingsubkey final.test" make -e check-TESTS

in lang/qt/tests under the build folder to get (a lot of) debug output.

relevant items start at line 4900 ...

[  274s] + pushd lang/qt/tests

Thanks! It seems that we pass the correct expiration date to gpg:

EditInteractor: action result "21000101T120000"

So, it's maybe a problem in gpg now.

Yes, it's a problem in gpg. gpg asks for the expiration date of the subkey

[  277s] EditInteractor: 4 -> nextState( GET_LINE, keygen.valid ) -> 5

gpgme replies with an ISO date

[  277s] EditInteractor: action result "21000101T120000"

Then gpg asks again for the expiration date

[  277s] EditInteractor: 5 -> nextState( GET_LINE, keygen.valid ) -> 4294967295

which gpgme doesn't expect, so that gpgme return a "general error".

The fact that gpg asks again for the expiration date means that it didn't accept "21000101T120000" as valid expiration date.

Looking at the code we end up in isotime2epoch (common/gettime.c in gnupg) which calls timegm with the parsed and adjusted values from the ISO date string. Since timegm returns time_t which is probably a signed 32-bit integer on 32-bit systems, I assume that timegm rejects our date and returns (time_t) -1.

This problem seems to be unfixable as long as we don't use a 64-bit time_t.

So much for "There is a reason that we switched to ISO Date strings in large parts of GnuPG ;-)". It's great to use ISO date string, but if, internally, we rely on time_t and on system functions like timegm, this doesn't really help avoid time_t's limitations.

I'll make gpgme return a better error code than "general error" if the expiration date is rejected and adjust the test accordingly.

It will be a lot of work to change this in gpg. Thus ISO dates were only introduced with gpgsm after the former glibc maintainer refused to switch to a 64 bit time_t - which would have been easy enough at that time (about the year 2001).

Actually an unsigned 32 bit time_t would be sufficient because the specs say that (time_t) (-1) indicates an error. However, I have seen lots of code using atime < 0 in conditions. It is all a mess. I once had one guy working on an entire replacement of the time functionality (implementing Markus Kuhn's proposal) until we realized that time functions are used all over the place and no portable way to add this to exitsing systems exists. What's left over from this project are the estream functions.

ikloecker moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

Thanks for reporting and testing my fixes.

Note the relationship to T4195, T4826, and T4766