Re: Banning QNetworkAccessManager

2020-02-20 Thread Albert Astals Cid
El dijous, 20 de febrer de 2020, a les 14:29:47 CET, Allen Winter va escriure:
> On Wednesday, February 19, 2020 6:09:02 PM EST Albert Astals Cid wrote:
> > El dimecres, 19 de febrer de 2020, a les 9:28:22 CET, Volker Krause va 
> > escriure:
> > > Additionally, improved documentation, a possible KNAM and/or driving the 
> > > QNAM 
> > > changes upstream can still be done alongside this obviously.
> > 
> > Also for Qt5 which will probably never be changed a clazy warning and 
> > making it easy to run clazy on gitlab CI would be great.
> > 
> 
> Krazy has a checker for QNetworkAccessManager use in Qt4 code.
> I could add a checker for Qt5 code if someone tells me what to look for
> (not that many people look at krazy reports. couldn't hurt. might help.

There's other ways to do it, but i guess we could start with something that 
checks for this?

https://community.kde.org/Policies/API_to_Avoid#QNetworkAccessManager

Cheers,
  Albert





Does KDE have a Secret Service Implementation?

2020-02-20 Thread Jacky Alcine
I'm working on a social media client for KDE [1] and I want to store secrets 
for this application "the right way". It deals with OAuth access tokens. I 
figure putting that into the secret service[2] was the right way but I get the 
following error[3].

If Secret Service isn't the way and something else is recommended (like 
QtKeychain or signond), I'd love to know.

[1]: https://invent.kde.org/jalcine/activitydesk/
[2]: https://specifications.freedesktop.org/secret-service/latest/
[3]: https://invent.kde.org/jalcine/activitydesk/issues/3
-- 
Bet,
*Jacky Alcine*





Re: Banning QNetworkAccessManager

