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

Adding tests for ASCONF chunks #487

Merged
merged 2 commits into from
Jun 23, 2020
Merged

Conversation

yuquanw2
Copy link
Contributor

Adding fuzzer coverage referring to the issue and a closed PR.

@@ -151,6 +151,7 @@ initialize_fuzzer(void) {

usrsctp_register_address((void *)1);
usrsctp_sysctl_set_sctp_pktdrop_enable(1);
usrsctp_sysctl_set_sctp_asconf_enable(1);
Copy link
Member

Choose a reason for hiding this comment

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

I guess you also want to enable support for SCTP authentication by adding before this line:

usrsctp_sysctl_set_sctp_auth_enable(1);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that the sctp_auth is set to 1 by default so that I skipped it. Do you think that it will be better to add it to the code?

Copy link
Member

Choose a reason for hiding this comment

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

I think that the sctp_auth is set to 1 by default so that I skipped it. Do you think that it will be better to add it to the code?

Isn't it the same true for sctp_asconf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, then I guess I only need to add a CORPUS. Sorry about this.

Copy link
Member

Choose a reason for hiding this comment

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

There is no problem in explicitly enabling what you want to enable. That way you are not dependent on the defaults. Just be consistent. Either enable auth and asconf to be sure, or none and depend on the defaults...

As a potential next step you might want to enable NR-SACKs. Using that would make sense for WebRTC. The support is implemented and should be working, it is just not standardised by the IETF...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! I will check on that part.

@tuexen tuexen merged commit 3da1aa5 into sctplab:master Jun 23, 2020
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