Skip to content

Mitigate inconsistent test results for ROBOT#2969

Merged
drwetter merged 2 commits into
3.3devfrom
mitigate_2083
Dec 19, 2025
Merged

Mitigate inconsistent test results for ROBOT#2969
drwetter merged 2 commits into
3.3devfrom
mitigate_2083

Conversation

@drwetter
Copy link
Copy Markdown
Collaborator

@drwetter drwetter commented Dec 15, 2025

As reported a longer while back in #2083 there were trailing bytes when receiving a TLS alert by the ROBOT check.

This PR corrects and thus normalizes the length of the TLS alert message to the correct value, supposed the length in the TLS alert is two bytes and it is an TLS alert.

Also this PR now uses a separate variable for the timeout. In 2ce0110 the timeout was changed by mistake as MAX_WAITSOCK was reduced from 10 to 5. For this check it is still 5 which seemed fine (TBC). Using a separate global variable however may offer some possibility for tuning the check when the latency to the target is high.

open:

  • update the docs wrt ROBOT_TIMEOUT
  • more checks

What is your pull request about?

  • Bug fix
  • Improvement
  • New feature (adds functionality)
  • Breaking change (bug fix, feature or improvement that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable

  • For the main program: My edits contain no tabs, indentation is five spaces and any line endings do not contain any blank chars
  • I've read CONTRIBUTING.md and Coding_Convention.md
  • I have tested this fix or improvement against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

As reported a longer while back in #2083 there were trailing bytes
when receiving a TLS alert by the ROBOT check.

This PR corrects and thus normalizes the length of the TLS alert message to the
correct value, supposed the length in the TLS alart is two bytes and it is an
TLS alert.

Also this PR now uses a separate variable for the timeout. In 2ce0110 the timeout
was changed by mistake as MAX_WAITSOCK was reduced from 10 to 5. For this check it
is still 5 which seemed fine (TBC). Using a separate global variable however may offer
some possibility for tuning the check when the latency to the target is high.
@drwetter drwetter requested a review from dcooper16 December 15, 2025 12:02
@drwetter
Copy link
Copy Markdown
Collaborator Author

This needs to be backported to 3.2

@drwetter drwetter added 3.3dev next dev 3.2 stable labels Dec 15, 2025
@drwetter drwetter merged commit 61d0189 into 3.3dev Dec 19, 2025
5 checks passed
@drwetter
Copy link
Copy Markdown
Collaborator Author

I merged this to mitigate the problem and move things forward.

Not sure really about the timeout , @dcooper16 ? In practice and so far it seemed fine to me. The implementation though is not as easy to understand for me. If you have a minute or two, your input would be highly appreciated.

At a certain point of time I'd like to backport this to 3.2 . At that point though I'd like to be more certain whether I did the right thing.

@drwetter drwetter deleted the mitigate_2083 branch December 19, 2025 14:20
@drwetter drwetter removed the 3.2 stable label Dec 20, 2025
drwetter added a commit that referenced this pull request Dec 20, 2025
As reported a longer while back in #2083 there were trailing bytes when receiving a TLS alert by the ROBOT check.

This PR corrects and thus normalizes the length of the TLS alert message to the correct value, supposed the length in the TLS alert is two bytes and it is an TLS alert. PR for 3.3dev was #2969 .

Also this PR now uses a separate variable for the timeout. Using a separate global variable may offer some possibility for tuning the check when the latency to the target is high. This is still subject of research.
The variable is 10 seconds here to be in line with MAX_WAITSOCK which (name) was used previously.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3dev next dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant