Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tls statemachine during tls v1.2 renegotiation #712

Conversation

fAuernigg
Copy link
Contributor

No description provided.

@fAuernigg fAuernigg changed the title Fix tls statemachine during tls 1 2 renegotiation Fix tls statemachine during tls v1.2 renegotiation Feb 27, 2023
@cspiel1 cspiel1 marked this pull request as ready for review February 27, 2023 14:49
&& SSL_state(tc->ssl) != TLS_ST_CW_FINISHED
&& SSL_state(tc->ssl) != TLS_ST_SW_SRVR_DONE
#endif
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it is a bit difficult to read ...

perhaps it is better to declare a bool before the if and use that bool inside the if ?

@fAuernigg fAuernigg force-pushed the fix_tls_statemachine_during_tls_1_2_renegotiation branch 2 times, most recently from 5801dfb to 4e82824 Compare February 28, 2023 11:57
@alfredh
Copy link
Contributor

alfredh commented Feb 28, 2023

thanks, this looks much better :)

@@ -217,6 +217,7 @@ static bool recv_handler(int *err, struct mbuf *mb, bool *estab, void *arg)
{
struct tls_conn *tc = arg;
int r;
bool reneg_state = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer reneg as name for this flag.

@fAuernigg fAuernigg force-pushed the fix_tls_statemachine_during_tls_1_2_renegotiation branch from 4e82824 to a586547 Compare February 28, 2023 13:28
@fAuernigg fAuernigg force-pushed the fix_tls_statemachine_during_tls_1_2_renegotiation branch from a586547 to 4832ccc Compare February 28, 2023 13:29
@cspiel1
Copy link
Collaborator

cspiel1 commented Feb 28, 2023

Looks good to me.

@sreimers
Copy link
Member

sreimers commented Feb 28, 2023

@fAuernigg fAuernigg closed this Mar 1, 2023
@cspiel1
Copy link
Collaborator

cspiel1 commented Mar 2, 2023

Maybe we could make it configurable for specific HTTP URL paths to allow TLS 1.2 renegotiation in a hopefully simple and readable way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants