From 14c5b7ebafe2ac7a0b39373c875c9cb4f2051243 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Sat, 20 Feb 2021 06:13:49 +0000 Subject: [PATCH] ss: state violations need to report lifecycle tags The state tracking and violation detection is very powerful at enforcing only legal transitions, but if it's busy, we don't get to see which stream had to problem. Add a pointer to the handle lc tag, do that rather than just pass the handle so we can deal with ss and sspc handles cleanly. --- .../private-lib-secure-streams.h | 3 +- lib/secure-streams/secure-streams-client.c | 2 +- lib/secure-streams/secure-streams-process.c | 3 +- lib/secure-streams/secure-streams-serialize.c | 49 +++++++++---------- lib/secure-streams/secure-streams.c | 22 ++++++--- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/lib/secure-streams/private-lib-secure-streams.h b/lib/secure-streams/private-lib-secure-streams.h index 91c3705a2..ab0b661e4 100644 --- a/lib/secure-streams/private-lib-secure-streams.h +++ b/lib/secure-streams/private-lib-secure-streams.h @@ -473,7 +473,8 @@ lws_sspc_event_helper(lws_sspc_handle_t *h, lws_ss_constate_t cs, lws_ss_tx_ordinal_t flags); int -lws_ss_check_next_state(uint8_t *prevstate, lws_ss_constate_t cs); +lws_ss_check_next_state(lws_lifecycle_t *lc, uint8_t *prevstate, + lws_ss_constate_t cs); void lws_proxy_clean_conn_ss(struct lws *wsi); diff --git a/lib/secure-streams/secure-streams-client.c b/lib/secure-streams/secure-streams-client.c index 3e4315b45..a8effb196 100644 --- a/lib/secure-streams/secure-streams-client.c +++ b/lib/secure-streams/secure-streams-client.c @@ -26,7 +26,7 @@ lws_sspc_event_helper(lws_sspc_handle_t *h, lws_ss_constate_t cs, if (!h) return LWSSSSRET_OK; - if (lws_ss_check_next_state(&h->prev_ss_state, cs)) + if (lws_ss_check_next_state(&h->lc, &h->prev_ss_state, cs)) return LWSSSSRET_DESTROY_ME; if (!h->ssi.state) diff --git a/lib/secure-streams/secure-streams-process.c b/lib/secure-streams/secure-streams-process.c index 47b1fce58..6f6fb79f7 100644 --- a/lib/secure-streams/secure-streams-process.c +++ b/lib/secure-streams/secure-streams-process.c @@ -484,7 +484,8 @@ callback_ss_proxy(struct lws *wsi, enum lws_callback_reasons reason, break; case LWS_CALLBACK_RAW_WRITEABLE: - // lwsl_notice("LWS_CALLBACK_RAW_PROXY_SRV_WRITEABLE\n"); + lwsl_debug("%s: %s: LWS_CALLBACK_RAW_WRITEABLE, state 0x%x\n", + __func__, lws_wsi_tag(wsi), lwsi_state(wsi)); /* * We can transmit something back to the client from the dsh diff --git a/lib/secure-streams/secure-streams-serialize.c b/lib/secure-streams/secure-streams-serialize.c index 7223736e0..eac5956cb 100644 --- a/lib/secure-streams/secure-streams-serialize.c +++ b/lib/secure-streams/secure-streams-serialize.c @@ -408,35 +408,30 @@ lws_ss_deserialize_parse(struct lws_ss_serialization_parser *par, case LWSSS_SER_TXPRE_ONWARD_CONNECT: if (client) goto hangup; + if (*state != LPCSPROX_OPERATIONAL) goto hangup; par->ps = RPAR_TYPE; - lwsl_notice("%s: LWSSS_SER_TXPRE_ONWARD_CONNECT\n", __func__); + lwsl_notice("%s: ONWARD_CONNECT\n", __func__); - if (proxy_pss_to_ss_h(pss) && - !proxy_pss_to_ss_h(pss)->wsi) - /* - * We're going to try to do the onward - * connect, but that could end in any - * of the ways like DESTROY_ME etc - */ - switch (_lws_ss_client_connect( - proxy_pss_to_ss_h(pss), 0, parconn)) { - case LWSSSSRET_OK: - /* well, connect is ongoing */ - break; - case LWSSSSRET_TX_DONT_SEND: - /* it has failed already... */ - break; - case LWSSSSRET_DISCONNECT_ME: -// if (lws_ss_backoff(h)) -// /* has been destroyed */ -// return 1; - break; - case LWSSSSRET_DESTROY_ME: - goto hangup; - } + /* + * Shrug it off if we are already connecting or + * connected + */ + + if (!proxy_pss_to_ss_h(pss) || + proxy_pss_to_ss_h(pss)->wsi) + break; + + /* + * We're going to try to do the onward connect + */ + + if (_lws_ss_client_connect(proxy_pss_to_ss_h(pss), + 0, parconn) == + LWSSSSRET_DESTROY_ME) + goto hangup; break; case LWSSS_SER_TXPRE_STREAMTYPE: @@ -1240,7 +1235,8 @@ payload_ff: h->creating_cb_done = 1; - if (lws_ss_check_next_state(&h->prev_ss_state, LWSSSCS_CREATING)) + if (lws_ss_check_next_state(&h->lc, &h->prev_ss_state, + LWSSSCS_CREATING)) return LWSSSSRET_DESTROY_ME; h->prev_ss_state = (uint8_t)LWSSSCS_CREATING; @@ -1396,7 +1392,8 @@ payload_ff: if (cs == LWSSSCS_DISCONNECTED) h->ss_dangling_connected = 0; - if (lws_ss_check_next_state(&h->prev_ss_state, cs)) + if (lws_ss_check_next_state(&h->lc, + &h->prev_ss_state, cs)) return LWSSSSRET_DESTROY_ME; if (cs < LWSSSCS_USER_BASE) diff --git a/lib/secure-streams/secure-streams.c b/lib/secure-streams/secure-streams.c index 2cfcbd515..a5bd6c9e1 100644 --- a/lib/secure-streams/secure-streams.c +++ b/lib/secure-streams/secure-streams.c @@ -172,7 +172,8 @@ static const uint32_t ss_state_txn_validity[] = { }; int -lws_ss_check_next_state(uint8_t *prevstate, lws_ss_constate_t cs) +lws_ss_check_next_state(lws_lifecycle_t *lc, uint8_t *prevstate, + lws_ss_constate_t cs) { if (cs >= LWSSSCS_USER_BASE) /* @@ -183,29 +184,34 @@ lws_ss_check_next_state(uint8_t *prevstate, lws_ss_constate_t cs) if (cs >= LWS_ARRAY_SIZE(ss_state_txn_validity)) { /* we don't recognize this state as usable */ - lwsl_err("%s: bad new state %u\n", __func__, cs); + lwsl_err("%s: %s: bad new state %u\n", __func__, lc->gutag, cs); assert(0); return 1; } if (*prevstate >= LWS_ARRAY_SIZE(ss_state_txn_validity)) { /* existing state is broken */ - lwsl_err("%s: bad existing state %u\n", __func__, - (unsigned int)*prevstate); + lwsl_err("%s: %s: bad existing state %u\n", __func__, + lc->gutag, (unsigned int)*prevstate); assert(0); return 1; } if (ss_state_txn_validity[*prevstate] & (1u << cs)) { + + lwsl_notice("%s: %s: %s -> %s\n", __func__, lc->gutag, + lws_ss_state_name((int)*prevstate), + lws_ss_state_name((int)cs)); + /* this is explicitly allowed, update old state to new */ *prevstate = (uint8_t)cs; return 0; } - lwsl_err("%s: transition from %s -> %s is illegal\n", __func__, - lws_ss_state_name((int)*prevstate), - lws_ss_state_name((int)cs)); + lwsl_err("%s: %s: transition from %s -> %s is illegal\n", __func__, + lc->gutag, lws_ss_state_name((int)*prevstate), + lws_ss_state_name((int)cs)); assert(0); @@ -232,7 +238,7 @@ lws_ss_event_helper(lws_ss_handle_t *h, lws_ss_constate_t cs) if (!h) return LWSSSSRET_OK; - if (lws_ss_check_next_state(&h->prev_ss_state, cs)) + if (lws_ss_check_next_state(&h->lc, &h->prev_ss_state, cs)) return LWSSSSRET_DESTROY_ME; if (cs == LWSSSCS_CONNECTED)