Refactor Identity Hint handling

Don't call strncpy with strlen of the source as the length. Don't call
strlen multiple times. Eventually we will want to replace this with a proper
PACKET style handling (but for construction of PACKETs instead of just
reading them as it is now). For now though this is safe because
PSK_MAX_IDENTITY_LEN will always fit into the destination buffer.

This addresses an OCAP Audit issue.

Reviewed-by: Richard Levitte <levitte@openssl.org>

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 82fced51dc7e1d6711def9705a2d68ce600a8408..d38fc3a892df8cf177b3929a46f468c2590e20d4 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1830,10 +1830,19 @@ int tls_construct_server_key_exchange(SSL *s)
     if (type & SSL_PSK) {
         /* copy PSK identity hint */
         if (s->cert->psk_identity_hint) {
-            s2n(strlen(s->cert->psk_identity_hint), p);
-            strncpy((char *)p, s->cert->psk_identity_hint,
-                    strlen(s->cert->psk_identity_hint));
-            p += strlen(s->cert->psk_identity_hint);
+            size_t len = strlen(s->cert->psk_identity_hint);
+            if (len > PSK_MAX_IDENTITY_LEN) {
+                /*
+                 * Should not happen - we already checked this when we set
+                 * the identity hint
+                 */
+                SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
+                       ERR_R_INTERNAL_ERROR);
+                goto err;
+            }
+            s2n(len, p);
+            memcpy(p, s->cert->psk_identity_hint, len);
+            p += len;
         } else {
             s2n(0, p);
         }