Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Palo Alto ARP table plugin #2613
Palo Alto ARP table plugin #2613
Changes from all commits
769e96d
a779f85
4c09030
857efb9
a56f641
4b46355
a8ddd84
5d05e27
671e13a
ee7f680
3af3a05
118ac26
adeb592
877a483
77852a0
0689720
1da5e40
677709a
cac72d4
f103424
d716984
772aea9
ba9ce3d
d246665
82aae58
acaceb5
cd390f5
a00e9af
303197f
7ff53ee
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 56 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L55-L56
Check warning on line 63 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L63
Check warning on line 72 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L72
Check warning on line 75 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L75
Check warning on line 80 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L77-L80
Check warning on line 82 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L82
Check warning on line 84 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L84
Check warning on line 92 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L92
Check warning on line 104 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L104
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not accustomed to using the Twisted client libraries for this sort of thing. Does this in fact mean that we explicitly turn off TLS certificate verification? If so, is that really what we want when talking to a security-specific product? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this at UiT last year. Since we are only extracting ARP data this should be fine as long as the keys and permissions on the Palo Alto device are configured correctly (To only allow for fetching ARP data). But yes we are explicitly turning of TLS certificate verification on all requests made by the plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to implement certificate pinning instead of this solution just in case someone manages to misconfigure their permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A MITM attack might also allow for data infiltration, certificate pinning is starting to seem like the best option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To get this PR done with, I would just make note of this fact in the docs. That way, you could submit a new PR with changes for certificate config options (i.e. switch verification on/off or point to a pinned certificate)
Check warning on line 121 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L120-L121
Check warning on line 125 in python/nav/ipdevpoll/plugins/paloaltoarp.py
Codecov / codecov/patch
python/nav/ipdevpoll/plugins/paloaltoarp.py#L125