Re: Gnutls investigation and request for advice for Jessie
On 2018-08-31 21:30:14, Ola Lundqvist wrote:
> Hi Antoine
>
> Thank you for the input this is valuable. I have some comments below.
>
> On Fri, 31 Aug 2018 at 21:03, Antoine Beaupré <anarcat@orangeseeds.org> wrote:
>>
>> On 2018-08-31 13:29:29, Ola Lundqvist wrote:
>> > Hi all LTS contributors
>> >
>> > My question is whether removing default ciphers and introducing new
>> > options is acceptable so late in the release cyckle. My assumption is
>> > no, but let me know if you have another opinion. More details below.
>>
>> A priori, I think it is if it fixes serious security vulnerabilities and
>> there is no easier way to do so.
>
> I'm not so sure this is a serious issue as it is only exploitable in a
> rather uncommon case, that is when Encrypt-then-MAC (RFC7366) is
> unsupported by the peer.
> Most software do have that support as I understand it.
That's the crucial part, I guess. :) I am not sure.
[...]
>> > Upstream do in fact not even plan to do this as the problem only occur
>> > in the following cases:
>> > - CBC cipher
>> > - If Encrypt-then-MAC (RFC7366) is unsupported by the peer
>> >
>> > Instead upstream have done the following:
>> > 1) Introduced a new option to enforce Encrypt-then-MAC (for CVE-2018-10846)
>> > 2) Do a fix with SHA384 (for CVE-2018-10845)
>> > 3) Remove SHA256 and SHA384 from the default MAC ciphers (for
>> > CVE-2018-10844 and CVE-2018-10845)
>> > 4) A try to fix the problem (CVE-2018-10844 and potentially CVE-2018-10845)
>> >
>> > 1: I do not think we can do 1 such late in the release cycle. I mean
>> > new options now would be rather pointless. I will mark CVE-2018-10846
>> > as ignored due to this. Or do you think new options is ok also in
>> > Jessie?
>>
>> I looked at the upstream patch (MR#657) for this and it doesn't look
>> that invasive. According to GitLab, it is "21 files with 738 additions
>> and 148 deletions", which seems fairly small considering the scope of
>> the change:
>>
>> https://gitlab.com/gnutls/gnutls/merge_requests/657.patch
>
> I agree on that. But introducing an option will not help in most cases
> that use gnutls. I mean gnutls is used in most cases as a library and
> unless the other software is changed then there is no use of
> introducing the option.
That's true! I'm not sure how to deal with that... I guess it means
library users need to do the right thing then? Unless they mess around
with cipher lists the way charybdis does?
[...]
>> This, again, is also part of the larger fix in MR#657.
>>
>> Combined, those last two patches make about +- 50 lines of diff,
>> compared to the 700+ lines of diff for the larger merge request. But do
>> consider that a large part of MR#657 (+400 lines) is the introduction of
>> tests/tls-force-etm.c, a standalone file which shouldn't cause conflicts
>> at the very least.
>>
>> > I think I should do 2 and 4, but not 1 and 3.
>> >
>> > What do you think? If I do not hear any objections I'll do so.
>>
>> I would give it a shot and try to backport the whole thing. I would also
>> carefully look at the 3.3.x series to see if there's an official commit
>> shipping those. At first glance, it does look like a release was made on
>> all branches, including a 3.3.30 release that bundles some of those
>> patches.
>
> But not the default cipher removal, right?
Actually, looking at the NEWS file, they *did* remove the ciphers from
the defaults list as well.
>> In fact, I'd be tempted to seriously consider upgrading to that upstream
>> release after comparing our changelog with theirs to see if there's
>> anything missing either side...
>
> That is another way. I'm not so comfortable with doing that
> considering that I broke some software I did that with a library
> package. I do not remember what library it was now but some browser
> did not work after that.
You mean the NSS library and #843624? :) That was a hairy issue and it
affected an unsupported package (chromium). I don't think you should
stop from working on difficult package because of one difficulty. If
anything, that was a learning experience and you are now *more*
qualified to deal with such packages now. ;)
If you are unsure about updates, upload a test package somewhere and ask
people for feedback. I do it all the time and it actually works, if you
give people time (sometimes a week or more). For an update as critical,
it would certainly be warranted.
>> It seems that upstream did the right thing here and backported a bunch
>> of stuff for us already - it'd be too bad to waste that effort and skip
>> those CVEs. ;)
>
> :-D on the other hand maybe we break backwards compatibility. If it
> wasn't for the backwards compatibility I would not have asked. :-)
Yeah, I guess I don't know what's in those updates exactly, that's a
good point.
> Again thank you for the input.
Glad to be of service!
A.
Reply to: