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

Make sensitivity of is_vulnerable_to_client_renegotiation_dos configurable #661

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

mxsasha
Copy link
Contributor

@mxsasha mxsasha commented Aug 20, 2024

Trying renegotiation multiple times to find mitigations makes sense. However, it is quite rare for servers to allow 1 but not too many renegotiations. For my use, I want to test compliance with requirements that do not allow it at all. Therefore, I'd like it to be configurable.

@mxsasha mxsasha force-pushed the configurable-renegotiation-limit branch from c7dee9a to 9775655 Compare September 3, 2024 14:12
@mxsasha mxsasha force-pushed the configurable-renegotiation-limit branch from 9775655 to f262aa8 Compare September 3, 2024 14:32
@nabla-c0d3
Copy link
Owner

Hello !

Thanks for the contribution, and nice catch on using an ExtraArgument class VS ServerConnectivityInfo for passing the setting around.

One last request : could you add a simple test that just ensures that client_renegotiation_attempts is being used when it supplied via SessionRenegotiationExtraArgument.

Thanks!

@mxsasha mxsasha force-pushed the configurable-renegotiation-limit branch from 4655a00 to 2c7a787 Compare October 8, 2024 11:41
@mxsasha mxsasha force-pushed the configurable-renegotiation-limit branch from 2c7a787 to fca777b Compare October 8, 2024 11:48
@mxsasha
Copy link
Contributor Author

mxsasha commented Oct 8, 2024

One last request : could you add a simple test that just ensures that client_renegotiation_attempts is being used when it supplied via SessionRenegotiationExtraArgument.

The only practical way I see to test that, is to track the number of attempts. Which actually can be useful for users that get into these details anyways. Code may be a bit clunky, I was struggling with the best way to return multiple values.

@nabla-c0d3 nabla-c0d3 changed the base branch from release to dev December 26, 2024 16:55
@nabla-c0d3 nabla-c0d3 merged commit a0ec707 into nabla-c0d3:dev Dec 27, 2024
9 of 11 checks passed
@nabla-c0d3
Copy link
Owner

It looks good - merging this as is. Thanks !

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.

2 participants