Submitted by: Eric Rescorla <ekr@rtfm.com>

Fix encoding of use_srtp extension to be compliant with RFC5764

diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index c6fd68d6db7ee1db689e37044a7774761c4f4847..e9e6f5a67ea81c0d973a51092fe976000d5df644 100644 (file)
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -278,19 +278,25 @@ int ssl_add_clienthello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int max
                        return 1;
                        }
 
-               if((ct*2) > maxlen)
+               if((2 + ct*2 + 1) > maxlen)
                        {
                        SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_USE_SRTP_EXT,SSL_R_SRTP_PROTECTION_PROFILE_LIST_TOO_LONG);
                        return 1;
                        }
 
+                /* Add the length */
+                s2n(ct * 2, p);
                for(i=0;i<ct;i++)
                        {
                        prof=sk_SRTP_PROTECTION_PROFILE_value(clnt,i);
                        s2n(prof->id,p);
                        }
+
+                /* Add an empty use_mki value */
+                *p++ = 0;
                }
-       *len=ct*2;
+
+       *len=2 + ct*2 + 1;
     
        return 0;
        }
@@ -300,23 +306,48 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
        {
        SRTP_PROTECTION_PROFILE *cprof,*sprof;
        STACK_OF(SRTP_PROTECTION_PROFILE) *clnt=0,*srvr;
+        int ct;
+        int mki_len;
        int i,j;
        int id;
        int ret;
-    
-       if(len%2)
+
+         /* Length value + the MKI length */
+        if(len < 3)
+               {            
+               SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
+               *al=SSL_AD_DECODE_ERROR;
+               return 1;
+                }
+
+        /* Pull off the length of the cipher suite list */
+        n2s(d, ct);
+        len -= 2;
+        
+        /* Check that it is even */
+       if(ct%2)
+               {
+               SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
+               *al=SSL_AD_DECODE_ERROR;
+               return 1;
+               }
+        
+        /* Check that lengths are consistent */
+       if(len < (ct + 1)) 
                {
                SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
                *al=SSL_AD_DECODE_ERROR;
                return 1;
                }
 
+        
        clnt=sk_SRTP_PROTECTION_PROFILE_new_null();
 
-       while(len)
+       while(ct)
                {
                n2s(d,id);
-               len-=2;
+               ct-=2;
+                len-=2;
 
                if(!find_profile_by_num(id,&cprof))
                        {
@@ -328,6 +359,17 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
                        }
                }
 
+        /* Now extract the MKI value as a sanity check, but discard it for now */
+        mki_len = *d;
+        d++; len--;
+
+        if (mki_len != len)
+               {
+               SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_MKI_VALUE);
+               *al=SSL_AD_DECODE_ERROR;
+               return 1;
+               }
+
        srvr=SSL_get_srtp_profiles(s);
 
        /* Pick our most preferred profile. If no profiles have been
@@ -364,7 +406,7 @@ int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int max
        {
        if(p)
                {
-               if(maxlen < 2)
+               if(maxlen < 3)
                        {
                        SSLerr(SSL_F_SSL_ADD_SERVERHELLO_USE_SRTP_EXT,SSL_R_SRTP_PROTECTION_PROFILE_LIST_TOO_LONG);
                        return 1;
@@ -377,8 +419,9 @@ int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int max
                        }
 
                s2n(s->srtp_profile->id,p);
-               }    
-       *len=2;
+                *p++ = 0;
+               }
+       *len=3;
     
        return 0;
        }
@@ -391,7 +434,7 @@ int ssl_parse_serverhello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
        STACK_OF(SRTP_PROTECTION_PROFILE) *clnt;
        SRTP_PROTECTION_PROFILE *prof;
 
-       if(len!=2)
+       if(len!=3)
                {
                SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
                *al=SSL_AD_DECODE_ERROR;
@@ -399,6 +442,12 @@ int ssl_parse_serverhello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
                }
 
        n2s(d,id);
+        if (*d)  /* Must be no MKI, since we never offer one */
+               {
+               SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_USE_SRTP_EXT,SSL_R_BAD_SRTP_MKI_VALUE);
+               *al=SSL_AD_ILLEGAL_PARAMETER;
+               return 1;
+               }
 
        clnt=SSL_get_srtp_profiles(s);
 

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 3d027af3cca3ee30929089edfb7194cad2430388..5ef1aeb4825f266d38835f6341ff0a697b8ea55b 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2343,6 +2343,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_BAD_SRP_G_LENGTH                          350
 #define SSL_R_BAD_SRP_N_LENGTH                          351
 #define SSL_R_BAD_SRP_S_LENGTH                          352
+#define SSL_R_BAD_SRTP_MKI_VALUE                        371
 #define SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST          360
 #define SSL_R_BAD_SSL_FILETYPE                          124
 #define SSL_R_BAD_SSL_SESSION_ID_LENGTH                         125

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index e94ffe0141471fde4c8809a4fd3a086c66032591..6a8091b0f0692ffb71f9423bbfaaae7ba0c85931 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -330,6 +330,7 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {ERR_REASON(SSL_R_BAD_SRP_G_LENGTH)      ,"bad srp g length"},
 {ERR_REASON(SSL_R_BAD_SRP_N_LENGTH)      ,"bad srp n length"},
 {ERR_REASON(SSL_R_BAD_SRP_S_LENGTH)      ,"bad srp s length"},
+{ERR_REASON(SSL_R_BAD_SRTP_MKI_VALUE)    ,"bad srtp mki value"},
 {ERR_REASON(SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST),"bad srtp protection profile list"},
 {ERR_REASON(SSL_R_BAD_SSL_FILETYPE)      ,"bad ssl filetype"},
 {ERR_REASON(SSL_R_BAD_SSL_SESSION_ID_LENGTH),"bad ssl session id length"},