From nobody Sun Jul 16 15:33:58 2017 Return-Path: X-Original-To: codec@ietf.org Delivered-To: codec@ietfa.amsl.com Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 894FB12441E; Sun, 16 Jul 2017 15:33:57 -0700 (PDT) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit From: internet-drafts@ietf.org To: Cc: codec@ietf.org X-Test-IDTracker: no X-IETF-IDTracker: 6.56.0 Auto-Submitted: auto-generated Precedence: bulk Message-ID: <150024443753.32757.11397414381390098959@ietfa.amsl.com> Date: Sun, 16 Jul 2017 15:33:57 -0700 Archived-At: Subject: [codec] I-D Action: draft-ietf-codec-opus-update-07.txt X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Jul 2017 22:33:57 -0000 A New Internet-Draft is available from the on-line Internet-Drafts directories. This draft is a work item of the Internet Wideband Audio Codec of the IETF. Title : Updates to the Opus Audio Codec Authors : Jean-Marc Valin Koen Vos Filename : draft-ietf-codec-opus-update-07.txt Pages : 10 Date : 2017-07-16 Abstract: This document addresses minor issues that were found in the specification of the Opus audio codec in RFC 6716. The IETF datatracker status page for this draft is: https://datatracker.ietf.org/doc/draft-ietf-codec-opus-update/ There are also htmlized versions available at: https://tools.ietf.org/html/draft-ietf-codec-opus-update-07 https://datatracker.ietf.org/doc/html/draft-ietf-codec-opus-update-07 A diff from the previous version is available at: https://www.ietf.org/rfcdiff?url2=draft-ietf-codec-opus-update-07 Please note that it may take a couple of minutes from the time of submission until the htmlized version and diff are available at tools.ietf.org. Internet-Drafts are also available by anonymous FTP at: ftp://ftp.ietf.org/internet-drafts/ From nobody Tue Jul 25 15:34:18 2017 Return-Path: X-Original-To: codec@ietfa.amsl.com Delivered-To: codec@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 25391131FB2; Tue, 25 Jul 2017 15:34:17 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.881 X-Spam-Level: X-Spam-Status: No, score=-1.881 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.001, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8yBIFdGVSgiy; Tue, 25 Jul 2017 15:34:15 -0700 (PDT) Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 000BA131FB0; Tue, 25 Jul 2017 15:34:11 -0700 (PDT) Received: from [10.0.1.63] (cpe-66-25-7-22.tx.res.rr.com [66.25.7.22]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id v6PMYAxE084850 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 25 Jul 2017 17:34:11 -0500 (CDT) (envelope-from ben@nostrum.com) X-Authentication-Warning: raven.nostrum.com: Host cpe-66-25-7-22.tx.res.rr.com [66.25.7.22] claimed to be [10.0.1.63] From: Ben Campbell Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Message-Id: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> Date: Tue, 25 Jul 2017 17:34:10 -0500 Cc: codec@ietf.org To: draft-ietf-codec-opus-update.all@ietf.org X-Mailer: Apple Mail (2.3273) Archived-At: Subject: [codec] AD Evaluation of draft-ietf-codec-opus-update-07 X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jul 2017 22:34:17 -0000 Hi, This is my AD evaluation of draft-ietf-codec-opus-update-07. I have a = few material comments, and some nits. I=E2=80=99d like to address the = material comments prior to IETF last call. The nits can be handled now, = or later along with any IETF last call comments. Thanks! Ben. Material Comments: - Introduction: =E2=80=94 I don=E2=80=99t think we can expect IETF LC and IESG = reviewers to know the history of RFC 6716 going into this. Please expand = the first paragraph in the introduction to explain how the normative = part of RFC 6716 is in the form of attached C code in appendix A, and = that the patches in this draft patch that code. =E2=80=94 How can people be certain that the =E2=80=9Cformatted=E2=80=9D = patches match the patches in this draft, and do not change over time? = Would it make sense to include a hash in this document, simularly to how = 6716 has the hashes for the test vectors? (And are the IETF98 = proceedings really where you want to store these?) - 11: The draft should include hashes for the new test vectors, = similarly to in 6716. -12: The security considerations need some real content. For example, = several of the fixes here seem to fix potential buffer overruns or other = memory management errors. Does that reduce the attach surface for DoS or = other attacks? I suspect the answer is =E2=80=9Cno=E2=80=9D, since = several of those sections mention that the bugs don=E2=80=99t appear to = be exploitable. But the security considerations should at least = summarize these sorts of changes and say why you believe they have no = material impact on security. Nits: -3, last paragraph: Is no =E2=80=9Csignificant=E2=80=9D impact on test = vectors the same as no impact? Also, I note that none of the other patch = sections talk about test vector impact, and the section on new test = vectors explains which patches impact the vectors. Could the comment in = this section just be removed? -5, list items 2 and 3: These paragraphs (and others in the draft) = mention symbols from the code without any explanation what they mean. = Some of the symbol names are self-documenting, but that is not = consistently true. It would be helpful to add short comments about the = meaning of each of these (perhaps in parentheses). One might think of = this as similar to expanding acronyms on first use. Also, the use of underscores for emphasis is, to my knowledge, not = meaningful in the context of an RFC. Is the emphasis really needed? (I = assume you don=E2=80=99t want to wait for the new RFC format for this = sort of thing :-) ) =E2=80=94 paragraph after the list: "However, proving that is non- obvious.=E2=80=9D - Odd sentence structure. Would it make sense to = say =E2=80=9CHowever, the authors know of no obvious approach to proving = that.=E2=80=9D? -8: Please expand NaN on first mention. -9: Please expand LCG on first mention From nobody Tue Jul 25 21:09:19 2017 Return-Path: X-Original-To: codec@ietfa.amsl.com Delivered-To: codec@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CBDF6132019 for ; Tue, 25 Jul 2017 21:09:18 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.6 X-Spam-Level: X-Spam-Status: No, score=-2.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_LOW=-0.7] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=jmvalin-ca.20150623.gappssmtp.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sAZsCuFOhUwF for ; Tue, 25 Jul 2017 21:09:16 -0700 (PDT) Received: from mail-io0-x22f.google.com (mail-io0-x22f.google.com [IPv6:2607:f8b0:4001:c06::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 0FEB3129AD3 for ; Tue, 25 Jul 2017 21:09:16 -0700 (PDT) Received: by mail-io0-x22f.google.com with SMTP id m88so54644786iod.2 for ; Tue, 25 Jul 2017 21:09:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jmvalin-ca.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to; bh=BzZRNdGciWRpxz1Xtv2NjMHWsTlilABhLAJONPZyk90=; b=D2uCfB3BGlp7HtICq55EOcDPzSXnGiPDvEVbmpZJAgHKThbTyeNZiJ1ismmtzS2R4W WUCg8MOVHyr/kvyKG4N9LO/KgLd8TTSKG2xHNDnuDXc8qMbW6SUrX0MhbWa4fuI0vro3 K2ANZuNTkKR+STga0Ys6/L6Ow0LlcKrn/e6C+kU9vWqNx/RbOtGbqDsMqCooUdz46wFF ueO1Iy2P/0d4Zhq7XyryXq9E0r0s7ArGvBk6rU6UATrgGZs41LsUHJK6gnWumbBhqiWy aStxi2/X/IG+vlklG167YfR36UpoQKdtpujqnUjXFR3kYhSq4VpC7fEDmUCTce057eD/ aIOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=BzZRNdGciWRpxz1Xtv2NjMHWsTlilABhLAJONPZyk90=; b=UOFC84IJ1arQdn6/kfoRl8T1m86gv7S9uZ7FKX2D8iYKG6O53Yw1zop3tVyi3h0R4O rVyUloPQ3QMZPTQUN2e7ngz0OxSWHgAU7bPOiNH7/jE0RG5LNxN/vpyiFF8sXABDNSNF wUNKji6+HmTKTfcjJcBBOUwEPG5RgXPbcygGQx1e9Bc470qAxZL1ctPRRMz7XN52qsyy 7kJdanzjDRIpjQlqwIP3aLwFE37JrHwFcIoCOqiXcyLErWl9fg3Kh/IY08DC/1K2c9hK XN3xWZE/dWvj5PNzl0qka9DCnJva3zzoG4kZAGstDSNfyGeBVt+aAY9M7AJL9Adj700a fzoA== X-Gm-Message-State: AIVw113YwtlQHJ+/nqsaBa7JsSW1XiJE7A0E+4Sp9oIqz1WSWDyNDxcW EqikWeP3S9zM7sBCRdA= X-Received: by 10.107.171.193 with SMTP id u184mr15932347ioe.39.1501042155010; Tue, 25 Jul 2017 21:09:15 -0700 (PDT) Received: from panoramix.jmvalin.ca (modemcable067.31-56-74.mc.videotron.ca. [74.56.31.67]) by smtp.gmail.com with ESMTPSA id q133sm7798703ioe.58.2017.07.25.21.09.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 25 Jul 2017 21:09:13 -0700 (PDT) To: Ben Campbell , draft-ietf-codec-opus-update.all@ietf.org References: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> Cc: codec@ietf.org From: Jean-Marc Valin Message-ID: <3e689239-f217-2185-96e2-c6ae35b4d0f3@jmvalin.ca> Date: Wed, 26 Jul 2017 00:09:12 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> Content-Type: multipart/mixed; boundary="------------EE9B9E23FAE7DCA154593629" Archived-At: Subject: Re: [codec] AD Evaluation of draft-ietf-codec-opus-update-07 X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 04:09:19 -0000 This is a multi-part message in MIME format. --------------EE9B9E23FAE7DCA154593629 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hi Ben, Thanks for taking the time to review this document. I agree with all your comments and I attached the XML diff for the changes I'm proposing to address them. Let me know if you're happy with these changes and I'll submit an updated version. See inline for more details. On 25/07/17 06:34 PM, Ben Campbell wrote: > — I don’t think we can expect IETF LC and IESG reviewers to know the > history of RFC 6716 going into this. Please expand the first > paragraph in the introduction to explain how the normative part of > RFC 6716 is in the form of attached C code in appendix A, and that > the patches in this draft patch that code. This is the updated paragraph: This document addresses minor issues that were discovered in the reference implementation of the Opus codec. Unlike most IETF specifications, Opus is defined in RFC 6716 [RFC6716] in terms of a normative reference decoder implementation rather than from the associated text description. That's why only issues affecting the decoder are listed here. An up-to-date implementation of the Opus encoder can be found at . > — How can people be certain that the “formatted” patches match the > patches in this draft, and do not change over time? Would it make > sense to include a hash in this document, simularly to how 6716 has > the hashes for the test vectors? (And are the IETF98 proceedings > really where you want to store these?) I added the hash of the patch and each testvector like in RFC6716. As for putting the tar.gz in the proceedings, that's also what we did for RFC6716 and as far as I know there is still no better solution for publishing files at a fixed location. > - 11: The draft should include hashes for the new test vectors, > similarly to in 6716. Done (see above). > -12: The security considerations need some real content. For example, > several of the fixes here seem to fix potential buffer overruns or > other memory management errors. Does that reduce the attach surface > for DoS or other attacks? I suspect the answer is “no”, since several > of those sections mention that the bugs don’t appear to be > exploitable. But the security considerations should at least > summarize these sorts of changes and say why you believe they have no > material impact on security. I expanded the security considerations section to discuss the two issues that have potential security implications and that had associated CVEs. The new text is: This document fixes two security issues reported on Opus and that affect the reference implementation in RFC 6716 [RFC6716]: CVE- 2013-0899 and CVE-2017-0381. CVE-2013-0899 is fixed by Section 4 and could theoretically cause information leak, but the leaked information would at the very least go through the decoder process before being accessible to the attacker. Also, the bug can only be triggered by Opus packets at least 24 MB in size. CVE-2017-0381 is fixed by Section 7 as far as the authors are aware, could not be exploited in any way (despite the claims in the CVE) unless the read- only table was somehow placed very close to sensitive data, which is highly unlikely. Beyond the two fixed CVEs, this document adds no new security considerations on top of RFC 6716 [RFC6716]. > -3, last paragraph: Is no “significant” impact on test vectors the > same as no impact? Also, I note that none of the other patch sections > talk about test vector impact, and the section on new test vectors > explains which patches impact the vectors. Could the comment in this > section just be removed? Most of the changes do not affect test vectors at all, but I'm pointing out the few that do have an impact. I agree that "no significant impact" was a bit vague, so here's the updated text: This change affects the normative output of the decoder, but the amount of change is within the tolerance and too small to make the testvector check fail. > -5, list items 2 and 3: These paragraphs (and others in the draft) > mention symbols from the code without any explanation what they mean. > Some of the symbol names are self-documenting, but that is not > consistently true. It would be helpful to add short comments about > the meaning of each of these (perhaps in parentheses). One might > think of this as similar to expanding acronyms on first use. I added an explanation for nSamplesIn and fs_in_khZ. I thought the remaining ones were clear enough, but let me know if I missed any that would be useful. > Also, the use of underscores for emphasis is, to my knowledge, not > meaningful in the context of an RFC. Is the emphasis really needed? > (I assume you don’t want to wait for the new RFC format for this sort > of thing :-) ) Removed the underscores. > — paragraph after the list: "However, proving that is non- obvious.” > - Odd sentence structure. Would it make sense to say “However, the > authors know of no obvious approach to proving that.”? Changed to the sentence you suggested. > -8: Please expand NaN on first mention. Done (NaN = "not a number"). > -9: Please expand LCG on first mention Simply replaced "LCG noise" by "white noise" (the fact that it's generated by an LCG is irrelevant to the document). Cheers, Jean-Marc > > > > > _______________________________________________ codec mailing list > codec@ietf.org https://www.ietf.org/mailman/listinfo/codec > --------------EE9B9E23FAE7DCA154593629 Content-Type: text/x-patch; name="proposed_changes.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="proposed_changes.patch" diff --git a/doc/draft-ietf-codec-opus-update.xml b/doc/draft-ietf-codec-= opus-update.xml index ad0d569..a2419fe 100644 --- a/doc/draft-ietf-codec-opus-update.xml +++ b/doc/draft-ietf-codec-opus-update.xml @@ -58,8 +58,10 @@
This document addresses minor issues that were discovered in th= e reference - implementation of the Opus codec that serves as the specification = in - RFC 6716. Only issues affecting th= e decoder are + implementation of the Opus codec. Unlike most IETF specifications,= Opus is defined + in RFC 6716 in terms of a normativ= e reference + decoder implementation rather than from the associated text descri= ption. + That's why only issues affecting the decoder are listed here. An up-to-date implementation of the Opus encoder can = be found at . @@ -75,7 +77,8 @@ at the end of a line and the white space at the beginning of the following line are not part of the patch. A properly formatte= d patch including all changes is available at - . + + and has a SHA1 029e3aa88fc342c91e67a21e7bfbc9458661cd5f. =20
@@ -110,8 +113,8 @@ ]]> - This change affects the normative part of the decoder, although the= - amount of change is too small to make a significant impact on testv= ectors. + This change affects the normative output of the decoder, but the + amount of change is within the tolerance and too small to make the = testvector check fail. =20 @@ -146,7 +149,7 @@ This packet parsing issue is limited to reading memory up to about 60 kB beyond the compressed buffer. This can only be t= riggered by a compressed packet more than about 16 MB long, so it's not = a problem - for RTP. In theory, it could cras= h a file + for RTP. In theory, it could crash a file decoder (e.g. Opus in Ogg) if the memory just after the incomin= g packet is out-of-range, but our attempts to trigger such a crash in a = production application built using an affected version of the Opus decoder= failed. @@ -159,19 +162,19 @@ local buffer was opus_int16. Because the size was wrong, this potentially allowed the source and destination regions of the memcpy() to overlap. - We believe that nSamplesIn is at= least fs_in_khZ, + We believe that nSamplesIn (number of input samples) is at lea= st fs_in_khZ (sampling rate in kHz), which is at least 8. Since RESAMPLER_ORDER_FIR_12 is only 8, that should not be a prob= lem once the type size is fixed. The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, bu= t the - data stored in it was actually _twice_ the input batch size + data stored in it was actually twice the input batch size (nSamplesIn<<1). The fact that the code never produced any error in testing (includ= ing when run under the Valgrind memory debugger), suggests that in practice the batch sizes are reasonable enough that none of the issues above= - was ever a problem. However, proving that is non-obvious. + was ever a problem. However, the authors know of no obvious approac= h to proving that. The code can be fixed by applying the following changes to line 7= 8 of silk/resampler_private_IIR_FIR.c: @@ -266,7 +269,7 @@ rc_mult2 ), mult2Q); =20 -
+
It was discovered -- also from decoder fuzzing -- that an intege= r wrap-around could occur when decoding line spectral frequency coefficients from ex= treme bitstreams. @@ -294,7 +297,7 @@ silk_ADD_SAT16( NLSF_Q15[i-1], NDeltaMin_Q15[i] ) );
On extreme bit-streams, it is possible for log-domain band ener= gy levels to exceed the maximum single-precision floating point value once= converted - to a linear scale. This would later cause the decoded values to = be NaN, + to a linear scale. This would later cause the decoded values to = be NaN (not a number), possibly causing problems in the software using the PCM values. = This can be avoided with the following patch to line 552 of celt/quant_bands= =2Ec: @@ -318,7 +321,7 @@ silk_ADD_SAT16( NLSF_Q15[i-1], NDeltaMin_Q15[i] ) ); enough bits to code a single CELT band (8 - 9.6 kHz). When that = happens, the second band (CELT band 18, from 9.6 to 12 kHz) cannot use fo= lding because it is wider than the amount already coded, and falls bac= k to - LCG noise. Because it can also happen on transients (e.g. stops)= , it + white noise. Because it can also happen on transients (e.g. stop= s), it can cause audible pre-echo. @@ -424,11 +427,65 @@ effective_lowband+N); The new test vectors are located at . + The SHA1 hash of the test vectors are: +
+ + + +
+ Note that the decoder input bitstream files (.bit) are unchanged.
=20
- This document adds no new security considerations on top of + This document fixes two security issues reported on Opus and th= at affect the + reference implementation in RFC 6716: CVE-2013-0899 + and CVE-2017-0381. CVE-2013-0899 is fixed by and + could theoretically cause information leak, but the + leaked information would at the very least go through the decode= r process before + being accessible to the attacker. Also, the bug can only be trig= gered by Opus packets + at least 24 MB in size. CVE-2017-0381 is fixed by as far + as the authors are aware, could not be exploited in any way (des= pite the claims in + the CVE) unless the read-only table + was somehow placed very close to sensitive data, which is highly= unlikely. + Beyond the two fixed CVEs, this document adds no new security co= nsiderations on top of RFC 6716.
@@ -442,7 +499,8 @@ effective_lowband+N); =20
We would like to thank Juri Aedla for reporting the issue with = the parsing of - the Opus padding. Also, thanks to Jonathan Lennox and Mark Harris = for their + the Opus padding. Thanks to Felicia Lim for reporting the LSF inte= ger overflow issue. + Also, thanks to Tina le Grand, Jonathan Lennox, and Mark Harris fo= r their feedback on this document.
--------------EE9B9E23FAE7DCA154593629-- From nobody Tue Jul 25 21:41:07 2017 Return-Path: X-Original-To: codec@ietfa.amsl.com Delivered-To: codec@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 0779312708C; Tue, 25 Jul 2017 21:41:05 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.881 X-Spam-Level: X-Spam-Status: No, score=-1.881 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.001, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wly4xGCXRx8i; Tue, 25 Jul 2017 21:41:03 -0700 (PDT) Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 3E5C4124D68; Tue, 25 Jul 2017 21:41:03 -0700 (PDT) Received: from [10.0.1.63] (cpe-66-25-7-22.tx.res.rr.com [66.25.7.22]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id v6Q4f08w046040 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 25 Jul 2017 23:41:01 -0500 (CDT) (envelope-from ben@nostrum.com) X-Authentication-Warning: raven.nostrum.com: Host cpe-66-25-7-22.tx.res.rr.com [66.25.7.22] claimed to be [10.0.1.63] Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) From: Ben Campbell In-Reply-To: <3e689239-f217-2185-96e2-c6ae35b4d0f3@jmvalin.ca> Date: Tue, 25 Jul 2017 23:41:00 -0500 Cc: draft-ietf-codec-opus-update.all@ietf.org, codec@ietf.org Content-Transfer-Encoding: quoted-printable Message-Id: <15358143-0339-4B75-A5FD-010D465DA603@nostrum.com> References: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> <3e689239-f217-2185-96e2-c6ae35b4d0f3@jmvalin.ca> To: Jean-Marc Valin X-Mailer: Apple Mail (2.3273) Archived-At: Subject: Re: [codec] AD Evaluation of draft-ietf-codec-opus-update-07 X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 04:41:05 -0000 Thanks for the quick response. A few minor comments inline. I removed = sections that do not seem to need further discussion. Otherwise the proposed update looks good. Please feel free to submit a = new revision after resolving the comments below to your own = satisfaction. Thanks! Ben. > On Jul 25, 2017, at 11:09 PM, Jean-Marc Valin = wrote: >=20 > Hi Ben, >=20 > Thanks for taking the time to review this document. I agree with all > your comments and I attached the XML diff for the changes I'm = proposing > to address them. Let me know if you're happy with these changes and = I'll > submit an updated version. See inline for more details. >=20 > On 25/07/17 06:34 PM, Ben Campbell wrote: >> =E2=80=94 I don=E2=80=99t think we can expect IETF LC and IESG = reviewers to know the >> history of RFC 6716 going into this. Please expand the first >> paragraph in the introduction to explain how the normative part of >> RFC 6716 is in the form of attached C code in appendix A, and that >> the patches in this draft patch that code. >=20 > This is the updated paragraph: >=20 > This document addresses minor issues that were discovered in the > reference implementation of the Opus codec. Unlike most IETF > specifications, Opus is defined in RFC 6716 [RFC6716] in terms of a > normative reference decoder implementation rather than from the > associated text description. That's why only issues affecting the > decoder are listed here. An up-to-date implementation of the Opus > encoder can be found at . I suggest adding a sentence to the effect of the following after =E2=80=9C= =E2=80=A6 associated text description.=E2=80=9D: "That RFC includes the reference decoder implementation as Appendix A." >> -12: The security considerations need some real content. For example, >> several of the fixes here seem to fix potential buffer overruns or >> other memory management errors. Does that reduce the attach surface >> for DoS or other attacks? I suspect the answer is =E2=80=9Cno=E2=80=9D,= since several >> of those sections mention that the bugs don=E2=80=99t appear to be >> exploitable. But the security considerations should at least >> summarize these sorts of changes and say why you believe they have no >> material impact on security. >=20 > I expanded the security considerations section to discuss the two = issues > that have potential security implications and that had associated = CVEs. Thanks, this is exactly the sort of text I had in mind. I only have a = couple of nit comments, below: > The new text is: >=20 > This document fixes two security issues reported on Opus and that > affect the reference implementation in RFC 6716 [RFC6716]: CVE- > 2013-0899 and CVE-2017-0381. CVE-2013-0899 is fixed by Section 4 and > could theoretically cause information leak, but the leaked > information would at the very least go through the decoder process > before being accessible to the attacker. Also, the bug can only be > triggered by Opus packets at least 24 MB in size. CVE-2017-0381 is > fixed by Section 7 as far as the authors are aware, could not be Is there a missing word? It=E2=80=99s not clear if you mean to say that = as far as the authors are aware it is fixed, or as far as the authors = are aware it could not be exploited. > exploited in any way (despite the claims in the CVE) unless the read- > only table was somehow placed very close to sensitive data, which is > highly unlikely. Beyond the two fixed CVEs, this document adds no > new security considerations on top of RFC 6716 [RFC6716]. Can you add some context about the CVEs, such as where they are reported = and where they can be found? [=E2=80=A6] >> -5, list items 2 and 3: These paragraphs (and others in the draft) >> mention symbols from the code without any explanation what they mean. >> Some of the symbol names are self-documenting, but that is not >> consistently true. It would be helpful to add short comments about >> the meaning of each of these (perhaps in parentheses). One might >> think of this as similar to expanding acronyms on first use. >=20 > I added an explanation for nSamplesIn and fs_in_khZ. I thought the > remaining ones were clear enough, but let me know if I missed any that > would be useful. I think that=E2=80=99s fine. >=20 >> Also, the use of underscores for emphasis is, to my knowledge, not >> meaningful in the context of an RFC. Is the emphasis really needed? >> (I assume you don=E2=80=99t want to wait for the new RFC format for = this sort >> of thing :-) ) >=20 > Removed the underscores. So, as I looked at the XML diff, I realize the emphasis is added using = XML tags rather than by hand entering the underscores. So I may have = been incorrect to say they have no meaning in the context of an RFC :-) = I think the text is still better without them, but do not have strong = feelings if you prefer to keep them. [=E2=80=A6]= From nobody Tue Jul 25 22:01:26 2017 Return-Path: X-Original-To: codec@ietf.org Delivered-To: codec@ietfa.amsl.com Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 0C90F1243F3; Tue, 25 Jul 2017 22:01:24 -0700 (PDT) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit From: internet-drafts@ietf.org To: Cc: codec@ietf.org X-Test-IDTracker: no X-IETF-IDTracker: 6.57.0 Auto-Submitted: auto-generated Precedence: bulk Message-ID: <150104528401.26282.15351978639673072656@ietfa.amsl.com> Date: Tue, 25 Jul 2017 22:01:24 -0700 Archived-At: Subject: [codec] I-D Action: draft-ietf-codec-opus-update-08.txt X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 05:01:24 -0000 A New Internet-Draft is available from the on-line Internet-Drafts directories. This draft is a work item of the Internet Wideband Audio Codec WG of the IETF. Title : Updates to the Opus Audio Codec Authors : Jean-Marc Valin Koen Vos Filename : draft-ietf-codec-opus-update-08.txt Pages : 12 Date : 2017-07-25 Abstract: This document addresses minor issues that were found in the specification of the Opus audio codec in RFC 6716. The IETF datatracker status page for this draft is: https://datatracker.ietf.org/doc/draft-ietf-codec-opus-update/ There are also htmlized versions available at: https://tools.ietf.org/html/draft-ietf-codec-opus-update-08 https://datatracker.ietf.org/doc/html/draft-ietf-codec-opus-update-08 A diff from the previous version is available at: https://www.ietf.org/rfcdiff?url2=draft-ietf-codec-opus-update-08 Please note that it may take a couple of minutes from the time of submission until the htmlized version and diff are available at tools.ietf.org. Internet-Drafts are also available by anonymous FTP at: ftp://ftp.ietf.org/internet-drafts/ From nobody Tue Jul 25 22:09:09 2017 Return-Path: X-Original-To: codec@ietfa.amsl.com Delivered-To: codec@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 18BA8131E25 for ; Tue, 25 Jul 2017 22:09:08 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.9 X-Spam-Level: X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001] autolearn=unavailable autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=jmvalin-ca.20150623.gappssmtp.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DIZwYzYQ4Hye for ; Tue, 25 Jul 2017 22:09:05 -0700 (PDT) Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 87377124D68 for ; Tue, 25 Jul 2017 22:09:05 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id v205so44876598itf.1 for ; Tue, 25 Jul 2017 22:09:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jmvalin-ca.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=ELXWcFC3aEetKRDGyQcYFgvLPz5cP4K/kHjSzq1MPoM=; b=XulfFnBse1iiwfc7UX0SPATFu+j1V8p40pxO0YOzz2z5mhFO3CsoK9Gn9tqBa6Xh32 Gj/GvVOkJDq4x0scz36OKjm7DAcr+GwlcS0WTgduNVNy5j1AVxkrlB3AJBVs2QHfh37d l5lSs+RSjyZOBj+UQT4lfLcHvozSNO7DEJQCVXo239TyEDA4G66qIubhwa94WCR44b7X unduLrffFML39Zj19pc9eVlaPaWspPEGmj/3V7ukAmESLMxtn6Wd5MWwSpD3kpjj7R6F ZE5Tc7oGhsR7rJPFqkYZknIZCbtal/Up+muhhO6U27fx8shfSJMTYZyVsqQVsGYY9qZM U0Qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=ELXWcFC3aEetKRDGyQcYFgvLPz5cP4K/kHjSzq1MPoM=; b=B8kkhOsmSHecwUt8mjlUnkqwpVEHrFDZ/YS72gfxxMeavlWmU3+yFT9zjge1AkuZE/ MgnauNj1l+RH/YgdGP52SIwwWLgu1RWr5T6YRKctHxKo7SsCB7ccppV+JNLwFWblmeBx +r5Xe3qPPApBP757ilCrndV4CLE06kEHPDVYbnPCYb4ahPl7VDhEg+0SfivhTezP/Z4m GI7j1OaZDgfqHOYJAv2LHKX0Ia1WC4wRb749psdJXUpU3X+yMxcuqlqJyD/gt+o8Mleg lNc1WJt9PE07kqCaR3Q5xecqmfk9jfMlvtbs2SJ7KNn/qZUI2IzJEnZNpfomgT0w/BlA IuUQ== X-Gm-Message-State: AIVw1131o3ukOxMC+K7EswnpQFKRGAeBmgfTlwenGpV5SsRiKuoFCulN MiGQQSrwbtKTQDBBSQk= X-Received: by 10.36.160.4 with SMTP id o4mr13419617ite.157.1501045744691; Tue, 25 Jul 2017 22:09:04 -0700 (PDT) Received: from panoramix.jmvalin.ca (modemcable067.31-56-74.mc.videotron.ca. [74.56.31.67]) by smtp.gmail.com with ESMTPSA id b83sm5759028iod.35.2017.07.25.22.09.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 25 Jul 2017 22:09:04 -0700 (PDT) To: Ben Campbell References: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> <3e689239-f217-2185-96e2-c6ae35b4d0f3@jmvalin.ca> <15358143-0339-4B75-A5FD-010D465DA603@nostrum.com> Cc: draft-ietf-codec-opus-update.all@ietf.org, codec@ietf.org From: Jean-Marc Valin Message-ID: <747d9352-f3b0-56cd-0b2c-9945ba764178@jmvalin.ca> Date: Wed, 26 Jul 2017 01:09:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <15358143-0339-4B75-A5FD-010D465DA603@nostrum.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Archived-At: Subject: Re: [codec] AD Evaluation of draft-ietf-codec-opus-update-07 X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 05:09:08 -0000 Just submitted version -08 addressing your last set of comments. See below for details. On 26/07/17 12:41 AM, Ben Campbell wrote: > I suggest adding a sentence to the effect of the following after “… > associated text description.”: > > "That RFC includes the reference decoder implementation as Appendix > A." Done. >> This document fixes two security issues reported on Opus and that >> affect the reference implementation in RFC 6716 [RFC6716]: CVE- >> 2013-0899 and CVE-2017-0381. CVE-2013-0899 is fixed by Section 4 >> and could theoretically cause information leak, but the leaked >> information would at the very least go through the decoder process >> before being accessible to the attacker. Also, the bug can only >> be triggered by Opus packets at least 24 MB in size. CVE-2017-0381 >> is fixed by Section 7 as far as the authors are aware, could not >> be > > Is there a missing word? It’s not clear if you mean to say that as > far as the authors are aware it is fixed, or as far as the authors > are aware it could not be exploited. There was indeed a missing "and": CVE-2017-0381 is fixed by Section 7 and, as far as the authors are aware, could not be exploited in any way... > Can you add some context about the CVEs, such as where they are > reported and where they can be found? Added links to the CVEs > So, as I looked at the XML diff, I realize the emphasis is added > using XML tags rather than by hand entering the underscores. So I may > have been incorrect to say they have no meaning in the context of an > RFC :-) I think the text is still better without them, but do not > have strong feelings if you prefer to keep them. I agree that the underscores weren't adding much, so I'm leaving them out. Cheers, Jean-Marc From nobody Wed Jul 26 13:22:51 2017 Return-Path: X-Original-To: codec@ietfa.amsl.com Delivered-To: codec@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A31F213146C; Wed, 26 Jul 2017 13:22:49 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.881 X-Spam-Level: X-Spam-Status: No, score=-1.881 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.001, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Rqlvu7mNTbF0; Wed, 26 Jul 2017 13:22:48 -0700 (PDT) Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id DDBDC12EC46; Wed, 26 Jul 2017 13:22:47 -0700 (PDT) Received: from [10.0.1.63] (cpe-66-25-7-22.tx.res.rr.com [66.25.7.22]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id v6QKMkth065949 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 26 Jul 2017 15:22:47 -0500 (CDT) (envelope-from ben@nostrum.com) X-Authentication-Warning: raven.nostrum.com: Host cpe-66-25-7-22.tx.res.rr.com [66.25.7.22] claimed to be [10.0.1.63] Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) From: Ben Campbell In-Reply-To: <747d9352-f3b0-56cd-0b2c-9945ba764178@jmvalin.ca> Date: Wed, 26 Jul 2017 15:22:45 -0500 Cc: draft-ietf-codec-opus-update.all@ietf.org, codec@ietf.org Content-Transfer-Encoding: quoted-printable Message-Id: <1B21E198-2219-4831-861A-2F8939D3BD8D@nostrum.com> References: <44ADD827-E40E-4BBE-91DB-EFFC249AA10E@nostrum.com> <3e689239-f217-2185-96e2-c6ae35b4d0f3@jmvalin.ca> <15358143-0339-4B75-A5FD-010D465DA603@nostrum.com> <747d9352-f3b0-56cd-0b2c-9945ba764178@jmvalin.ca> To: Jean-Marc Valin X-Mailer: Apple Mail (2.3273) Archived-At: Subject: Re: [codec] AD Evaluation of draft-ietf-codec-opus-update-07 X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 20:22:50 -0000 Thanks! I requested IETF Last Call of version 08. Ben. > On Jul 26, 2017, at 12:09 AM, Jean-Marc Valin = wrote: >=20 > Just submitted version -08 addressing your last set of comments. See > below for details. >=20 > On 26/07/17 12:41 AM, Ben Campbell wrote: >> I suggest adding a sentence to the effect of the following after = =E2=80=9C=E2=80=A6 >> associated text description.=E2=80=9D: >>=20 >> "That RFC includes the reference decoder implementation as Appendix >> A." >=20 > Done. >=20 >>> This document fixes two security issues reported on Opus and that=20 >>> affect the reference implementation in RFC 6716 [RFC6716]: CVE-=20 >>> 2013-0899 and CVE-2017-0381. CVE-2013-0899 is fixed by Section 4 >>> and could theoretically cause information leak, but the leaked=20 >>> information would at the very least go through the decoder process=20= >>> before being accessible to the attacker. Also, the bug can only >>> be triggered by Opus packets at least 24 MB in size. CVE-2017-0381 >>> is fixed by Section 7 as far as the authors are aware, could not >>> be >>=20 >> Is there a missing word? It=E2=80=99s not clear if you mean to say = that as >> far as the authors are aware it is fixed, or as far as the authors >> are aware it could not be exploited. >=20 > There was indeed a missing "and": >=20 > CVE-2017-0381 is fixed by Section 7 and, as far as the authors > are aware, could not be exploited in any way... >=20 >> Can you add some context about the CVEs, such as where they are >> reported and where they can be found? >=20 > Added links to the CVEs >=20 >> So, as I looked at the XML diff, I realize the emphasis is added >> using XML tags rather than by hand entering the underscores. So I may >> have been incorrect to say they have no meaning in the context of an >> RFC :-) I think the text is still better without them, but do not >> have strong feelings if you prefer to keep them. >=20 > I agree that the underscores weren't adding much, so I'm leaving them = out. >=20 > Cheers, >=20 > Jean-Marc From nobody Wed Jul 26 14:48:13 2017 Return-Path: X-Original-To: codec@ietf.org Delivered-To: codec@ietfa.amsl.com Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 72ADB131D0B; Wed, 26 Jul 2017 14:48:03 -0700 (PDT) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit From: The IESG To: "IETF-Announce" X-Test-IDTracker: no X-IETF-IDTracker: 6.57.0 Auto-Submitted: auto-generated Precedence: bulk CC: ben@nostrum.com, codec@ietf.org, tterriberry@mozilla.com, Tim Terriberry , codec-chairs@ietf.org, draft-ietf-codec-opus-update@ietf.org Reply-To: ietf@ietf.org Sender: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-ID: <150110568337.20005.8076475579769854293.idtracker@ietfa.amsl.com> Date: Wed, 26 Jul 2017 14:48:03 -0700 Archived-At: Subject: [codec] Last Call: (Updates to the Opus Audio Codec) to Proposed Standard X-BeenThere: codec@ietf.org X-Mailman-Version: 2.1.22 List-Id: Codec WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 21:48:03 -0000 The IESG has received a request from the Internet Wideband Audio Codec WG (codec) to consider the following document: - 'Updates to the Opus Audio Codec' as Proposed Standard The IESG plans to make a decision in the next few weeks, and solicits final comments on this action. Please send substantive comments to the ietf@ietf.org mailing lists by 2017-08-09. Exceptionally, comments may be sent to iesg@ietf.org instead. In either case, please retain the beginning of the Subject line to allow automated sorting. Abstract This document addresses minor issues that were found in the specification of the Opus audio codec in RFC 6716. The file can be obtained via https://datatracker.ietf.org/doc/draft-ietf-codec-opus-update/ IESG discussion can be tracked via https://datatracker.ietf.org/doc/draft-ietf-codec-opus-update/ballot/ No IPR declarations have been submitted directly on this I-D.