Same problem with konqueror and HTTPS server certificates ... I check "accept forever" but the popup always come back.

I found that the "invalid certificate ignore preference" was saved to ~/.trinity/share/config/ksslpolicies 
This file looks correctly written to me, so I guess our problem is related to reading the "always accepted certificates".

I've found out the reason of my problem.

Situation: I'm using an home-hosted web server, using SSL with a self-signed certificate which has all flaws (self signed, subject does not match FQDN, validity expired ...).
BUT I still want to *always* accept it, without being prompted.

I've seen the following strangeness in the code handling SSL certificates. (I'm in kdelibs 3.5.13.1)..

In file: kio/kio/tcpslavebase.cpp (line 856)
Before the following code, the "cp" (Certificate Policy) is set to KSSLCertificateCache::Accept, because it is written in config file ~/.trinity/share/config/ksslpolicies.

856         if (!_IPmatchesCN && cp == KSSLCertificateCache::Accept) {
857            cp = KSSLCertificateCache::Prompt;
859         }

That means, whatever setting I've saved in my config file, if the _IPMatchesCN variable is false, I will get the popup dialog.
Since my SSL certificate was issued to "localhost.localdomain" (or anything that does not resolve to a valid Internet IP address), I will always get the TDE's dialog box because the URL I type in konqueror is not "localhost.localdomain".

I think we should reconsider this case (maybe look at what is done in KDE4 if this code still exists ?), because in my case, this behaviour is not desirable.
Maybe we should replace the "always prompt" behaviour with a big warning, like in Firefox, saying "Hey, if you accept an invalid certificate, we cannot guarantee your safety, are you really really sure you want to always accept it ?"

Created attachment 1069 [details]
kdelibs 3.5.13.1 : Disable "always prompt" for invalid SSL certificate with invalid IP address

The attached patch simply removes the unexpected (In my opinion) behaviour in SSL-certificate caching.
When the user has chosen "Always Accept" for a certificate, he should *never* be prompted again about the same certificate, even it the certificate is totally invalid.

The patch seems to be a sledge hammer approach. :) Seems the correct way to resolve the problem is to honor the ksslpolicies Permanent=true/false setting?

No, I don't think the patch is sledge hammer. In fact, I think the original code was not in the right place. Let me explain.

It looks like that the SSL certificate caching is handled by a KIO and a KIO-slave.

The KIO is responsible to manage the SSL certificate cache

No, I don't think the patch is sledge hammer. In fact, I think the original code I've disabled is not in the right place. It looks like it's a quick fix for some specific bug. Let me explain.

The SSL certificate caching is handled by a KIO-server, which is accessed through a KIO-slave.

The KIO-server is responsible to manage the SSL certificate cache. It is the only one allowed to read and write the cache (The cache is the file: ~/.trinity/share/config/ksslpolicies).

When an application, like Konqueror or Kmail, tries to validate a certificate, it uses the KIO-slave. Only the KIO-slave can communicate with the KIO-server.

So, basically, in chronological order, the validation cycle should be:
1) Konqueror tries to validate a certificate. It sends the certificate to KIO-slave and awaits an answer, "accepted" or "refused".
2) The KIO-slave sends the certificate to the KIO-server for validation and awaits an answer like "accept", "refuse", or "prompt the user".
3) The KIO-server reads the certificate cache, and depending on what it finds in there, returns the correct answer. In the case where the user had previously chosen "Accept Forever", the KIO-server returns "accept" because the cache contains "Permanent=True".
4) The KIO-slave returns the answer "accepted" or "refused" to the calling application, depending on what KIO-server said. In case the server answered "prompt", KIO-slave displays a graphical popup so that the user must decide if he accepts or refuses. BUT, for an unknown reason, the KIO-slave also performs its own check on the certificate validity (this is the code I've disabled in the patch). Even if the KIO-server returned "certificate is valid (do not prompt)", the KIO-slave can decide to prompt the user anyway.
5) The application establishes or aborts the SSL connection to the remote server, depending of the KIO-slave answer.

