On 2018-06-07 22:51:07, Moritz Muehlenhoff wrote:
> On Thu, Jun 07, 2018 at 08:08:06AM -0400, Antoine Beaupré wrote:
>> On 2018-06-07 04:45:06, Chris Lamb wrote:
>> > Hi Antoine,
>> >
>> >> A peculiar thing with the patchset is that it adds the --debug flag to
>> >> the test suite: I don't know why, but it's the only way to make it pass
>> >> the (new) test-http-permissions.t tests. Otherwise it just hangs there
>> >> forever.
>> >
>> > Personally, it would make me very very hesitant to propose a patch
>> > (yet alone release it via security update) when I had not discovered
>> > the reason for this.
>> >
>> > Bear in mind that the cause may not be in the testsuite and could be
>> > in the actual run-time code itself — ie. the improved testsuite is
>> > simply exposing it, just as it should.
>> 
>> The problem is the testsuite is forward-ported from wheezy, which itself
>> is backported from i don't remember where.
>
> Never do that! If the fix e.g. requires changes in jessie which were stripped
> from the wheezy backport as the affected code isn't present there, you
> end up with an incomplete security update.

The patch that was forward-ported is the testsuite base, not the actual
security fix. It was imported to have *something* to patch in jessie,
otherwise the testsuite controls introduced to test for the security
issue are not present at all in 3.x.

And yes, this could have been imported straight from upstream's 4.0 or
4.6, but I am not sure what that would bring as an improvement. The
point is to backport the security patch, and I believe this was actually
done correctly.

> Any backport always needs to be done invidually per release from the upstream 
> patch.

The debdiff add three patches:

 * CVE-2017-17458-1-80d7dbda9294.patch: introduces
   tests/test-audit-subrepo.t so the upstream security patch applies,
   forward-ported from wheezy

 * CVE-2017-17458-2-071cbeba4212.patch: actually fixes CVE-2017-17458,
   comparable with upstream patch, except for a chunk in the test suite
   that does not apply because missing from jessie. I've reviewed it and
   it compares well to the upstream patch, which is:

   https://www.mercurial-scm.org/repo/hg/raw-rev/071cbeba4212

 * CVE-2017-9462-77eaf9539499.patch: fix for CVE-2017-9462. I've
   reviewed it and it compares well with the upstream patch, which is:

   https://www.mercurial-scm.org/repo/hg/raw-rev/77eaf9539499

So while *one* of the three patches was forward-ported from wheezy, the
actual security fixes were actually backported from upstream. Or if they
were backported from wheezy (I honestly don't clearly remember anymore),
I have just reviewed the patches to make sure they include all the
fixes proposed by upstream.

Sounds better?

A.

PS: since I worked on the package, it looks like three more (CVE-less,
for now) issues came up, so I guess I'll keep on updating mercurial a
bit longer before an upload eh? :)

-- 
I'm no longer accepting the things I cannot change.
I'm changing the things I cannot accept.
                        - Angela Davis

Reply via email to