Skip to content

Commit

Permalink
tls: add stop listen handler
Browse files Browse the repository at this point in the history 
Change-Id: I233d02a669b6a0504cd54590c6c8e4fefadc4713
Signed-off-by: Florin Coras <[email protected]>
  • Loading branch information
@florincoras
florincoras authored and Damjan Marion committed Mar 5, 2018
1 parent 03f942a commit e3e2f07
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 20 deletions.
113 changes: 102 additions & 11 deletions src/tests/vnet/session/tcp_echo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,58 @@ echo_main_t echo_main;
#define NITER 4000000
#endif

const char test_srv_crt_rsa[] =
"-----BEGIN CERTIFICATE-----\r\n"
"MIIDNzCCAh+gAwIBAgIBAjANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER\r\n"
"MA8GA1UEChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EwHhcN\r\n"
"MTEwMjEyMTQ0NDA2WhcNMjEwMjEyMTQ0NDA2WjA0MQswCQYDVQQGEwJOTDERMA8G\r\n"
"A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcN\r\n"
"AQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do8Ss86ExbQWuTN\r\n"
"owCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFKz\r\n"
"NtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kM\r\n"
"tQCQ4dqCEGZ9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8P\r\n"
"hYvai0Re4hjGYi/HZo36Xdh98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjya\r\n"
"HT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lkvEcf36hIBMJcCAwEAAaNNMEswCQYD\r\n"
"VR0TBAIwADAdBgNVHQ4EFgQUpQXoZLjc32APUBJNYKhkr02LQ5MwHwYDVR0jBBgw\r\n"
"FoAUtFrkpbPe0lL2udWmlQ/rPrzH/f8wDQYJKoZIhvcNAQEFBQADggEBAJxnXClY\r\n"
"oHkbp70cqBrsGXLybA74czbO5RdLEgFs7rHVS9r+c293luS/KdliLScZqAzYVylw\r\n"
"UfRWvKMoWhHYKp3dEIS4xTXk6/5zXxhv9Rw8SGc8qn6vITHk1S1mPevtekgasY5Y\r\n"
"iWQuM3h4YVlRH3HHEMAD1TnAexfXHHDFQGe+Bd1iAbz1/sH9H8l4StwX6egvTK3M\r\n"
"wXRwkKkvjKaEDA9ATbZx0mI8LGsxSuCqe9r9dyjmttd47J1p1Rulz3CLzaRcVIuS\r\n"
"RRQfaD8neM9c1S/iJ/amTVqJxA1KOdOS5780WhPfSArA+g4qAmSjelc3p4wWpha8\r\n"
"zhuYwjVuX6JHG0c=\r\n" "-----END CERTIFICATE-----\r\n";
const u32 test_srv_crt_rsa_len = sizeof (test_srv_crt_rsa);

const char test_srv_key_rsa[] =
"-----BEGIN RSA PRIVATE KEY-----\r\n"
"MIIEpAIBAAKCAQEAwU2j3efNHdEE10lyuJmsDnjkOjxKzzoTFtBa5M2jAIin7h5r\r\n"
"lqdStJDvLXJ6PiSa/LY0rCT1d+AmZIycsCh9odrqjObJHJa8/sEEUrM21KP64bF2\r\n"
"2JDBYbRmUjaiJlOqq3ReB30Zgtsq2B+g2Q0cLUlm91slc0boC4pPaQy1AJDh2oIQ\r\n"
"Zn2uVCuLZXmRoeJhw81ASQjuaAzxi4bSRr/QuKoRAx5/VqgaHkQYDw+Fi9qLRF7i\r\n"
"GMZiL8dmjfpd2H3zJ4kpAcWQDj8n8TDISg7v1t7HxydrxwU9esQCPJodPg/oNJhb\r\n"
"y3NLUpbYEaIsgIhpOVrTD7DeWS8Rx/fqEgEwlwIDAQABAoIBAQCXR0S8EIHFGORZ\r\n"
"++AtOg6eENxD+xVs0f1IeGz57Tjo3QnXX7VBZNdj+p1ECvhCE/G7XnkgU5hLZX+G\r\n"
"Z0jkz/tqJOI0vRSdLBbipHnWouyBQ4e/A1yIJdlBtqXxJ1KE/ituHRbNc4j4kL8Z\r\n"
"/r6pvwnTI0PSx2Eqs048YdS92LT6qAv4flbNDxMn2uY7s4ycS4Q8w1JXnCeaAnYm\r\n"
"WYI5wxO+bvRELR2Mcz5DmVnL8jRyml6l6582bSv5oufReFIbyPZbQWlXgYnpu6He\r\n"
"GTc7E1zKYQGG/9+DQUl/1vQuCPqQwny0tQoX2w5tdYpdMdVm+zkLtbajzdTviJJa\r\n"
"TWzL6lt5AoGBAN86+SVeJDcmQJcv4Eq6UhtRr4QGMiQMz0Sod6ettYxYzMgxtw28\r\n"
"CIrgpozCc+UaZJLo7UxvC6an85r1b2nKPCLQFaggJ0H4Q0J/sZOhBIXaoBzWxveK\r\n"
"nupceKdVxGsFi8CDy86DBfiyFivfBj+47BbaQzPBj7C4rK7UlLjab2rDAoGBAN2u\r\n"
"AM2gchoFiu4v1HFL8D7lweEpi6ZnMJjnEu/dEgGQJFjwdpLnPbsj4c75odQ4Gz8g\r\n"
"sw9lao9VVzbusoRE/JGI4aTdO0pATXyG7eG1Qu+5Yc1YGXcCrliA2xM9xx+d7f+s\r\n"
"mPzN+WIEg5GJDYZDjAzHG5BNvi/FfM1C9dOtjv2dAoGAF0t5KmwbjWHBhcVqO4Ic\r\n"
"BVvN3BIlc1ue2YRXEDlxY5b0r8N4XceMgKmW18OHApZxfl8uPDauWZLXOgl4uepv\r\n"
"whZC3EuWrSyyICNhLY21Ah7hbIEBPF3L3ZsOwC+UErL+dXWLdB56Jgy3gZaBeW7b\r\n"
"vDrEnocJbqCm7IukhXHOBK8CgYEAwqdHB0hqyNSzIOGY7v9abzB6pUdA3BZiQvEs\r\n"
"3LjHVd4HPJ2x0N8CgrBIWOE0q8+0hSMmeE96WW/7jD3fPWwCR5zlXknxBQsfv0gP\r\n"
"3BC5PR0Qdypz+d+9zfMf625kyit4T/hzwhDveZUzHnk1Cf+IG7Q+TOEnLnWAWBED\r\n"
"ISOWmrUCgYAFEmRxgwAc/u+D6t0syCwAYh6POtscq9Y0i9GyWk89NzgC4NdwwbBH\r\n"
"4AgahOxIxXx2gxJnq3yfkJfIjwf0s2DyP0kY2y6Ua1OeomPeY9mrIS4tCuDQ6LrE\r\n"
"TB6l9VGoxJL4fyHnZb8L5gGvnB1bbD8cL6YPaDiOhcRseC9vBiEuVg==\r\n"
"-----END RSA PRIVATE KEY-----\r\n";
const u32 test_srv_key_rsa_len = sizeof (test_srv_key_rsa);

