Clean up SSL_export_keying_material implementation.

Fix up the variable names. Also avoid the messy logic of checking whether the
label and context collide with the normal key expansion ones in the face of
adverserial inputs. Make that the caller's responsibility, just as it's already
the caller's responsibility to ensure that different calls don't overlap.  (The
label should be a constant string in an IANA registry anyway.)

Change-Id: I062fadb7b6a18fa946b883be660ea9b3f0f6277c
Reviewed-on: https://boringssl-review.googlesource.com/4216
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/tls1.h

@@ -298,16 +298,16 @@ extern "C" {
 
 OPENSSL_EXPORT const char *SSL_get_servername(const SSL *s, const int type);
 OPENSSL_EXPORT int SSL_get_servername_type(const SSL *s);
-/* SSL_export_keying_material exports a value derived from the master secret,
- * as specified in RFC 5705. It writes |olen| bytes to |out| given a label and
+
+/* SSL_export_keying_material exports a value derived from the master secret, as
+ * specified in RFC 5705. It writes |out_len| bytes to |out| given a label and
  * optional context. (Since a zero length context is allowed, the |use_context|
  * flag controls whether a context is included.)
  *
- * It returns 1 on success and zero otherwise. */
-OPENSSL_EXPORT int SSL_export_keying_material(SSL *s, uint8_t *out, size_t olen,
-                                              const char *label, size_t llen,
-                                              const uint8_t *p, size_t plen,
-                                              int use_context);
+ * It returns one on success and zero otherwise. */
+OPENSSL_EXPORT int SSL_export_keying_material(
+    SSL *s, uint8_t *out, size_t out_len, const char *label, size_t label_len,
+    const uint8_t *context, size_t context_len, int use_context);
 
 OPENSSL_EXPORT int SSL_get_sigalgs(SSL *s, int idx, int *psign, int *phash,
                                    int *psignandhash, uint8_t *rsig,

ssl/ssl_lib.c

@@ -1705,15 +1705,16 @@ void SSL_get0_alpn_selected(const SSL *ssl, const uint8_t **data,
   }
 }
 
-int SSL_export_keying_material(SSL *s, uint8_t *out, size_t olen,
-                               const char *label, size_t llen, const uint8_t *p,
-                               size_t plen, int use_context) {
+int SSL_export_keying_material(SSL *s, uint8_t *out, size_t out_len,
+                               const char *label, size_t label_len,
+                               const uint8_t *context, size_t context_len,
+                               int use_context) {
   if (s->version < TLS1_VERSION) {
     return 0;
   }
 
-  return s->enc_method->export_keying_material(s, out, olen, label, llen, p,
-                                               plen, use_context);
+  return s->enc_method->export_keying_material(
+      s, out, out_len, label, label_len, context, context_len, use_context);
 }
 
 static uint32_t ssl_session_hash(const SSL_SESSION *a) {

ssl/ssl_locl.h

@@ -893,9 +893,10 @@ int tls1_final_finish_mac(SSL *s, const char *str, int slen, uint8_t *p);
 int tls1_cert_verify_mac(SSL *s, int md_nid, uint8_t *p);
 int tls1_generate_master_secret(SSL *s, uint8_t *out, const uint8_t *premaster,
                                 size_t premaster_len);
-int tls1_export_keying_material(SSL *s, uint8_t *out, size_t olen,
-                                const char *label, size_t llen,
-                                const uint8_t *p, size_t plen, int use_context);
+int tls1_export_keying_material(SSL *s, uint8_t *out, size_t out_len,
+                                const char *label, size_t label_len,
+                                const uint8_t *context, size_t context_len,
+                                int use_context);
 int tls1_alert_code(int code);
 int ssl3_alert_code(int code);
 int ssl_ok(SSL *s);

ssl/t1_enc.c

@@ -865,82 +865,36 @@ int tls1_generate_master_secret(SSL *s, uint8_t *out, const uint8_t *premaster,
   return SSL3_MASTER_SECRET_SIZE;
 }
 
-int tls1_export_keying_material(SSL *s, uint8_t *out, size_t olen,
-                                const char *label, size_t llen,
-                                const uint8_t *context, size_t contextlen,
+int tls1_export_keying_material(SSL *s, uint8_t *out, size_t out_len,
+                                const char *label, size_t label_len,
+                                const uint8_t *context, size_t context_len,
                                 int use_context) {
-  uint8_t *val = NULL;
-  size_t vallen, currentvalpos;
-  int ret;
-
-  /* construct PRF arguments we construct the PRF argument ourself rather than
-   * passing separate values into the TLS PRF to ensure that the concatenation
-   * of values does not create a prohibited label. */
-  vallen = llen + SSL3_RANDOM_SIZE * 2;
+  size_t seed_len = 2 * SSL3_RANDOM_SIZE;
   if (use_context) {
-    vallen += 2 + contextlen;
-  }
-
-  val = OPENSSL_malloc(vallen);
-  if (val == NULL) {
-    goto err2;
-  }
-
-  currentvalpos = 0;
-  memcpy(val + currentvalpos, (uint8_t *)label, llen);
-  currentvalpos += llen;
-  memcpy(val + currentvalpos, s->s3->client_random, SSL3_RANDOM_SIZE);
-  currentvalpos += SSL3_RANDOM_SIZE;
-  memcpy(val + currentvalpos, s->s3->server_random, SSL3_RANDOM_SIZE);
-  currentvalpos += SSL3_RANDOM_SIZE;
-
-  if (use_context) {
-    val[currentvalpos] = (contextlen >> 8) & 0xff;
-    currentvalpos++;
-    val[currentvalpos] = contextlen & 0xff;
-    currentvalpos++;
-    if (contextlen > 0 || context != NULL) {
-      memcpy(val + currentvalpos, context, contextlen);
+    if (context_len >= 1u << 16) {
+      OPENSSL_PUT_ERROR(SSL, tls1_export_keying_material, ERR_R_OVERFLOW);
+      return 0;
     }
+    seed_len += 2 + context_len;
   }
-
-  /* disallow prohibited labels note that SSL3_RANDOM_SIZE > max(prohibited
-   * label len) = 15, so size of val > max(prohibited label len) = 15 and the
-   * comparisons won't have buffer overflow. */
-  if (memcmp(val, TLS_MD_CLIENT_FINISH_CONST,
-             TLS_MD_CLIENT_FINISH_CONST_SIZE) == 0 ||
-      memcmp(val, TLS_MD_SERVER_FINISH_CONST,
-             TLS_MD_SERVER_FINISH_CONST_SIZE) == 0 ||
-      memcmp(val, TLS_MD_MASTER_SECRET_CONST,
-             TLS_MD_MASTER_SECRET_CONST_SIZE) == 0 ||
-      memcmp(val, TLS_MD_KEY_EXPANSION_CONST,
-             TLS_MD_KEY_EXPANSION_CONST_SIZE) == 0) {
-    goto err1;
+  uint8_t *seed = OPENSSL_malloc(seed_len);
+  if (seed == NULL) {
+    OPENSSL_PUT_ERROR(SSL, tls1_export_keying_material, ERR_R_MALLOC_FAILURE);
+    return 0;
   }
 
-  /* SSL_export_keying_material is not implemented for SSLv3, so passing
-   * everything through the label parameter works. */
-  assert(s->version != SSL3_VERSION);
-  ret = s->enc_method->prf(s, out, olen, s->session->master_key,
-                           s->session->master_key_length, (const char *)val,
-                           vallen, NULL, 0, NULL, 0);
-  goto out;
-
-err1:
-  OPENSSL_PUT_ERROR(SSL, tls1_export_keying_material,
-                    SSL_R_TLS_ILLEGAL_EXPORTER_LABEL);
-  ret = 0;
-  goto out;
-
-err2:
-  OPENSSL_PUT_ERROR(SSL, tls1_export_keying_material, ERR_R_MALLOC_FAILURE);
-  ret = 0;
-
-out:
-  if (val != NULL) {
-    OPENSSL_free(val);
+  memcpy(seed, s->s3->client_random, SSL3_RANDOM_SIZE);
+  memcpy(seed + SSL3_RANDOM_SIZE, s->s3->server_random, SSL3_RANDOM_SIZE);
+  if (use_context) {
+    seed[2 * SSL3_RANDOM_SIZE] = (uint8_t)(context_len >> 8);
+    seed[2 * SSL3_RANDOM_SIZE + 1] = (uint8_t)context_len;
+    memcpy(seed + 2 * SSL3_RANDOM_SIZE + 2, context, context_len);
   }
 
+  int ret = s->enc_method->prf(s, out, out_len, s->session->master_key,
+                               s->session->master_key_length, label, label_len,
+                               seed, seed_len, NULL, 0);
+  OPENSSL_free(seed);
   return ret;
 }