So my patch basically disables the KIO-slave certificate check, which can override the KIO-server's decision. IMO the validation should only be done by the KIO-server.
If the "IPmatches" check is relevant, it should be done by the KIO-server anyway.

Patch in attachment 1069 [details] updated for R14 and pushed to GIT in commit 5975b91a.

The original patch for 3.5.13.x needs to be pushed to GIT.

After applying the patch and building a new package set, I'm closing this bug report as resolved. I'm no longer experiencing the stupid message box.

Thanks Francois. :-)

The explanation is excellent. The patch works great. François, great job, thank you.

However, I suggest either add to the code a link to above explanation, or (better) this part of the code to remove completely - with referring above explanation in the commit log.

The way it is now, I fear that the random reader can have question, why the code is commented out. And there is a danger that somebody can enable it again.

What is your opinion?

Do it! :-)

I am not opposed to the patch, however this discussion does bring up a more fundamental flaw in the TDE certificate override system.  TDE should record which flaws exist with a particular overridden certificate; if the list of flaws changes since the manual override TDE should prompt, otherwise it should not.

For instance, someone may override a trust chain verification for a self signed certificate, but still expect to be notified if the IP address of the server changes (e.g. an attack is in progress).  Perhaps a FIXME for this in the code would be sufficient for now, along with a feature request bug report?

Please do not close this report. Within the past few days this bug has returned. I don't know why.

This problem still persists.

Comment on attachment 1069 [details]
kdelibs 3.5.13.1 : Disable "always prompt" for invalid SSL certificate with invalid IP address

Note: the patch in attachment 1069 [details] was pushed to git in commit 5975b91a 2013-03-02.

By the way, this bug was reported in KDE 3.5.x and never resolved:

https://bugs.kde.org/show_bug.cgi?id=116723

I have moved my mail list participation to kmail. This bug is now becoming a big nuisance. Every day I have to respond to these dialogs.

Still present in 14.0.1. This is a major nuisance, as it prevents automatic downloading of emails from some accounts.

I am seeing this issue, but with receiving emails over IMAP instead of sending emails. I see the dialog roughly once a day, and when it appears, I also get a second dialog (at exactly the same time) alerting me that it was disconnected from the IMAP server. Accepting the certificate forver resolves the issue for roughly another 24 hours.

Setting up my first kmail client using imap/ssl and smtp/ssl. Setting up the account,  I use the "accounts>modify>security> 'check what server supports' and get a response from the server.. I can send mail using smtp/ssl/port 465. Trying to retrieve mail nothing happens until, after a period of time, a message dialog pops up saying ~ connection timed out...

Without any other error messages, that I know of, I am guessing it is related to kmail's processing of certificates.

Hi,
interestingly I have the issue too. I am asked to accept certificate even I have said before that I accept this permanently. No going through this bug I got curios and when I looked into the file ~/.trinity/share/config/ksslpolicies 

Expires=2935093,2,28,0,0,0

This should be what exactly? It looks broken to me.

Here is what I did.
1. checked the file .. there was year/month/day there of expiration, the Permanent=false
2. started Kontact
3. when prompted on certificates I said forever
4. checked again the config file with above result
5. stopped kontact (wait all to shut down)
6. started kontact
7. checked the config file 
 - Expires=2016,9,21,22,29,28
 - Permanent=false

where is this piece of code that writes the date there? this was bugging me since more than an year already.

BTW I just rebuild and deployed the tdelibs from git yesterday

Created attachment 2695 [details]
patch to fix the ssl accept permanent issue in kmail

This might be not sufficient, but quick testing showed it worked.
It might be the check needs to be done in some other places before writing the datetime to the config file. However I noticed this is the first function it calls before setting the expiry date.

My concern is in the use case when cert expires - will the patch honer this?

Hi Emanoil,
I had a quick look through the patch. This looks more like a work around (setting expiring date 10y later) than a full patch.
If would be better if we could fix the code so that when the "permanent" flag is set, it does not ask for permission and accept.

