[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: ntp security update



On Sun, 2015-10-25 at 11:19 +0100, Kurt Roeckx wrote:
> On Sun, Oct 25, 2015 at 01:30:18PM +0900, Ben Hutchings wrote:
> > I've looked through the upstream repository for the patches that fix he
> > recently announced issues.  Quite a few of them turned out not to apply
> > to squeeze, or the newer stable releases, and I've updated the security
> > tracker accordingly.
> > 
> > I backported the remaining fixes as best I can, and uploaded the source
> > package to:
> > https://people.debian.org/~benh/packages/squeeze-lts/
> > 
> > Would you be willing to review this package?
> > 
> > I noticed that you entirely reverted the upstream patch that was
> > supposed to fix CVE-2015-7704 and -7705, and then applied a different
> > fix for -7704.  I think this means -7705 isn't fixed in sid, though the
> > security tracker currently says it is.  Who's right?
> 
> I can't seem to ge getting much information out of anything from
> upstream.  Lots of things don't seem to be affecting the 4.2.6
> version.
>
> From what I currently understand the following don't apply to the
> 4.2.6 versions:
> CVE-2015-5196
[...]
> So it seems they renamed CVE-2015-5196 to CVE-2015-7703.  Your
> patch probably makes sense and I should get that fixed in jessie
> and wheezy too.
> 
> I'm just wondering why you didn't move the T_Pidfile like upstream
> did, that part seems to apply.

Not in squeeze; there aren't any separate parsing rules for local and
remote.

> Your bug-2899.patch patch looks a little different.  You have:
> @@ -2207,8 +2221,8 @@ crypto_bob(
>        vp->sig = emalloc(sign_siglen);
>        EVP_SignInit(&ctx, sign_digest);
>        EVP_SignUpdate(&ctx, (u_char *)&vp->tstamp, 12);
> -      EVP_SignUpdate(&ctx, vp->ptr, vallen);
> -      if (EVP_SignFinal(&ctx, vp->sig, &vallen, sign_pkey))
> +      EVP_SignUpdate(&ctx, vp->ptr, len);
> +      if (EVP_SignFinal(&ctx, vp->sig, &len, sign_pkey))
>                vp->siglen = htonl(sign_siglen);
>        return (XEVNT_OK);
>  }
> 
> The patch from upstream and the one from redhat has:
> @@ -2214,9 +2228,9 @@ crypto_bob(
>         vp->sig = emalloc(sign_siglen);
>         EVP_SignInit(&ctx, sign_digest);
>         EVP_SignUpdate(&ctx, (u_char *)&vp->tstamp, 12);
> -       EVP_SignUpdate(&ctx, vp->ptr, vallen);
> -       if (EVP_SignFinal(&ctx, vp->sig, &vallen, sign_pkey))
> -               vp->siglen = htonl(sign_siglen);
> +       EVP_SignUpdate(&ctx, vp->ptr, len);
> +       if (EVP_SignFinal(&ctx, vp->sig, &len, sign_pkey))
> +               vp->siglen = htonl(len);
>         return (XEVNT_OK);
>  }
> 
> 
> As in, the htonl() call changes sign_siglen to len.

No, it changes vallen to len.  But in 4.2.6 vallen is ignored and the
previously calculated sign_siglen is assumed to be correct.  I didn't
want to change that.

> Your CVE-2015-7850 patch, in mvsyslog() calls vsnprintf() while
> mine calls mvsnprintf().

Because mvsnprintf() doesn't exist in the squeeze version, and anyway
it's a wrapper that's only needed to make %m work on some C libraries
other than glibc.

> You somehow seems to have the patches applied, you have a .pc
> directory in it ...

That's dpkg-source's doing.

> So you applied the following patches:
> CVE-2015-5300.patch
> bug-2899.patch (CVE-2015-7691, CVE-2015-7692, CVE-2015-7702)
> CVE-2015-7701.patch
> CVE-2015-7703.patch
> CVE-2015-7704.patch
> CVE-2015-7850.patch
> CVE-2015-7852.patch
> CVE-2015-7855.patch
> CVE-2015-7871.patch
> 
> While I have addiotional patches for:
> CVE-2014-9750.patch (it was missing 1 patch while it was fixed it
> seems)

Which is split from CVE-2014-9297.

> ntp-4.2.6p5-cve-2015-5219.patch
> ntp-4.2.6p5-cve-2015-5195.patch
> ntp-4.2.6p5-cve-2015-5194.patch
> ntp-4.2.6p5-cve-2015-5146.patch

These were already marked as no-DSA-required in the security tracker.

> CVE-2015-7705.patch

Where does this come from?

> CVE-2015-7851.patch

VMS only, so I didn't bother.

> CVE-2015-7853.patch

This really isn't needed because 4.2.6 doesn't have the incorrect cast
from size_t to int.  Please revert your change in the security tracker.

> Which leaves, which I think really don't affect 4.2.6:
> CVE-2015-7848
> CVE-2015-7849
> CVE-2015-7854

Ben.

-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: