BUG/MINOR: ssl: fix memcpy overlap without consequences.
A trick is used to set SESSION_ID, and SESSION_ID_CONTEXT lengths
to 0 and avoid ASN1 encoding of these values.
There is no specific function to set the length of those parameters
to 0 so we fake this calling these function to a different value
with the same buffer but a length to zero.
But those functions don't seem to check the length of zero before
performing a memcpy of length zero but with src and dst buf on the
same pointer, causing valgrind to bark.
So the code was re-work to pass them different pointers even
if buffer content is un-used.
In a second time, reseting value, a memcpy overlap
happened on the SESSION_ID_CONTEXT. It was re-worked and this is
now reset using the constant global value SHCTX_APPNAME which is a
different pointer with the same content.
This patch should be backported in every version since ssl
support was added to haproxy if we want valgrind to shut up.
This is tracked in github issue #56.
(cherry picked from commit eb46965bbb21291aab75ae88f033d9c9bab4a785)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index a37ab6d..4307bbb 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4161,18 +4161,31 @@
unsigned char encid[SSL_MAX_SSL_SESSION_ID_LENGTH]; /* encoded id */
unsigned char *p;
int data_len;
- unsigned int sid_length, sid_ctx_length;
+ unsigned int sid_length;
const unsigned char *sid_data;
- const unsigned char *sid_ctx_data;
/* Session id is already stored in to key and session id is known
* so we dont store it to keep size.
+ * note: SSL_SESSION_set1_id is using
+ * a memcpy so we need to use a different pointer
+ * than sid_data or sid_ctx_data to avoid valgrind
+ * complaining.
*/
sid_data = SSL_SESSION_get_id(sess, &sid_length);
- sid_ctx_data = SSL_SESSION_get0_id_context(sess, &sid_ctx_length);
- SSL_SESSION_set1_id(sess, sid_data, 0);
- SSL_SESSION_set1_id_context(sess, sid_ctx_data, 0);
+
+ /* copy value in an other buffer */
+ memcpy(encid, sid_data, sid_length);
+
+ /* pad with 0 */
+ if (sid_length < SSL_MAX_SSL_SESSION_ID_LENGTH)
+ memset(encid + sid_length, 0, SSL_MAX_SSL_SESSION_ID_LENGTH-sid_length);
+
+ /* force length to zero to avoid ASN1 encoding */
+ SSL_SESSION_set1_id(sess, encid, 0);
+
+ /* force length to zero to avoid ASN1 encoding */
+ SSL_SESSION_set1_id_context(sess, (const unsigned char *)SHCTX_APPNAME, 0);
/* check if buffer is large enough for the ASN1 encoded session */
data_len = i2d_SSL_SESSION(sess, NULL);
@@ -4184,9 +4197,6 @@
/* process ASN1 session encoding before the lock */
i2d_SSL_SESSION(sess, &p);
- memcpy(encid, sid_data, sid_length);
- if (sid_length < SSL_MAX_SSL_SESSION_ID_LENGTH)
- memset(encid + sid_length, 0, SSL_MAX_SSL_SESSION_ID_LENGTH-sid_length);
shctx_lock(ssl_shctx);
/* store to cache */
@@ -4194,8 +4204,8 @@
shctx_unlock(ssl_shctx);
err:
/* reset original length values */
- SSL_SESSION_set1_id(sess, sid_data, sid_length);
- SSL_SESSION_set1_id_context(sess, sid_ctx_data, sid_ctx_length);
+ SSL_SESSION_set1_id(sess, encid, sid_length);
+ SSL_SESSION_set1_id_context(sess, (const unsigned char *)SHCTX_APPNAME, strlen(SHCTX_APPNAME));
return 0; /* do not increment session reference count */
}