Yes agree. It looks like work around - actually a quick fix.
The proper solution would be to use the datetime from the certificate. Per default it is impossible to create certificate without expiration date.
This would imply that this date is checked first.
I put it on the todo list.
As you can imagine it would be helpful to apply the quick fix first until the proper solution is in place.
I don't know how many people use kmail/kontacts for reading mail, but for me it was annoying. Relieved now being not prompted each hour.

Looking into the problem further I noticed that even the datetime is now set to 10y in the future. The permanent is set to false each time I start kontact.
I also tested with konquerer. There the hosts value is not being populated.

So this looks like to be a deeper issue and needs to be handled with more care.

My conclusion is that it is wrong to set the expire time to currentDateTime+3600

The question in the prompt of Konqueror/Kmail etc is "keep forever" or "for current session"
So the check should be done based on the shutdown of the application and not based on cert_accept_time+3600.

In the present situation if I shutdown whatever I use in the time frame < cert_accept_time+3600 the time is overwritten at the next start again but  app_start_time+3600 so I never get prompted even the time where I said to keep it for current cert_accept_time+3600 expired.

Do you know what is meant by session exactly - is it the close_time of the app, or is there a dedicated session?

On the permanent true/false issue - need to investigate where the methods are called and why it overwrites true to false each time I start kontact.

regards

Created attachment 2726 [details]
extended log from kssld with permanent and expires

I changed the status to new as the patch did not resolve the problem.

In the attached log (I added some more debugging in all the functions to trace the order and those two variables). so it is obvious that all goes fine until cacheAddCertificate is called. It looks like the caller passes permanent=false or it goes to default and this overwrites the stored value.

I hope I can deliver final patch next

Created attachment 2727 [details]
patch to fix the ssl accept permanent issue in kmail

I think this is the good one.

I am not sure if we need to check the expiration when adding a cert to the cache

example: 
  if (!node->expires.isValid() || node->expires < TQDateTime::currentDateTime() )

The first condition might be true only when we hit a new cert that was not added I guess.

Thanks Emanoil,
at first sight this patch looks much better. I will test soon, maybe later this week.

Created attachment 2728 [details]
patch to fix the ssl accept permanent issue in kmail

There was one or two unhandled case(s). Uploading a new one.
The main question is what happens if we have 'permanent' and certificate expires?
Can someone check the patch with this use case, please?
I tested the normal behavior and it works for me.

Comment on attachment 2728 [details]
patch to fix the ssl accept permanent issue in kmail

Partially cleaned up and modified.
Pushed to GIT in commit 7406ed0 (R14.1) and 46887a3 (R14.0)

Created attachment 2734 [details]
handle case when certificare expires (??)

Emanoil,
can you test the attached patch (against the latest sources) and see if it handles the case "when a certificate expires" correctly?
First test normally. Then change the date of the system to a date that would invalidate the certificate under test and see if the certificate is correctly removed.
If it is ok, I will later push the patch and close the bug.

By the way, as for all other patches you have submitted so far, I have assumed the patch you submitted was your own work and so proceeded to push it and signoff with your name. If that was not the case, please let me know.
I have to ask each time, as long as the CLA is not done. Otherwise I am not allow to push the patch ;-)

I think I'll have to update the test env I have to do this. I'm not sure when I'll be able to do this, but I set target date end of next weekend.
It is a vital patch as the bug is annoying and I feel relieved after I applied the patch, so I'll try to speed it up.
However I would do it the proper way: define the test cases and work them out. It will take some time doing so. Here is what I have in mind

+ Certificate valid
  - user accepted certificate permanently
  - user accepted certificate temporary

+ Certificate expires
  - user accepted certificate permanently
  - user accepted certificate temporary

If you see some other cases let me know

The temporary part works fine, so it is just regression. For the other part I need to create cert that expires and configure a mail server with this cert.

The question is what happens when cert expires and user has accepted this cert permanently. Should the user be warned each time or what is expected behaviour?

regards

The main bug should now be fixed, thanks to your patch which I have already pushed.

>The question is what happens when cert expires and user has accepted this cert 
>permanently. Should the user be warned each time or what is expected behaviour?
Once a certificate expires, it is no longer valid, so IMO it should be deleted even in case the user had previously accepted the certificate permanently. In this case, I consider "permanent" as "until the end date validity" of the certificate.
The patch I have attached tries to handle this case, but I need your help to test this since I do not use KMail at all.

