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

Change Unknown/unknown to behave the same on both linux and windows #12

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

OpreaCristian2002
Copy link
Contributor

Bug

There were differences in the number of attack graphs generated between users running SAGE on Linux and Windows. We believe this was due to the "Unknown/unknown" protocol and how Linux and Windows handle case sensitivity of file names.

Proposed fix

In case the IANA mapping doesn't have a port, instead of overriding it with "Unknown", override it with "unknown". This way there is no confusion with case sensitivity.

Example of wrong behaviour

For example, on a Linux machine, one would get 167 attack graphs for the 2017 dataset, while on a Windows machine, one would get 162.

Example behaviour after fix

After the fix, the results are consistent between Linux and Windows users, with both machines getting 162 attack graphs.

@jzelenjak
Copy link
Collaborator

Indeed. In the screenshot below, you can see that Windows overwrites the files with "Unknown" and "unknown" that Linux does not overwrite (Windows treats files and directories as case-insensitive, whereas Linux as case-sensitive). I have also verified that this "Unknown" is assigned only in the load_IANA_mapping method, but everywhere else "unknown" is used.

Finally, after implementing both fixes (this and in another PR), my scripts show that for both Windows and Linux, the generated AG are the same in terms of the filenames (that is found victim-objective pairs), node names and edge names and labels.
image
image

@Alexandru-Dumitriu
Copy link

This bug fix is indeed corect, it solves the problem we have been discussing for the last week with the inconsistent values between Linux and Windows. Thank you for creating the pull requests for our change.

@azqanadeem azqanadeem merged commit 1804838 into tudelft-cda-lab:main Jul 9, 2023
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