static u8 *
format_api_error (u8 * s, va_list * args)
{
Expand Down Expand Up @@ -191,6 +243,9 @@ void
application_send_attach (echo_main_t * em)
{
vl_api_application_attach_t *bmp;
vl_api_application_tls_cert_add_t *cert_mp;
vl_api_application_tls_key_add_t *key_mp;

u32 fifo_size = 4 << 20;
bmp = vl_msg_api_alloc (sizeof (*bmp));
memset (bmp, 0, sizeof (*bmp));
Expand All @@ -206,6 +261,24 @@ application_send_attach (echo_main_t * em)
bmp->options[APP_OPTIONS_ADD_SEGMENT_SIZE] = 128 << 20;
bmp->options[APP_OPTIONS_SEGMENT_SIZE] = 256 << 20;
vl_msg_api_send_shmem (em->vl_input_queue, (u8 *) & bmp);

cert_mp = vl_msg_api_alloc (sizeof (*cert_mp) + test_srv_crt_rsa_len);
memset (cert_mp, 0, sizeof (*cert_mp));
cert_mp->_vl_msg_id = ntohs (VL_API_APPLICATION_TLS_CERT_ADD);
cert_mp->client_index = em->my_client_index;
cert_mp->context = ntohl (0xfeedface);
cert_mp->cert_len = clib_host_to_net_u16 (test_srv_crt_rsa_len);
clib_memcpy (cert_mp->cert, test_srv_crt_rsa, test_srv_crt_rsa_len);
vl_msg_api_send_shmem (em->vl_input_queue, (u8 *) & cert_mp);

key_mp = vl_msg_api_alloc (sizeof (*key_mp) + test_srv_key_rsa_len);
memset (key_mp, 0, sizeof (*key_mp) + test_srv_key_rsa_len);
key_mp->_vl_msg_id = ntohs (VL_API_APPLICATION_TLS_KEY_ADD);
key_mp->client_index = em->my_client_index;
key_mp->context = ntohl (0xfeedface);
key_mp->key_len = clib_host_to_net_u16 (test_srv_key_rsa_len);
clib_memcpy (key_mp->key, test_srv_key_rsa, test_srv_key_rsa_len);
vl_msg_api_send_shmem (em->vl_input_queue, (u8 *) & key_mp);
}

int
Expand Down Expand Up @@ -1218,17 +1291,35 @@ vl_api_disconnect_session_reply_t_handler (vl_api_disconnect_session_reply_t *
session_print_stats (em, session);
}

#define foreach_tcp_echo_msg \
_(BIND_URI_REPLY, bind_uri_reply) \
_(UNBIND_URI_REPLY, unbind_uri_reply) \
_(ACCEPT_SESSION, accept_session) \
_(CONNECT_SESSION_REPLY, connect_session_reply) \
_(DISCONNECT_SESSION, disconnect_session) \
_(DISCONNECT_SESSION_REPLY, disconnect_session_reply) \
_(RESET_SESSION, reset_session) \
_(APPLICATION_ATTACH_REPLY, application_attach_reply) \
_(APPLICATION_DETACH_REPLY, application_detach_reply) \
_(MAP_ANOTHER_SEGMENT, map_another_segment) \
static void
vl_api_application_tls_cert_add_reply_t_handler
(vl_api_application_tls_cert_add_reply_t * mp)
{
if (mp->retval)
clib_warning ("failed to add tls cert");
}

static void
vl_api_application_tls_key_add_reply_t_handler
(vl_api_application_tls_key_add_reply_t * mp)
{
if (mp->retval)
clib_warning ("failed to add tls key");
}

#define foreach_tcp_echo_msg \
_(BIND_URI_REPLY, bind_uri_reply) \
_(UNBIND_URI_REPLY, unbind_uri_reply) \
_(ACCEPT_SESSION, accept_session) \
_(CONNECT_SESSION_REPLY, connect_session_reply) \
_(DISCONNECT_SESSION, disconnect_session) \
_(DISCONNECT_SESSION_REPLY, disconnect_session_reply) \
_(RESET_SESSION, reset_session) \
_(APPLICATION_ATTACH_REPLY, application_attach_reply) \
_(APPLICATION_DETACH_REPLY, application_detach_reply) \
_(MAP_ANOTHER_SEGMENT, map_another_segment) \
_(APPLICATION_TLS_CERT_ADD_REPLY, application_tls_cert_add_reply) \
_(APPLICATION_TLS_KEY_ADD_REPLY, application_tls_key_add_reply) \

void
tcp_echo_api_hookup (echo_main_t * em)
Expand Down
35 changes: 28 additions & 7 deletions src/vnet/session-apps/tls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ tls_listener_ctx_alloc (void)
}

void
tls_ctx_listener_free (tls_ctx_t * ctx)
tls_listener_ctx_free (tls_ctx_t * ctx)
{
pool_put (tls_main.half_open_ctx_pool, ctx);
pool_put (tls_main.listener_ctx_pool, ctx);
}

tls_ctx_t *
Expand Down Expand Up @@ -936,6 +936,13 @@ tls_disconnect (u32 ctx_index, u32 thread_index)
app_session->server_tx_fifo);
session_free (app_session);
}
if (ctx->ssl.conf->endpoint == MBEDTLS_SSL_IS_SERVER)
{
mbedtls_x509_crt_free (&ctx->srvcert);
mbedtls_pk_free (&ctx->pkey);
}
mbedtls_ssl_free (&ctx->ssl);
mbedtls_ssl_config_free (&ctx->conf);
tls_ctx_free (ctx);
}

Expand Down Expand Up @@ -974,12 +981,26 @@ tls_start_listen (u32 app_listener_index, transport_endpoint_t * tep)
}