Slavek and I are planning to freeze R14.0.4 very soon, so we would appreciate if you could test as early as possible ;-)

Ok, thanks, at least regarding cert expired we have same understanding. I'll try my best to test all the cases described by EOW.

Hi Michele, Slavek,
I set up now a test env - vmware where I installed trinity by the book (from jessie/stable)

The strange thing is that I see slightly different behavior than on my system.
Example: the outgoing mail also has certificate. I set permanent to true when asked - first it showed wrong date as observed before. I used the TDE cert manager and deleted all certs I accepted when setting up the mailbox. After starting kmail and being asked to accept cert I set again to true and now date is 3000,1,1,0,0,0 (not broken, but still wrong as the cert is set to expire 17.04.2030)

I'll try tomorrow the rest of the test procedure as Michele have requested - change time etc.

Let me know if you have any concerns

BTW I signed and sent the CLA to Tim

Created attachment 2735 [details]
testing latest patch by Michele

Hi Michele,
unfortunately this is not working correctly, because even if the certificate is set to permanent I am asked after the next reboot.
Attached a screenshot.

I tested now my patch and it does not handle the case when cert is set to permanent and it does expire
Perhaps I'll try next to do something based on yours and mine, but I first have to test the sync stuff as they want to release with the backend.
Perhaps you have some time to improve the patch before I can do this.

>Hi Michele,
>unfortunately this is not working correctly, because even if the certificate is >set to permanent I am asked after the next reboot.
>Attached a screenshot.

>I tested now my patch and it does not handle the case when cert is set to >permanent and it does expire
>Perhaps I'll try next to do something based on yours and mine, but I first have >to test the sync stuff as they want to release with the backend.
>Perhaps you have some time to improve the patch before I can do this.

Hi Emanoil,
I am confused. Your patch has already been pushed (without some commented kdDebug). My patch is meant to be tested on top of that.
How have you tested exactly? I am not sure I have understood correctly.

Hi Michele,
no need to be confused. I understood everything and tested as you described your patch on top of mine with the provided result. It seems that the changes you made do not solve the issue completely. It just asks for confirmation the first time kmail is started, despite the fact the certificate was accepted permanently.
I was in hurry, sorry for not providing more detailed input.

No problem, thanks for the clarification.
I will try to do some work on this during the weekend ;-)

Created attachment 2736 [details]
handle case when cert expires and clean up the code

Hi,

The problem with your patch was simply the condition 
  expires >= TQDateTime::currentDateTime()
in updatePoliciesConfig, which is called only when object is created - this explains I saw it only at first start.

However I got a bit pissed by the code in general and went through each function. I looked into all conditions and based on the assumption we made that despite permanent=true if cert expires it should be set to expired I modified all those condition where relevant and removed this insane checking and saving to the disk in each function (which was inconsistent btw). I wonder why it was put there in first place. I think it might be that one tried to solve the permanent problem.
In general there shouldn't be saving when data is not modified and there shouldn't be condition in the save function.
I also set default permanent to false in the constructor as this seems more convenient. 
I also tested all the cases mentioned earlier and results are satisfying. I do not like setting expiration date to now+3600secs, so I changed it to 10sec. This becomes not really relevant because the values are valid until logging out or rebooting and then it asks again, which covers the requirement "for this session only".

I think the code is much better off now. 

Just use your time to review the changes. Apply this patch on top of yours.

regards

Created attachment 2737 [details]
handle case when cert expires and clean up the code

I have overlooked something. 
Also in your patch it was the condition not in the updatePoliciesConfig but in the cacheLoadDefaultPolicies.

Thanks Emanoil, your patch seems to work fine. Pushed to GIT in commit f3fadb8 (R14.1) and 9c010f4 (R14.0). Thanks for the effort you put in.

I just noticed my patch was wrong, I had mistakenly written "expires >= TQDateTime::currentDateTime()" when what I wanted to do was actually the opposite... it was a quick, untested patch :-)