Banning QNetworkAccessManager

[Johnny Jazeix]

Hi,

as Volker said, without any alternative, we can't just ban QNAM.
And even with one, we would need time to implement it (for example,
for GCompris, this part of the code was written by someone who is less
active now, so rewriting it, testing it and be sure it works will take
some time).

Can't we use lxr to find all applications using it, check if they use
it in a good way or not instead?

Johnny


Le mer. 19 févr. 2020 à 10:05, Ben Cooksley <bcooksley at kde.org> a écrit :
>
> On Wed, Feb 19, 2020 at 9:30 PM Volker Krause <vkrause at kde.org> wrote:
> >
> > 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.
>
> 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 would move to whatever replacement we have for QNAM
> should it be banned :)
>
> >
> > 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.
> >
>
> It isn't perfect yes.
>
> Sadly however the negative user experience created by countless broken
> applications due to the Qt Project being uncooperative in this matter
> for many years doesn't really give us much in the way of other
> options.
>
> It doesn't help when Developers come to us asking for temporary
> measures to be put in place to keep their applications alive as
> they're getting bug reports about broken functionality. Saying no
> saves us the effort of maintaining the fixes, but leaves applications
> broken - which is not a fun position to be in as the one making the
> decision.
>
> > 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).
> >
>
> Having a Sysadmin review an implementation that will be talking to a
> server is in general always a good idea, even outside of this
> particular usecase.
> It certainly helps protect against this class of issues that is for sure.
>
> It isn't a perfect defense though, as if the review isn't requested
> issues could slip past :(
>
> > 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.
>
> Something like KNAM would allow for a more refined ban yes (allowing
> QNAM in the repository that hosts KNAM, but blocking it elsewhere)
>
> >
> > Regards,
> > Volker
>
> Cheers,
> Ben