Skip to content

Disallow broadcast and multicast in DNS#3474

Merged
robgjansen merged 3 commits intoshadow:mainfrom
robgjansen:dns-invalid-addr
Dec 24, 2024
Merged

Disallow broadcast and multicast in DNS#3474
robgjansen merged 3 commits intoshadow:mainfrom
robgjansen:dns-invalid-addr

Conversation

@robgjansen
Copy link
Copy Markdown
Member

@robgjansen robgjansen commented Dec 23, 2024

closes #3465

@robgjansen robgjansen self-assigned this Dec 23, 2024
@github-actions github-actions bot added Component: Testing Unit and integration tests and frameworks Component: Main Composing the core Shadow executable labels Dec 23, 2024
@robgjansen
Copy link
Copy Markdown
Member Author

robgjansen commented Dec 23, 2024

@stevenengler Do we want to validate the address at the config parsing step too, or is it OK just in the dns module?

@stevenengler
Copy link
Copy Markdown
Contributor

stevenengler commented Dec 23, 2024 •
edited
Loading

@stevenengler Do we want to validate the address at the config parsing step too, or is it OK just in the dns module?

I don't have a strong opinion either way. I think it might be nice to validate at the config parsing step, but we have other address validation that doesn't happen at config parsing, such as checking that two hosts don't have the same IP address (in assign_ips). I think checking just at the dns module level is okay since it happens only at the beginning of the simulation. And that's what we've always done in the past anyways, so doesn't need to block this PR.

Does the error message from register end up being nice for the user when this does happen?

@robgjansen
Copy link
Copy Markdown
Member Author

robgjansen commented Dec 23, 2024

Does the error message from register end up being nice for the user when this does happen?

Better now, after 162e4a6 :)

@robgjansen robgjansen enabled auto-merge December 23, 2024 18:00
@github-actions github-actions bot added the Component: Documentation In-repository documentation, under docs/ label Dec 23, 2024
stevenengler
stevenengler approved these changes Dec 23, 2024
View reviewed changes
@stevenengler stevenengler mentioned this pull request Dec 24, 2024
Merged
@robgjansen robgjansen merged commit 19edfee into shadow:main Dec 24, 2024
@robgjansen robgjansen deleted the dns-invalid-addr branch December 24, 2024 17:32
stevenengler added a commit that referenced this pull request Dec 28, 2024
@stevenengler
Document automatic IP address assignment (#3477)
8157451 
Closes #3427.

Documents the behaviour as of #3474.

I don't want to go into too much detail since it would be bad if users
write their simulations in a way that relies on this behaviour, but
changing the behaviour would be a breaking change anyways so I think
it's okay to specify some of the details (like the starting address
11.0.0.1).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevenengler stevenengler stevenengler approved these changes

Assignees

@robgjansen robgjansen

Labels

Component: Documentation In-repository documentation, under docs/ Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks

Projects

None yet

Milestone

Code health and maintenance

Development

Successfully merging this pull request may close these issues.

Re-add some restricted IP ranges

2 participants

@robgjansen @stevenengler