Bug#807698: srtp: CVE-2015-6360

2016-04-01 Thread John Foley
Yes, that fix should address the vulnerability.  There was one Cisco 
product that reported an issue with this patch.  Specifically, it 
prevents a zero length payload packet from being decrypted.  We never 
received reports of this problem from downstream open source projects.  
So you're probably safe with only applying this patch. But we did 
augment the patch after the Cisco issue to address the zero payload 
decryption problem.



On 03/31/2016 04:21 PM, Markus Koschany wrote:

Control: severity -1 grave
Control: tags -1 patch

Am 31.03.2016 um 15:14 schrieb John Foley:

It's my understanding the obsolete versions of libsrtp are vulnerable.
Quoting the original text from Randell Jesup...

 srtp_unprotect (netwerk\srtp\src\srtp\srtp.c) can experience an
 integer underflow. If it does, it calls a decryption function with a
 buffer pointer pointing to memory to which it has no right, and with
 a very large buffer length. This call could scramble large portions
 of memory, causing incorrect and possibly insecure behavior.

 The bug is in this code:

 950: err_status_t
 951: srtp_unprotect(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len) {
 ...
 1073:   if (stream->rtp_services & sec_serv_conf) {
 1074: enc_start = (uint32_t *)hdr + uint32s_in_rtp_header + hdr->cc;
 1075: if (hdr->x == 1) {
 1076:   srtp_hdr_xtnd_t *xtn_hdr = (srtp_hdr_xtnd_t *)enc_start;
 1077:   enc_start += (ntohs(xtn_hdr->length) + 1);
 1078: }
 1079: enc_octet_len = (uint32_t)(*pkt_octet_len - tag_len
 1080: - ((enc_start - (uint32_t *)hdr) << 2));
 1081:   } else {
 1082: enc_start = NULL;
 1083:   }


Thanks for your quick response and clarification. If I understand
correctly we can basically apply the same patch for our version in
Wheezy and Jessie and guard against the potential integer underflow by using

if (!((uint8_t*)enc_start < (uint8_t*)hdr + (*pkt_octet_len - tag_len)))
return err_status_parse_err;

before

enc_octet_len = (uint32_t)(*pkt_octet_len - tag_len
- ((enc_start - (uint32_t *)hdr) << 2));

Since it is clear now that Jessie and Sid are affected, I am going to
raise the severity to grave again. Please find attached my proposed
debdiffs.

Regards,

Markus






Bug#763887: ITP: libest -- Enrollment over Secure Transport reference implementation

2014-10-03 Thread John Foley
Package: wnpp
Severity: wishlist
Owner: John Foley 

* Package name: libest
  Version : 1.1.0p
  Upstream Author : John Foley 
* URL : https://github.com/cisco/libest
* License : BSD
  Programming Lang: C
  Description : Enrollment over Secure Transport reference
implementation (RFC 7030)

RFC 7030 defines Enrollment over Secure Transport (EST).  EST is used to
securely provision X.509 certificates.  libest provides both a client-side
and server-side implementation of EST.  There are currently no upstream
packages dependent on libest.  However, libest provides both a library
and a command line utility.  The CLI utility can be used as a
stand-alone application to provision certificates on a Linux host.
EST is a new protocol. It is expected that upstream projects will
integrate libest in the future.  I am not aware of any existing Debian
packages that provide support for EST.

I plan to be the package maintainer and am currently looking for a sponsor.

Thank you.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#628583: [Srtp-development] bus error on sparc

2013-07-19 Thread John Foley
Hi Daniel,

Thanks for the guest access to your sparc 64 system.  The problem with
stat_driver is intermittent.  The attached patch appears to resolve the
problem, can you confirm using your view?  It looks like the nonce value
is never initialized to zero.  This intermittently results in garbage
being used for the nonce value.

Thanks,
John


On 07/19/2013 05:16 AM, Daniel Pocock wrote:
> On 19/07/13 01:39, John Foley wrote:
>> With respect to the master branch, you should be able to avoid the patch
>> and compile with -DFORCE_64BIT_ALIGN.  The patch provides no value in
>> the master branch. 
> The version I am testing is the Debian git master, it is the same as
> master on github but with some local patches:
>
> http://anonscm.debian.org/gitweb/?p=collab-maint/srtp.git;a=tree;f=debian/patches;hb=HEAD
>
> Without FORCE_64BIT_ALIGN  and without the patch mentioned below (patch
> ID 2002 in Debian): I get the bus error in the test
>
> crypto/tests/cipher_driver
>
>
>
> With FORCE_64BIT_ALIGN  and without the patch mentioned below (patch ID
> 2002 in Debian): no more bus error, it fails in the test
>
> test/stat_driver  (although running the test a second time it passes)
>
>
> With FORCE_64BIT_ALIGN  and patch ID 2002 (remove pad, add algorithm to 
> cipher_t) the tests all pass on the first run
>
> If it helps, we can provide a guest account on the Debian sparc test machine 
> if you would be able to try this directly.
>
>
>> I'll need to address this problem in the openssl-feature branch.  There
>> should be no need to force the 64-bit alignment when the cipher_t struct
>> contains the 'algorithm' field. 
>>
>> Regarding x86_64, I have tested this platform and have never seen an
>> issue in this code.  The compiler is likely doing the alignment
>> automatically, knowing that it's compiling for a 64 bit target. 
>>
>>
>>
>> On 07/18/2013 06:02 PM, Daniel Pocock wrote:
>>> On 18/07/13 23:46, Michael Jerris wrote:
>>>> If you don't remove that block from the code after that other var was 
>>>> added… it will cause this error to come back on that branch now that 
>>>> you've forced the 64bit align
>>>>
>>> Ok, patching it like this:
>>>
>>> --- a/crypto/include/cipher.h
>>> +++ b/crypto/include/cipher.h
>>> @@ -161,8 +161,9 @@ typedef struct cipher_t {
>>>void  *state;
>>>intkey_len;
>>>  #ifdef FORCE_64BIT_ALIGN
>>> -  intpad;
>>> +  //intpad;
>>>  #endif
>>> +  int algorithm;
>>>  } cipher_t;
>>>
>>>
>>> and using
>>>
>>> CFLAGS += -DFORCE_64BIT_ALIGN
>>>
>>> gives me an successful runtest on sparc first time around.  The build
>>> still fails later during the doc phase on sparc, but that is unrelated
>>> to all this.
>>>
>>> I also tested on amd64 / x86_64 without FORCE_64BIT_ALIGN and it builds
>>> successfully.
>>>
>>> .
>>>
> .
>

diff --git a/crypto/test/stat_driver.c b/crypto/test/stat_driver.c
index f9d75b7..145cd02 100644
--- a/crypto/test/stat_driver.c
+++ b/crypto/test/stat_driver.c
@@ -51,6 +51,7 @@ main (int argc, char *argv[]) {
 
   printf("statistical tests driver\n");
 
+  v128_set_to_zero(&nonce);
   for (i=0; i < 2500; i++)
 buffer[i] = 0;



Bug#628583: [Srtp-development] bus error on sparc

2013-07-19 Thread John Foley
Yes, a guest account on your sparc system would be helpful.  Your
observations are curious.  I'm interested learning what's happening here.


On 07/19/2013 05:16 AM, Daniel Pocock wrote:
> and patch ID 2002

<>

Bug#628583: [Srtp-development] bus error on sparc

2013-07-18 Thread John Foley
With respect to the master branch, you should be able to avoid the patch
and compile with -DFORCE_64BIT_ALIGN.  The patch provides no value in
the master branch. 

I'll need to address this problem in the openssl-feature branch.  There
should be no need to force the 64-bit alignment when the cipher_t struct
contains the 'algorithm' field. 

Regarding x86_64, I have tested this platform and have never seen an
issue in this code.  The compiler is likely doing the alignment
automatically, knowing that it's compiling for a 64 bit target. 



On 07/18/2013 06:02 PM, Daniel Pocock wrote:
>
> On 18/07/13 23:46, Michael Jerris wrote:
>> If you don't remove that block from the code after that other var was added… 
>> it will cause this error to come back on that branch now that you've forced 
>> the 64bit align
>>
> Ok, patching it like this:
>
> --- a/crypto/include/cipher.h
> +++ b/crypto/include/cipher.h
> @@ -161,8 +161,9 @@ typedef struct cipher_t {
>void  *state;
>intkey_len;
>  #ifdef FORCE_64BIT_ALIGN
> -  intpad;
> +  //intpad;
>  #endif
> +  int algorithm;
>  } cipher_t;
>
>
> and using
>
> CFLAGS += -DFORCE_64BIT_ALIGN
>
> gives me an successful runtest on sparc first time around.  The build
> still fails later during the doc phase on sparc, but that is unrelated
> to all this.
>
> I also tested on amd64 / x86_64 without FORCE_64BIT_ALIGN and it builds
> successfully.
>
> .
>

<>

Bug#628583: [Srtp-development] bus error on sparc

2013-07-18 Thread John Foley
You're using the legacy crypto implementation with that configuration. 
OpenSSL is not in the picture.

Looking at the diffs between master and feature-openssl, there's not
much jumping out at me that would have resolved the problem.  There are
some changes to the Makefile.  The only other difference that may impact
alignment issues is the delta in crypto/include/cipher.h.  Here's the delta:

@@ -163,6 +163,7 @@ typedef err_status_t (*cipher_set_iv_func_t)
#ifdef FORCE_64BIT_ALIGN
   intpad;
 #endif
+  int algorithm;
 } cipher_t;
 
Maybe your compiler has laid-out this struct differently due to this new
member.  You might try applying this modification to master to see if
that resolves the problem.  If not, then I would suspect something in
the Makefile deltas.




On 07/18/2013 02:36 PM, Daniel Pocock wrote:
> On 18/07/13 20:17, John Foley wrote:
>> I should clarify this better.  The legacy crypto is still in the
>> branch.  It's pulled out depending on how you've configured the
>> library.  Can you provide the ./configure options you used to build
>> the library?
>
> I just ran
>
>   ./configure && make runtest
>
>
> This is my host
> Linux smetana 2.6.32-5-sparc64-smp #1 SMP Mon Feb 25 02:19:08 UTC 2013
> sparc64 GNU/Linux
>
> On master, the first test case fails, it is the same error output from
> Debian bug 628583, but they all run on the branch:
>
> Build done. Please run 'make runtest' to run self tests.
> running libsrtp test applications...
> crypto/test/cipher_driver -v >/dev/null
> crypto/test/kernel_driver -v >/dev/null
> test/rdbx_driver -v >/dev/null
> test/srtp_driver -v >/dev/null
> test/roc_driver -v >/dev/null
> test/replay_driver -v >/dev/null
> test/dtls_srtp_driver >/dev/null
> cd test; /home/pocock/ws/srtp/srtp-git/test/rtpw_test.sh >/dev/null   
> ./rtpw: couldn't open file /usr/share/dict/words
> /home/pocock/ws/srtp/srtp-git/test/rtpw_test.sh: 64: kill: No such process
>
> libsrtp test applications passed.
> make -C crypto runtest
> make[1]: Entering directory `/home/pocock/ws/srtp/srtp-git/crypto'
> test/env # print out information on the build environment
> CPU set to big-endian(WORDS_BIGENDIAN == 1)
> CPU set to RISC(CPU_RISC == 1)
> using native 64-bit type(NO_64_BIT_MATH == 0)
> using stdout for error reporting(ERR_REPORTING_STDOUT == 1)
> using /dev/urandom as a random source(DEV_URANDOM == /dev/urandom)
> running crypto test applications...
> test `test/aes_calc 000102030405060708090a0b0c0d0e0f
> 00112233445566778899aabbccddeeff` = 69c4e0d86a7b0430d8cdb78070b4c55a
> test `test/aes_calc
> 000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f
> 00112233445566778899aabbccddeeff` = 8ea2b7ca516745bfeafc49904b496089
> test/cipher_driver -v >/dev/null
> test/datatypes_driver -v >/dev/null
> test/stat_driver >/dev/null
> test/sha1_driver -v >/dev/null
> test/kernel_driver -v >/dev/null
> test/rand_gen -n 256 >/dev/null
> crypto test applications passed.
>
>
>
>
>
>
>>
>>
>> On 07/18/2013 02:12 PM, John Foley wrote:
>>> Which test case was failing?  It's possible the test case is no
>>> longer included in the feature-openssl branch.  I pulled out all the
>>> legacy crypto and math from libsrtp in that branch.   Have you
>>> confirmed the failing test case is still run under the
>>> feature-openssl branch?
>>>
>>>
>>> On 07/18/2013 01:42 PM, Daniel Pocock wrote:
>>>>
>>>> Further observation: the feature-openssl branch from git does not
>>>> have the bus error, test cases run successfully on SPARC
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 18/07/13 19:34, Daniel Pocock wrote:
>>>>> On 18/07/13 17:26, John Foley wrote:
>>>>>> We've seen BUS errors on some platforms.  I'm not confident the
>>>>>> following patch was ever pushed back to libsrtp.  There's a chance this
>>>>>> may resolve the problem on sparc.  Unfortunately I don't have a sparc
>>>>>> system to try this myself.
>>>>>
>>>>>
>>>>> Thanks for this feedback
>>>>>
>>>>> The patch doesn't apply - all but one hunk fails
>>>>>
>>>>> I tried it against the Debian source package and I also tried
>>>>> applying it against the repository
>>>>>
>>>>> https://github.com/cisco/libsrtp
>>>>>
>>>>> Can you tel

Bug#628583: [Srtp-development] bus error on sparc

2013-07-18 Thread John Foley
Which test case was failing?  It's possible the test case is no longer
included in the feature-openssl branch.  I pulled out all the legacy
crypto and math from libsrtp in that branch.   Have you confirmed the
failing test case is still run under the feature-openssl branch?


On 07/18/2013 01:42 PM, Daniel Pocock wrote:
>
> Further observation: the feature-openssl branch from git does not have
> the bus error, test cases run successfully on SPARC
>
>
>
>
>
> On 18/07/13 19:34, Daniel Pocock wrote:
>> On 18/07/13 17:26, John Foley wrote:
>>> We've seen BUS errors on some platforms.  I'm not confident the
>>> following patch was ever pushed back to libsrtp.  There's a chance this
>>> may resolve the problem on sparc.  Unfortunately I don't have a sparc
>>> system to try this myself.
>>
>>
>> Thanks for this feedback
>>
>> The patch doesn't apply - all but one hunk fails
>>
>> I tried it against the Debian source package and I also tried
>> applying it against the repository
>>
>> https://github.com/cisco/libsrtp
>>
>> Can you tell me the SVN URL where you got this and I can try checking
>> it out and building it?
>>
>>
>>
>>
>>> Modified: branches/proto/libsrtp_30/srtp/include/srtp.h
>>>  ===
>>>  --- branches/proto/libsrtp_30/srtp/include/srtp.h  2013-04-24 19:44:23 UTC 
>>> (rev 1292)
>>>  +++ branches/proto/libsrtp_30/srtp/include/srtp.h  2013-04-29 14:17:03 UTC 
>>> (rev 1293)
>>>  @@ -52,6 +52,11 @@
>>>   
>>>   #ifdef _MSC_VER
>>>   #pragma pack(4)
>>>  +#define PACK
>>>  +#elif defined(__GNUC__)
>>>  +#define PACK __attribute__ ((packed))
>>>  +#else
>>>  +#define PACK
>>>   #endif
>>>   
>>>   #include "crypto_kernel.h"
>>>  
>>>  Modified: branches/proto/libsrtp_30/srtp/include/srtp_priv.h
>>>  ===
>>>  --- branches/proto/libsrtp_30/srtp/include/srtp_priv.h 2013-04-24 
>>> 19:44:23 UTC (rev 1292)
>>>  +++ branches/proto/libsrtp_30/srtp/include/srtp_priv.h 2013-04-29 
>>> 14:17:03 UTC (rev 1293)
>>>  @@ -68,7 +68,7 @@
>>>* fully pack the bit fields.
>>>*/
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   unsigned char cc : 4;  /* CSRC count */
>>>   unsigned char x : 1;   /* header extension flag  */
>>>   unsigned char p : 1;   /* padding flag   */
>>>  @@ -82,7 +82,7 @@
>>>   
>>>   #else /*  BIG_ENDIAN */
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   unsigned char version : 2; /* protocol version*/
>>>   unsigned char p : 1;   /* padding flag   */
>>>   unsigned char x : 1;   /* header extension flag  */
>>>  @@ -96,7 +96,7 @@
>>>   
>>>   #endif
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   uint16_t profile_specific;  /* profile-specific info   */
>>>   uint16_t length;/* number of 32-bit words in extension */
>>>   } srtp_hdr_xtnd_t;
>>>  @@ -111,7 +111,7 @@
>>>   
>>>   #ifndef WORDS_BIGENDIAN
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   unsigned char rc : 5;   /* reception report count */
>>>   unsigned char p : 1;/* padding flag   */
>>>   unsigned char version : 2;  /* protocol version   */
>>>  @@ -120,7 +120,7 @@
>>>   uint32_t ssrc;  /* synchronization source */
>>>   } srtcp_hdr_t;
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   unsigned int index : 31; /* srtcp packet index in network order! */
>>>   unsigned int e : 1;  /* encrypted? 1=yes */
>>>   /* optional mikey/etc go here */
>>>  @@ -130,7 +130,7 @@
>>>   
>>>   #else /*  BIG_ENDIAN */
>>>   
>>>  -typedef struct {
>>>  +typedef struct PACK {
>>>   unsigned char version : 2;  /* protocol version   */
>>>   unsigned char p : 1;/* padding flag   */
>>>   unsigned char rc : 5;   /* reception report count */
>>>  @@ -139,7 +139,7 @@
>>>   uint32_t ssrc;  /* 

Bug#628583: [Srtp-development] bus error on sparc

2013-07-18 Thread John Foley
I should clarify this better.  The legacy crypto is still in the
branch.  It's pulled out depending on how you've configured the
library.  Can you provide the ./configure options you used to build the
library?


On 07/18/2013 02:12 PM, John Foley wrote:
> Which test case was failing?  It's possible the test case is no longer
> included in the feature-openssl branch.  I pulled out all the legacy
> crypto and math from libsrtp in that branch.   Have you confirmed the
> failing test case is still run under the feature-openssl branch?
>
>
> On 07/18/2013 01:42 PM, Daniel Pocock wrote:
>>
>> Further observation: the feature-openssl branch from git does not
>> have the bus error, test cases run successfully on SPARC
>>
>>
>>
>>
>>
>> On 18/07/13 19:34, Daniel Pocock wrote:
>>> On 18/07/13 17:26, John Foley wrote:
>>>> We've seen BUS errors on some platforms.  I'm not confident the
>>>> following patch was ever pushed back to libsrtp.  There's a chance this
>>>> may resolve the problem on sparc.  Unfortunately I don't have a sparc
>>>> system to try this myself.
>>>
>>>
>>> Thanks for this feedback
>>>
>>> The patch doesn't apply - all but one hunk fails
>>>
>>> I tried it against the Debian source package and I also tried
>>> applying it against the repository
>>>
>>> https://github.com/cisco/libsrtp
>>>
>>> Can you tell me the SVN URL where you got this and I can try
>>> checking it out and building it?
>>>
>>>
>>>
>>>
>>>> Modified: branches/proto/libsrtp_30/srtp/include/srtp.h
>>>>  ===
>>>>  --- branches/proto/libsrtp_30/srtp/include/srtp.h 2013-04-24 19:44:23 UTC 
>>>> (rev 1292)
>>>>  +++ branches/proto/libsrtp_30/srtp/include/srtp.h 2013-04-29 14:17:03 UTC 
>>>> (rev 1293)
>>>>  @@ -52,6 +52,11 @@
>>>>   
>>>>   #ifdef _MSC_VER
>>>>   #pragma pack(4)
>>>>  +#define PACK
>>>>  +#elif defined(__GNUC__)
>>>>  +#define PACK __attribute__ ((packed))
>>>>  +#else
>>>>  +#define PACK
>>>>   #endif
>>>>   
>>>>   #include "crypto_kernel.h"
>>>>  
>>>>  Modified: branches/proto/libsrtp_30/srtp/include/srtp_priv.h
>>>>  ===
>>>>  --- branches/proto/libsrtp_30/srtp/include/srtp_priv.h2013-04-24 
>>>> 19:44:23 UTC (rev 1292)
>>>>  +++ branches/proto/libsrtp_30/srtp/include/srtp_priv.h2013-04-29 
>>>> 14:17:03 UTC (rev 1293)
>>>>  @@ -68,7 +68,7 @@
>>>>* fully pack the bit fields.
>>>>*/
>>>>   
>>>>  -typedef struct {
>>>>  +typedef struct PACK {
>>>>   unsigned char cc : 4;  /* CSRC count */
>>>>   unsigned char x : 1;   /* header extension flag  */
>>>>   unsigned char p : 1;   /* padding flag   */
>>>>  @@ -82,7 +82,7 @@
>>>>   
>>>>   #else /*  BIG_ENDIAN */
>>>>   
>>>>  -typedef struct {
>>>>  +typedef struct PACK {
>>>>   unsigned char version : 2; /* protocol version*/
>>>>   unsigned char p : 1;   /* padding flag   */
>>>>   unsigned char x : 1;   /* header extension flag  */
>>>>  @@ -96,7 +96,7 @@
>>>>   
>>>>   #endif
>>>>   
>>>>  -typedef struct {
>>>>  +typedef struct PACK {
>>>>   uint16_t profile_specific;  /* profile-specific info   */
>>>>   uint16_t length;/* number of 32-bit words in extension */
>>>>   } srtp_hdr_xtnd_t;
>>>>  @@ -111,7 +111,7 @@
>>>>   
>>>>   #ifndef WORDS_BIGENDIAN
>>>>   
>>>>  -typedef struct {
>>>>  +typedef struct PACK {
>>>>   unsigned char rc : 5;   /* reception report count */
>>>>   unsigned char p : 1;/* padding flag   */
>>>>   unsigned char version : 2;  /* protocol version   */
>>>>  @@ -120,7 +120,7 @@
>>>>   uint32_t ssrc;  /* synchronization source */
>>>>   } srtcp_hdr_t;
>>>>   
>>>>  -typedef struct {
>>>>  +typedef struct PACK {
>>&g