Bug 2791

Regarding the date issue I tested on jessie gpg output shows
gpg --no-tty --with-colons --list-public-key "xxxxx"
tru::0:1501148681:1586308542:3:1:5
pub:u:2048:1:553E5E7E73874FEC:2011-09-19:::u:xxxxxxx <yyyyyyyy>::scESC:
sub:u:2048:1:5258E9862AE43899:2011-09-19::::::e:

on stretch (same key)


gpg --no-tty --with-colons --list-public-key "xxxxx"
tru::0:1501148681:1586308542:3:1:5
pub:u:2048:1:553E5E7E73874FEC:1316465282:::u:::scESC:::::::
fpr:::::::::5AD4453E885BE8AE3329CA18553E5E7E73874FEC:
uid:u::::1316465282::F6EE12BC1669E88D68BF0967EDC5BA2CC54E8AC3::xxxxxxx <yyyyyyyy>:
sub:u:2048:1:5258E9862AE43899:1316465282::::::e::::::
fpr:::::::::8BC76D91E875EDA49B5B742C5258E9862AE43899:


So very clearly it is visible:
1. the date format has changed
2. the User name is not in the pub anymore but in the uid
3. additional fpr field shows up

this explains it all

The question is if there is someone around with more knowledge how it was implemented to do the work

regards

I managed to fix it, but as I spent time looking into it, I thing few improvements can be introduced + I must test backward compatibility.
Stay tuned :)

Created attachment 2805 [details]
kgpg gnupg21 patch

I think this is good to go

bug 2518 looks the same

Created attachment 2806 [details]
improved version of the patch

- double checked the kgpg code and found few missed processings of "sec" and "pub" where identity might get lost in gnupg > v2.1
- fixed those and formatted some of the code I found ugly not indented etc.
- double checked the definition of variables in the introduced functionality and cleaned up few places

There is a problem with the patch when importing specific keys - they show up twice in the interface. It is not happening with all keys however.

I'll try to fix this next.

Perhaps false alarm for that

http://wwwkeys.pgp.net/pks/lookup?search=0x8F8F4B28&op=index&fingerprint=on

Type bits/keyID     Date       User ID

pub  1024R/8F8F4B28 2014-06-16 *** KEY REVOKED *** [not verified]
                               Fen Labalme <fen@civicactions.com>
	 Fingerprint=4B53 D683 1C2F 45F2 D409  214F A11C 52FA 8F8F 4B28 

pub  1024D/8F8F4B28 2001-11-09 Fen Labalme <fen@civicactions.com>
                               Fen Labalme <fen@2idi.com>
                               Fen Labalme <fen@well.com>
                               Fen Labalme <fen@comedia.com>
                               Fen Labalme <fen@alum.mit.edu>
                               Fen Labalme <fen@idcommons.net>
                               Fen Labalme <fen@broadcatch.com>
                               Fen Labalme <fen@openprivacy.org>
                               Fen Labalme (primary key) <fen@comedia.com>
	 Fingerprint=7B47 460E AB09 1F26 4797  D297 48A5 6BA9 8F8F 4B28 

and kgpg imports both keys - what is irritating is that both have same ID, is the first one the revocation?

Indeed it looks like a problem in the search and parse function

If it were listed --with-columns, it would show following

pub:4B53D6831C2F45F2D409214FA11C52FA8F8F4B28:1:1024:1402887092::r
uid:Fen Labalme <fen@civicactions.com>:1407195007::
pub:7B47460EAB091F264797D29748A56BA98F8F4B28:17:1024:1005333841:1478613459:
uid:Fen Labalme <fen@2idi.com>:1461074266::
uid:Fen Labalme <fen@well.com>:1461074266::
uid:Fen Labalme <fen@comedia.com>:1461074266::
uid:Fen Labalme <fen@alum.mit.edu>:1461074266::
uid:Fen Labalme <fen@idcommons.net>:1461074265::
uid:Fen Labalme <fen@broadcatch.com>:1461074266::
uid:Fen Labalme <fen@openprivacy.org>:1461074266::
uid:Fen Labalme <fen@civicactions.com>:1461074259::
uid:Fen Labalme (primary key) <fen@comedia.com>:1005333841::