2020-02-20 Thread Nicolás Alvarez
El jue., 20 de feb. de 2020 a la(s) 10:30, Allen Winter
(win...@kde.org) escribió:
>
> On Wednesday, February 19, 2020 6:09:02 PM EST Albert Astals Cid wrote:
> > El dimecres, 19 de febrer de 2020, a les 9:28:22 CET, Volker Krause va 
> > escriure:
> > > Additionally, improved documentation, a possible KNAM and/or driving the 
> > > QNAM
> > > changes upstream can still be done alongside this obviously.
> >
> > Also for Qt5 which will probably never be changed a clazy warning and 
> > making it easy to run clazy on gitlab CI would be great.
> >
>
> Krazy has a checker for QNetworkAccessManager use in Qt4 code.
> I could add a checker for Qt5 code if someone tells me what to look for
> (not that many people look at krazy reports. couldn't hurt. might help.

I started making a checker for this based on clang-static-analyzer.
Looks like clazy uses a different approach altogether by looking at
ASTs alone, so I don't think I can integrate into that...

-- 
Nicolás


Re: Banning QNetworkAccessManager

2020-02-20 Thread Allen Winter
On Wednesday, February 19, 2020 6:09:02 PM EST Albert Astals Cid wrote:
> El dimecres, 19 de febrer de 2020, a les 9:28:22 CET, Volker Krause va 
> escriure:
> > Additionally, improved documentation, a possible KNAM and/or driving the 
> > QNAM 
> > changes upstream can still be done alongside this obviously.
> 
> Also for Qt5 which will probably never be changed a clazy warning and making 
> it easy to run clazy on gitlab CI would be great.
> 

Krazy has a checker for QNetworkAccessManager use in Qt4 code.
I could add a checker for Qt5 code if someone tells me what to look for
(not that many people look at krazy reports. couldn't hurt. might help.






Re: Banning QNetworkAccessManager

2020-02-20 Thread Volker Krause
On Wednesday, 19 February 2020 10:04:11 CET Ben Cooksley wrote:
> On Wed, Feb 19, 2020 at 9:30 PM Volker Krause  wrote:
> > On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> > > On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > > > I agree on the problem of QNAM's default, see also
> > > > https://conf.kde.org/en/
> > > > akademy2019/public/events/135 on that subject.
> > > > 
> > > > On Saturday, 1 February 2020 23:24:14 CET Ben Cooksley wrote:
> > > > [...]
> > > > 
> > > > > Prior to now, i've taken the approach of advertising that
> > > > > QNetworkAccessManager is broken and needs a flag set within
> > > > > applications when used, however unfortunately this seems to have
> > > > > been
> > > > > an ineffective approach and cases of broken behaviour continue to
> > > > > appear (despite this brokeness being known about and advertised
> > > > > since
> > > > > back in the Qt 4.x series when this class was introduced)
> > > > > 
> > > > > Therefore, given the Qt Project is unlikely to change their position
> > > > 
> > > > > on this, I would like to propose the following:
> > > > The Qt Project is actually in favor of changing at least the
> > > > redirection
> > > > default to exactly what we want it to be.
> > > > https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the change,
> > > > and
> > > > got approval both by Lars and one of the maintainers. It is merely
> > > > stuck
> > > > in dealing with the unit test fallout in Qt's CI that somebody has to
> > > > take care of. Doesn't immediately help us of course, but claiming Qt
> > > > is
> > > > unwilling to change this is simply wrong.
> > > > 
> > > > > 1) That effective immediately, QNetworkAccessManager and it's
> > > > > children
> > > > > classes be banned and prohibited within KDE software, to be enforced
> > > > > by server side hooks.
> > > > > 2) That we fork QNetworkAccessManager and the associated classes
> > > > > within the appropriate Framework (to be determined), where the
> > > > > defective behaviour can then be corrected.
> > > > 
> > > > While this might solve the redirection problem, it brings us yet
> > > > another
> > > > then unmaintained HTTP implementation next to the one in KIO, which is
> > > > a
> > > > far bigger problem long term. We need less of those, not more.
> > > > 
> > > > To address the problem of people not using the correct defaults, I did
> > > > propose a much less extreme approach in the above mentioned talk at
> > > > Akademy, namely having a KNetworkAccessManager as a sub-class of QNAM
> > > > in
> > > > a low-tier frameworks that essentially just enables redirects and
> > > > HSTS.
> > > > QNAM's implementation isn't the problem after all, the defaults are.
> > > > 
> > > > Commit hooks warning about QNAM usage might still be a good idea, but
> > > > as a
> > > > warning only. Hard blocking QNAM-using code would block at least three
> > > > repos I spend a lot of time on currently, none of which even talks to
> > > > KDE
> > > > servers.
> > > > 
> > > > If you need to take more drastic measures against code not doing this
> > > > correctly regardless, you can do that my dropping the server-side
> > > > workarounds. Yes, that would break the affected application possibly,
> > > > but
> > > > at least it would not cause massive collateral damage for the people
> > > > using this correctly.
> > > > 
> > > > It would also help to know where specifically we have that problem, so
> > > > we
> > > > can actually solve it, and so we can figure out why we failed to fix
> > > > this
> > > > there earlier.
> > > 
> > > Just bringing this up again - it seems we've not had much movement on
> > > this aside from the Wiki page.
> > > 
> > > Examining that Qt merge request, it not only is targeted at just Qt
> > > 6.x, it also hasn't had any movement since the start of October 2019 -
> > > 4 months ago.
> > > Given that we are going to be on Qt 5.x for some time, that merge
> > > request can't be considered to be the 'fix' for this issue.
> > 
> > The targeting of Qt6 is due to this aiming at dev when that was still Qt
> > 5.x, but you are right of course, somebody needs to do the work there to
> > drive this to completion, no matter in which version it will actually
> > land.
> 
> Indeed.
> 
> > > We need a solution that will ensure software released today doesn't
> > > cause us pain in several years time, because as I illustrated in an
> > > earlier email to this thread, software has a habit of remaining in use
> > > for an extremely long amount of time (August 2014 being the release
> > > date of the Marble versions in question hitting that old URL).
> > > 
> > > The easiest path forward is to simply ban QNetworkAccessManager.
> > 
> > That might be the easiest path for you, but certainly not for me, given
> > two of the modules I work on most atm are using QNAM (not even to talk to
> > KDE infrastructure), and would suddenly no longer be allowed to be hosted
> > here?
> Ideally they wo

Re: Fixing QNetworkAccessManager use

2020-02-20 Thread Ben Cooksley
On Thu, Feb 20, 2020 at 2:09 AM Friedrich W. H. Kossebau
 wrote:
