[openssl-dev] DTLS_BAD_VER regression fixes for 1.0.2 and HEAD

[David Woodhouse]

On Tue, 2015-03-03 at 14:43 +0000, Matt Caswell wrote:
> 
> On 03/03/15 14:28, David Woodhouse wrote:
> > On Tue, 2015-03-03 at 12:00 +0000, Matt Caswell wrote:
> >>
> >>> I'll look at adding test cases to exercise the DTLS_BAD_VER support,
> >> to
> >>> try to avoid this kind of thing happening in future.
> >>>
> >>
> >> That would be fantastic to have.
> > 
> > I look a quick look at this. Adding DTLSv1 and DTLSv1.2 support to
> > ssl/ssltest.c isn't particularly hard,
> 
> If you fancy taking on that task, that would be really useful just in
> itself.

This works for 'ssltest -dtls1' but not with -bio_pair for some reason.
I got this far before concluding it wasn't going to be a helpful
approach...

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index c535a42..e550704 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2749,6 +2749,12 @@ const char *SSL_get_version(const SSL *s)
         return ("TLSv1");
     else if (s->version == SSL3_VERSION)
         return ("SSLv3");
+    else if (s->version == DTLS1_BAD_VER)
+        return ("DTLSv0.9");
+    else if (s->version == DTLS1_VERSION)
+        return ("DTLSv1");
+    else if (s->version == DTLS1_2_VERSION)
+        return ("DTLSv1.2");
     else
         return ("unknown");
 }
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index 89fb44a..511f674 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -787,6 +787,10 @@ static void sv_usage(void)
 #ifndef OPENSSL_NO_SSL3_METHOD
     fprintf(stderr, " -ssl3         - use SSLv3\n");
 #endif
+#ifndef OPENSSL_NO_DTLS
+    fprintf(stderr, " -dtls1        - use DTLSv1\n");
+    fprintf(stderr, " -dtls12       - use DTLSv1.2\n");
+#endif
     fprintf(stderr, " -tls1         - use TLSv1\n");
     fprintf(stderr, " -CApath arg   - PEM format directory of CA's\n");
     fprintf(stderr, " -CAfile arg   - PEM format file of CA's\n");
@@ -958,7 +962,7 @@ int main(int argc, char *argv[])
     int badop = 0;
     int bio_pair = 0;
     int force = 0;
-    int tls1 = 0, ssl3 = 0, ret = 1;
+    int dtls1 = 0, dtls12 = 0, tls1 = 0, ssl3 = 0, ret = 1;
     int client_auth = 0;
     int server_auth = 0, i;
     struct app_verify_arg app_verify_arg =
@@ -1136,6 +1140,16 @@ int main(int argc, char *argv[])
             no_protocol = 1;
 #endif
             ssl3 = 1;
+        } else if (strcmp(*argv, "-dtls1") == 0) {
+#ifdef OPENSSL_NO_DTLS
+            no_protocol = 1;
+#endif
+            dtls1 = 1;
+        } else if (strcmp(*argv, "-dtls12") == 0) {
+#ifdef OPENSSL_NO_DTLS
+            no_protocol = 1;
+#endif
+            dtls12 = 1;
         } else if (strncmp(*argv, "-num", 4) == 0) {
             if (--argc < 1)
                 goto bad;
@@ -1309,8 +1323,8 @@ int main(int argc, char *argv[])
         goto end;
     }
 
-    if (ssl3 + tls1 > 1) {
-        fprintf(stderr, "At most one of -ssl3, or -tls1 should "
+    if (ssl3 + tls1 + dtls1 + dtls12 > 1) {
+        fprintf(stderr, "At most one of -ssl3, -tls1, -dtls1 or -dtls12should "
                 "be requested.\n");
         EXIT(1);
     }
@@ -1327,10 +1341,10 @@ int main(int argc, char *argv[])
         goto end;
     }
 
-    if (!ssl3 && !tls1 && number > 1 && !reuse && !force) {
+    if (!ssl3 && !tls1 && !dtls1 && !dtls12 && number > 1 && !reuse && !force) {
         fprintf(stderr, "This case cannot work.  Use -f to perform "
                 "the test anyway (and\n-d to see what happens), "
-                "or add one of -ssl3, -tls1, -reuse\n"
+                "or add one of -ssl3, -tls1, -dtls1, dtls12, -reuse\n"
                 "to avoid protocol mismatch.\n");
         EXIT(1);
     }
@@ -1403,6 +1417,13 @@ int main(int argc, char *argv[])
         meth = SSLv3_method();
     else
 #endif
+#ifndef OPENSSL_NO_DTLS
+    if (dtls1)
+        meth = DTLSv1_method();
+    else if (dtls12)
+        meth = DTLSv1_2_method();
+    else
+#endif
     if (tls1)
         meth = TLSv1_method();
     else



> > So I'm torn between doing a minimal reimplementation of the server side
> > and making OpenSSL talk to that, or a dirty replay attack such as the
> > one I had when I was first working it out:
> > http://david.woodhou.se/dtls-test.c
> > 
> The minimal reimplementation sounds like it might be the more flexible
> base to work from (and could even be the basis for future DTLSv1/1.2
> tests). But it also sounds like quite a bit more work to me. Either way,
> having *some* tests has got to be a lot better than *no* tests like we
> have now!

Well, the evil "override RAND_bytes() and replay" trick isn't actually
working any more. I think I need to disable all extensions so the
ClientHello precisely matches the one in my capture, or redo a capture
with the options that we currently send.... which all in all is probably
a fairly strong hint that I ought not to be doing the replay thing :)

-- 
dwmw2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20150303/50f1111c/attachment-0001.bin>