Notice the 'r' for the revocation - this is entirely ignored in the present code. So it looks like I will rewrite this part and upload a new patch.

I guess this new version is also affecting the pgp servers. unfortunately I can not bring up my old sks to test the output, however while testing with wwwkeys.pgp.net, I see that searching for 0xABCDEF returns two entries with "pub" - one for the key and one for revokation (if revoked).
This results in importing both as separate keys.

For import signature I replaced slotImort() with slotSearch(), so that the popup appears and most important the response of the server is parsed before offered to the user.

For the import routine however it is impossible as it is passed to gpg --recv-keys, so that it automatically adds both keys.

I see few possible solutions:

1. the approach replacing slotImport() with slotSearch() all over the place
2. perhaps more proper add a --search-keys functionality to slotImport() infront of the actual import, so that if key is revoked, it does not get imported at all.
3. replace 0xABCDEF with the full key id - but this would go too deep in the functionality of kgpg as it uses the short form to find also keys,fingerprints etc related to the key

I think the same should be applied if key has expired.

In my opinion option would be the correct one and something is telling me that it would probably reuse some of the slotSearch() functionality.

Created attachment 2807 [details]
fix for gnupg2.1, default key server shows twice

I could not test all of this in jessie, but I hope it will work

Created attachment 2808 [details]
Testcases

Here are the test cases I've done so far.
This covers my needs and use of kgpg. Unfortunately I spent too much time on it and can't go further with testing.

Created attachment 2809 [details]
fix for gnupg2.1, default key server shows twice, sort keys by name in export

Created attachment 2810 [details]
Added export testcase

following the gnupg mailing list for almost 2 months now, I can summarize two facts: 

There is a sign that tqt pinentry can be added to the official branch .... but just on statistical bases - most likely if it happens it will take time.

Most important news is that gpg 1.4 is only for compatibility, so good that I did not spend time to make the code using both versions.

I went through a patch and conducted a basic tests - on Wheezy for gnupg < 2.1 and on Stretch for gnupg >= 2.1. It looks good.

As you rightly noticed, some parts of the original code are very poorly readable. To ensure better readability of the modified code sections, I have made some additional formatting adjustments.

Besides, I removed the two parts of the patch because they looked like the remains of some tests or abandoned.

1) The code in a comment that looks confusing:

+//     if (serverTmp.find(defaultKeyServer,0))
+//             int idx = 0;
+//     while  ( serverTmp.find(defaultKeyServer,0) )
+//             serverList.prepend(defaultKeyServer+" "+i18n("(Default)"));
+//     else {
+//             TQStringList l = serverList.grep(defaultKeyServer);
+//             for (TQStringList::Iterator it = l.begin(); it = l.end(); it++ )
+//
+//     }


2) A loop that does nothing:

+
+       for ( uint i = 0; i < signList.count(); ++i ) {
+       //      kdDebug(2100) << k_funcinfo << "List at: " << signList.at(i)->text(6) << endl;
+       }
+

Now it looks like the patch is ready to be pushed into the GIT.
Emanoil, good work, thank you.

Thank you Slavek,
in this context I want to add following:

1. perhaps you want to have a look at the pinentry-tqt (bug/request). I was able to convince the gnupg developers to include the code in the main stream pinentry, however it might be still not merged - there is the merge request - and it would be great to have it in the tde build list, so that we provide native tde pinentry.

2. I am using this kgpg2 since then in combination with the pinentry-tqt and kmail and it works great. However I have a small issue that if I shutdown with kgpg applet running and start again, after login sometimes it crashes on unique application. I don't think it is related to the patch itself, but I did not have time to dig into it. When it goes in I will file a bug, so that it can be fixed.

3. Regarding your note 1), I don't recall exactly where it is, but I recall that there was an issue with the default server shown twice (I think it was even in the bug list) - once from the config file and once from the default in the code. I think I solved it. I hope removing this part is not affecting this functionality.

