Banning QNetworkAccessManager

[Volker Krause]

On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> On Mon, Feb 3, 2020 at 7:42 AM Volker Krause <vkrause at kde.org> 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.

> 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?

Also, I have yet to see a suggestion for a viable alternative to QNAM. The 
initially proposed fork would mean trading the possibility of broken redirect 
handling for an unmaintained HTTP stack, I don't think that's a good deal for 
the security of our users.

Instead, how about the way we approached this for KUserFeedback? Before 
getting things deployed on KDE infrastructure for use by library or 
application code, sysadmins and developers review if the implementation is 
allowing easy and secure operation of the infrastructure, redirect handling 
being one part of this. In case of KUserFeedback you suggested protocol 
changes, they were implemented, and automatic tests for a number of possible 
redirect scenarios were added. Worked out fine IMHO, despite QNAM (the needed 
changes weren't due to missing redirect handling, but due to somewhat non-
standard redirects on HTTP POST).

There is a risk that the code breaks this again over time, independent of test 
coverage, but I'd say that's not sysadmin's problem then, it's due to 
application code not following the rules.

This of course only helps with new infrastructure deployments, but so would a 
QNAM ban only help with new releases.

Additionally, improved documentation, a possible KNAM and/or driving the QNAM 
changes upstream can still be done alongside this obviously.

Regards,
Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20200219/7be20fa1/attachment.sig>