u32
tls_stop_listen (u32 listener_index)
tls_stop_listen (u32 lctx_index)
{
clib_warning ("TBD");
tls_main_t *tm = &tls_main;
application_t *tls_app;
tls_ctx_t *lctx;
lctx = tls_listener_ctx_get (lctx_index);
tls_app = application_get (tm->app_index);
application_stop_listen (tls_app, lctx->tls_session_handle);
tls_listener_ctx_free (lctx);
return 0;
}

transport_connection_t *
tls_connection_get (u32 ctx_index, u32 thread_index)
{
tls_ctx_t *ctx;
ctx = tls_ctx_get_w_thread (ctx_index, thread_index);
return &ctx->connection;
}

transport_connection_t *
tls_listener_get (u32 listener_index)
{
Expand All @@ -999,9 +1020,8 @@ format_tls_ctx (u8 * s, va_list * args)
if (thread_index != child_ti)
clib_warning ("app and tls sessions are on different threads!");

s =
format (s, "[#%d][TLS] app %u child %u", child_ti, ctx->parent_app_index,
child_si);
s = format (s, "[#%d][TLS] app %u child %u", child_ti,
ctx->parent_app_index, child_si);
return s;
}

Expand Down Expand Up @@ -1055,6 +1075,7 @@ const static transport_proto_vft_t tls_proto = {
.open = tls_connect,
.close = tls_disconnect,
.bind = tls_start_listen,
.get_connection = tls_connection_get,
.get_listener = tls_listener_get,
.unbind = tls_stop_listen,
.tx_type = TRANSPORT_TX_INTERNAL,
Expand Down
16 changes: 14 additions & 2 deletions src/vnet/session/session_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1111,15 +1111,21 @@ vl_api_application_tls_cert_add_t_handler (vl_api_application_tls_cert_add_t *
vl_api_app_namespace_add_del_reply_t *rmp;
vnet_app_add_tls_cert_args_t _a, *a = &_a;
clib_error_t *error;
application_t *app;
u32 cert_len;
int rv = 0;
if (!session_manager_is_enabled ())
{
rv = VNET_API_ERROR_FEATURE_DISABLED;
goto done;
}
if (!(app = application_lookup (mp->client_index)))
{
rv = VNET_API_ERROR_APPLICATION_NOT_ATTACHED;
goto done;
}
memset (a, 0, sizeof (*a));
a->app_index = clib_net_to_host_u32 (mp->app_index);
a->app_index = app->index;
cert_len = clib_net_to_host_u16 (mp->cert_len);
vec_validate (a->cert, cert_len);
clib_memcpy (a->cert, mp->cert, cert_len);
Expand All @@ -1140,15 +1146,21 @@ vl_api_application_tls_key_add_t_handler (vl_api_application_tls_key_add_t *
vl_api_app_namespace_add_del_reply_t *rmp;
vnet_app_add_tls_key_args_t _a, *a = &_a;
clib_error_t *error;
application_t *app;
u32 key_len;
int rv = 0;
if (!session_manager_is_enabled ())
{
rv = VNET_API_ERROR_FEATURE_DISABLED;
goto done;
}
if (!(app = application_lookup (mp->client_index)))
{
rv = VNET_API_ERROR_APPLICATION_NOT_ATTACHED;
goto done;
}
memset (a, 0, sizeof (*a));
a->app_index = clib_net_to_host_u32 (mp->app_index);
a->app_index = app->index;
key_len = clib_net_to_host_u16 (mp->key_len);
vec_validate (a->key, key_len);
clib_memcpy (a->key, mp->key, key_len);
Expand Down

10 comments on commit e3e2f07

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello,when i user pool_put ,it happens a core dump
0x00007fed358fc37f in raise () from /lib64/libc.so.6
#1 0x00007fed358e6db5 in abort () from /lib64/libc.so.6
#2 0x000055cde7d8031a in os_exit (code=code@entry=1) at /usr/src/debug/vpp-21.06/src/vpp/vnet/main.c:431
#3 0x00007fed37810d67 in unix_signal_handler (signum=6, si=, uc=) at /usr/src/debug/vpp-21.06/src/vlib/unix/main.c:187
#4
#5 0x00007fed358fc37f in raise () from /lib64/libc.so.6
#6 0x00007fed358e6db5 in abort () from /lib64/libc.so.6
#7 0x000055cde7d802c3 in os_panic () at /usr/src/debug/vpp-21.06/src/vpp/vnet/main.c:407
#8 0x00007fed3601d4c5 in vec_resize_allocate_memory () from /lib64/libvppinfra.so.21.06
#9 0x00007fec354a6bf1 in _vec_resize_inline (numa_id=255, data_align=8, header_bytes=0, data_bytes=, length_increment=, v=) at /usr/src/debug/vpp-21.06/src/vppinfra/vec.h:172
#10 clib_bitmap_ori_notrim (i=3219795329229292237, ai=0x7fec386d9740) at /usr/src/debug/vpp-21.06/src/vppinfra/bitmap.h:648

follow is my debug info
8sizeof((uword*)0x7fec386d9740)
$23 = 64
(gdb) p ((vec_header_t *) (v) - 1)
No symbol "v" in current context.
(gdb) p ((vec_header_t *) (0x7fec386d9740) - 1)
$24 = (vec_header_t *) 0x7fec386d9738
(gdb) p *0x7fec386d9738
$25 = 734048780
(gdb) p *((vec_header_t *) (0x7fec386d9740) - 1)
$26 = {len = 734048780, numa_id = 255 '\377', vpad = "\000\000", vector_data = 0x7fec386d9740 ""}
(gdb) x/20xg 0x7fec386d9738
0x7fec386d9738: 0x000000ff2bc0b20c 0x0000000000000000
0x7fec386d9748: 0x0000000000000000 0x0000000000000000
0x7fec386d9758: 0x0000000000000033 0x0000000464612074
0x7fec386d9768: 0x000000ff00000000 0x0000000100000000
0x7fec386d9778: 0x0000000000000000 0x0000000000000000
0x7fec386d9788: 0x0000000000000041 0x00007fec3b2f1660
0x7fec386d9798: 0x00007fec3a3d8470 0x203a747065636341
0x7fec386d97a8: 0x00000000002a2f2a 0x0000000000000000
0x7fec386d97b8: 0x0000000000000000 0x0000000000000040
0x7fec386d97c8: 0x0000000000000082 0x00000004357c4188

I found that the element "free_bitmap" in pool_header_t is a vector, when i see the len of vector, the value is 734048780, I can not figure out how it happened, my application call pool_get_zero a few times, can not make the len of vector so big! I wonder if anyone has met the same problem

@florincoras
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

This is a really old patch, many things, including vppinfra have changed since 2018. Having said that, it looks like the pool might've been corrupted. First thing that comes to mind is thread safety, i.e., make sure only one thread allocates and frees elements from the pool.

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thandk you，
You are absolutely right. Our business logic involves a worker thread and a main thread. The worker thread calls pool_get_zero to obtain a piece of memory from the global variable event_pool, and then sends an event with the index of this memory block through the function vlib_process_signal_event_mt. On the other side, the main thread creates an event listener using the function vlib_process_create. When it receives an event from the worker thread, it retrieves the corresponding memory based on the index, uses it, and then returns it to the pool using pool_put. What are the potential issues with this usage method?

@florincoras
Copy link
Contributor Author

@florincoras florincoras commented on e3e2f07 Jul 4, 2024 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the debug info,i notice i=3219795329229292237, The value of i is the current pointer minus the memory pool base address. Normally, this value should be 1, but the stack information shows that this value is 3219795329229292237, which suggests that when calling poop_put to return memory, the memory pool base address may have changed, leading to incorrect calculations in the pool_put function where uword pool_var (l) = pool_var (e) - pool_var (p_);. As a result, the free_bitmap may expand beyond control, causing problems.

I would like to know if it is possible to use the pool_init_fixed function to pre-allocate a memory pool without using locks. This way, the memory pool base address will not change when pool_get re-applies for memory.

@florincoras
Copy link
Contributor Author

@florincoras florincoras commented on e3e2f07 Jul 4, 2024 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you are definitely right. so i try to use vec_validate to malloc a memory , vec_validate is thread safatey, right?

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to use vec_validate in a worker thread to allocate memory, then use vlib_process_signal_event_mt to send the memory address to the main thread. After the main thread uses this memory, it will call vec_free to release the memory. This logic should work fine.

@florincoras
Copy link
Contributor Author

@florincoras florincoras commented on e3e2f07 Jul 4, 2024 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjccc
Copy link

@fjccc fjccc commented on e3e2f07 Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it relies heavily on the locks in the dmalloc underlying and could potentially increase memory fragmentation if worker threads frequently call vec_validate

Please sign in to comment.