Anyway - thank you as well for taking care of it

regards

Pushed to GIT in commit 9125ead9 (master) and e2bee02e (r14.0.x).

(In reply to deloptes from comment #16)
> Thank you Slavek,
> in this context I want to add following:
> 
> 1. perhaps you want to have a look at the pinentry-tqt (bug/request). I was
> able to convince the gnupg developers to include the code in the main stream
> pinentry, however it might be still not merged - there is the merge request
> - and it would be great to have it in the tde build list, so that we provide
> native tde pinentry.
> 

Yes, about pinentry-tqt I am very interested. I myself long ago wanted to prepare the source so it could be compiled against the upstream pinentry, but I did not find enough time to finish it.

I imagine that we would build pinentry-tqt as a stand-alone package regardless of whether it will be integrated into the upstream.

> 2. I am using this kgpg2 since then in combination with the pinentry-tqt and
> kmail and it works great. However I have a small issue that if I shutdown
> with kgpg applet running and start again, after login sometimes it crashes
> on unique application. I don't think it is related to the patch itself, but
> I did not have time to dig into it. When it goes in I will file a bug, so
> that it can be fixed.
> 

On my test machine I did not notice problems.

> 3. Regarding your note 1), I don't recall exactly where it is, but I recall
> that there was an issue with the default server shown twice (I think it was
> even in the bug list) - once from the config file and once from the default
> in the code. I think I solved it. I hope removing this part is not affecting
> this functionality.
> 

The code was in the comment, and it seemed like it could not work. I suppose that its exclusion will not break anything :)

> Anyway - thank you as well for taking care of it
> 
> regards

(In reply to Slávek Banko from comment #18)
> (In reply to deloptes from comment #16)
> > Thank you Slavek,
> > in this context I want to add following:
> > 
> > 1. perhaps you want to have a look at the pinentry-tqt (bug/request). I was
> > able to convince the gnupg developers to include the code in the main stream
> > pinentry, however it might be still not merged - there is the merge request
> > - and it would be great to have it in the tde build list, so that we provide
> > native tde pinentry.
> > 
> 
> Yes, about pinentry-tqt I am very interested. I myself long ago wanted to
> prepare the source so it could be compiled against the upstream pinentry,
> but I did not find enough time to finish it.
> 
> I imagine that we would build pinentry-tqt as a stand-alone package
> regardless of whether it will be integrated into the upstream.
> 

Yes, this would be necessary. The pinentry-tqt is TDE specific, so we keep the code @gnupg and build the packages for TDE. We'll need to maintain only the specific debian build scripts for TDE. This is the plan. 

> > 2. I am using this kgpg2 since then in combination with the pinentry-tqt and
> > kmail and it works great. However I have a small issue that if I shutdown
> > with kgpg applet running and start again, after login sometimes it crashes
> > on unique application. I don't think it is related to the patch itself, but
> > I did not have time to dig into it. When it goes in I will file a bug, so
> > that it can be fixed.
> > 
> 
> On my test machine I did not notice problems.
> 

Great - it might be that the problem is in my setup. Thanks for confirmation.

> > 3. Regarding your note 1), I don't recall exactly where it is, but I recall
> > that there was an issue with the default server shown twice (I think it was
> > even in the bug list) - once from the config file and once from the default
> > in the code. I think I solved it. I hope removing this part is not affecting
> > this functionality.
> > 
> 
> The code was in the comment, and it seemed like it could not work. I suppose
> that its exclusion will not break anything :)
> 

great again

I don't have any further comments.

cheers

Created attachment 2828 [details]
unhandled use case open signature.asc in knode

Comment on attachment 2828 [details]
unhandled use case open signature.asc in knode

I found one unhandled use case in the new kgpg and I am not sure if I can solve this here or just open a new bug. Perhaps the latter is the proper way. Nevertheless I post here my findings and we can move them anytime to a new bug.

So when I try to open signature.asc in knode I get "Decryption failed" (see attached screenshot).

I just checked knode now - it looks like kgpg has it's own implementation there.
I will open a new bug next, to track and fix it