>
> Am Mittwoch, 19. Februar 2020, 08:05:01 CET schrieb Ben Cooksley:
> > On Mon, Feb 3, 2020 at 7:42 AM Volker Krause  wrote:
> > > It would also help to know where specifically we have that problem, so we
> > > can actually solve it, and so we can figure out why we failed to fix this
> > > there earlier.
> >
> > Just bringing this up again - it seems we've not had much movement on
> > this aside from the Wiki page.
>
> The wiki page currently still just recommends to set
> "networkAccessManger->setAttribute(QNetworkRequest::FollowRedirectsAttribute,
> true);"
>
> Which seems simple, but possible not what is enough in all cases.
>
> So my open questions here to be able to act on code I contribute to are:
>
> a) What about the mentioned QNetworkRequest::NoLessSafeRedirectPolicy, in
> which cases should that be used and when not?

For interacting with download.kde.org / files.kde.org, I would advise
against using this policy, as they will in virtually all instances
redirect to mirrors (who don't support https and are http only)

>
> b) What about the HSTS stuff, when is that recommended?

That should be enabled yes.

>
> c) What is a sane number for QNetworkRequest::maximumRedirectsAllowed?

5 to 10 redirects is a relatively sane number I would expect. At the
most I would expect our servers to issue a maximum of 3 redirects in a
given chain of URLs.
If it is longer than that then we are doing something wrong.

>
> Both in general and when it comes to KDE servers.
>
> Personally I am still unsure what the actual issue is. Why are redirects
> needed at all. Why all the address changes all the time? The "U" in
> "URL"/"URI" is for "uniform", not "unstable", isn't it ;)

Please see my other email regarding this.

>
> Can you give some examples for URLs of resources our code uses on KDE servers,
> and why they needed to change?

Get Hot New Stuff functionality (Gen 1), originally using a static
file tree under http://download.kde.org/khotnewstuff/
This needed to change for two reasons:
1) Mandatory HTTPS
2) The benefit of having these files mirrored, considering their
extremely small size and declining client base (KDE 3 and parts of KDE
4) was negligible and creating more load on our systems to support the
mirroring process than we got in terms of benefit of having them
mirrored. We therefore transitioned to serving these through a CDN.

Get Hot New Stuff functionality (Gen 2), originally using a dynamic
web service at http://newstuff.kde.org/ and http://data.kstuff.org/
needed to change for two reasons:
1) Mandatory HTTPS
2) The dynamic web service had not been updated in several years, and
was dependent on a very specific system setup we hadn't been able to
replicate and needed to decomission due to it's age. We therefore
needed to convert it to static files, and arrange for those to be
hosted elsewhere in our systems. newstuff.kde.org now converts the
requests sent to it to redirects to specific static files to keep
applications using it working (which includes KF5 era applications who
still actively use this and in at least one case continue to be
released using this)

Get Hot New Stuff functionality (Gen 3), originally used a file at
http://download.kde.org/ocs/providers.xml (now at
https://autoconfig.kde.org/ocs/providers.xml)
This needed to change for two reasons:
1) Mandatory HTTPS
2) It was necessary for non-sysadmins (particularly those involved in
running store.kde.org) to be able to update the file directly. As the
server hosting download.kde.org is sensitive and doesn't support
deploying changes from Git when they are committed, we had to move the
file to a different subdomain which could support this.

Marble maps, originally hosted under http://download.kde.org/ and
later at http://files.kde.org/marble/maps/ and now at
https://maps.kde.org/,
This need to be moved for couple of reasons:
1) When we transitioned download.kde.org to be a mirror redirector, it
was no longer possible for us to easily host non-mirrored resources
under the same domain (and the maps weren't mirrored), requiring they
be moved to files.kde.org (which as an added benefit also made it
possible for developers to update the maps themselves)
2) Later, it was discovered that Marble performance for loading maps
using files.kde.org after it transitioned to being a mirror redirector
as well was quite poor due to the large number of http requests
involved. We therefore shifted it to a CDN based resource which
eliminated these performance issues, known as maps.kde.org.

KStars resources, originally hosted under
http://download.kde.org/apps/kstars/ needed to be moved to
https://files.kde.org/ for the following reasons:
1) Mandatory HTTPS
2) To allow developers to freely update them as needed, something
which isn't possible on download.kde.org (which is restricted due to
it hosting the master copies of tarballs)

There have also been two instances where we have been de