Remove an ugly hack in the client certificate verification code that works

around broken GOST implementations.  It looks like client certificates with
GOST have been completely broken since reimport of the GOST code, so no-one
is using LibreSSL this way.  The client side was fixed only last week for
TLSv1.0 and TLSv1.1.  This workaround is now in the way of much needed
simplifcation and cleanup, so it is time for it to go.

suggested by and ok jsing

diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 0667ac8..80199d3 100644 (file)
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.61 2018/11/21 15:13:29 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.62 2018/12/07 07:22:09 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2149,55 +2149,53 @@ ssl3_get_cert_verify(SSL *s)
                goto f_err;
        }
 
-       /*
-        * Check for broken implementations of GOST ciphersuites.
-        *
-        * If key is GOST and n is exactly 64, it is a bare
-        * signature without length field.
-        */
-       /* This hack is awful and needs to die in fire */
-       if ((pkey->type == NID_id_GostR3410_94 ||
-            pkey->type == NID_id_GostR3410_2001) && CBS_len(&cbs) == 64) {
-               if (SSL_USE_SIGALGS(s))
-                       goto truncated;
-               CBS_dup(&cbs, &signature);
-               if (!CBS_skip(&cbs, CBS_len(&cbs)))
-                       goto err;
-       } else {
-               if (SSL_USE_SIGALGS(s)) {
-                       uint16_t sigalg_value;
-
-                       if (!CBS_get_u16(&cbs, &sigalg_value))
-                               goto truncated;
-                       if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
-                           tls12_sigalgs_len)) == NULL ||
-                           (md = sigalg->md()) == NULL) {
-                               SSLerror(s, SSL_R_UNKNOWN_DIGEST);
-                               al = SSL_AD_DECODE_ERROR;
-                               goto f_err;
-                       }
-                       if (!ssl_sigalg_pkey_ok(sigalg, pkey)) {
-                               SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
-                               al = SSL_AD_DECODE_ERROR;
-                               goto f_err;
-                       }
-               }
+       if (!SSL_USE_SIGALGS(s)) {
                if (!CBS_get_u16_length_prefixed(&cbs, &signature))
                        goto err;
-       }
-       if (CBS_len(&signature) > EVP_PKEY_size(pkey)) {
-               SSLerror(s, SSL_R_WRONG_SIGNATURE_SIZE);
-               al = SSL_AD_DECODE_ERROR;
-               goto f_err;
-       }
-       if (CBS_len(&cbs) != 0) {
-               al = SSL_AD_DECODE_ERROR;
-               SSLerror(s, SSL_R_EXTRA_DATA_IN_MESSAGE);
-               goto f_err;
+               if (CBS_len(&signature) > EVP_PKEY_size(pkey)) {
+                       SSLerror(s, SSL_R_WRONG_SIGNATURE_SIZE);
+                       al = SSL_AD_DECODE_ERROR;
+                       goto f_err;
+               }
+               if (CBS_len(&cbs) != 0) {
+                       al = SSL_AD_DECODE_ERROR;
+                       SSLerror(s, SSL_R_EXTRA_DATA_IN_MESSAGE);
+                       goto f_err;
+               }
        }
 
        if (SSL_USE_SIGALGS(s)) {
                EVP_PKEY_CTX *pctx;
+               uint16_t sigalg_value;
+
+               if (!CBS_get_u16(&cbs, &sigalg_value))
+                       goto truncated;
+               if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
+                   tls12_sigalgs_len)) == NULL ||
+                   (md = sigalg->md()) == NULL) {
+                       SSLerror(s, SSL_R_UNKNOWN_DIGEST);
+                       al = SSL_AD_DECODE_ERROR;
+                       goto f_err;
+               }
+               if (!ssl_sigalg_pkey_ok(sigalg, pkey)) {
+                       SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
+                       al = SSL_AD_DECODE_ERROR;
+                       goto f_err;
+               }
+
+               if (!CBS_get_u16_length_prefixed(&cbs, &signature))
+                       goto err;
+               if (CBS_len(&signature) > EVP_PKEY_size(pkey)) {
+                       SSLerror(s, SSL_R_WRONG_SIGNATURE_SIZE);
+                       al = SSL_AD_DECODE_ERROR;
+                       goto f_err;
+               }
+               if (CBS_len(&cbs) != 0) {
+                       al = SSL_AD_DECODE_ERROR;
+                       SSLerror(s, SSL_R_EXTRA_DATA_IN_MESSAGE);
+                       goto f_err;
+               }
+
                if (!tls1_transcript_data(s, &hdata, &hdatalen)) {
                        SSLerror(s, ERR_R_INTERNAL_ERROR);
                        al = SSL_AD_INTERNAL_ERROR;
@@ -2250,9 +2248,8 @@ ssl3_get_cert_verify(SSL *s)
                        SSLerror(s, SSL_R_BAD_ECDSA_SIGNATURE);
                        goto f_err;
                }
-       } else
 #ifndef OPENSSL_NO_GOST
-       if (pkey->type == NID_id_GostR3410_94 ||
+       } else if (pkey->type == NID_id_GostR3410_94 ||
            pkey->type == NID_id_GostR3410_2001) {
                unsigned char sigbuf[128];
                unsigned int siglen = sizeof(sigbuf);
@@ -2297,9 +2294,8 @@ ssl3_get_cert_verify(SSL *s)
                }
 
                EVP_PKEY_CTX_free(pctx);
-       } else
 #endif
-       {
+       } else {
                SSLerror(s, ERR_R_INTERNAL_ERROR);
                al = SSL_AD_UNSUPPORTED_CERTIFICATE;
                goto f_err;