Skip to content

Detect the location of p0f fingerprint libraries. #1839

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

Closed

Conversation

william-stearns
Copy link

This is just a checklist to guide you. You can remove it safely.

Checklist:

  • If you are new to Scapy: I have checked https://github.com/secdev/scapy/blob/master/CONTRIBUTING.md (esp. section submitting-pull-requests)
  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests for Python2 and Python3 (using tox or, cd test && ./run_tests_py2, cd test && ./run_tests_py3)
    I don't believe the unit tests would apply for this type of fix - I'd have to rename or move files around on the user's system to do so.
    A number of the tests fail on my Mac OS, but the failures do not seem to be related to the patch.

brief description what this PR will do, e.g. fixes broken dissection of XXX
Finds the p0f fingerprint files the 2 common alternate locations, falling back on /etc/p0f as a default.

if required - short explanation why you fixed something in a way that may look more complicated as it actually is

if required - outline impacts on other parts of the library

@codecov
Copy link

codecov bot commented Feb 9, 2019

Codecov Report

Merging #1839 into master will increase coverage by 0.03%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #1839      +/-   ##
==========================================
+ Coverage   85.46%   85.49%   +0.03%     
==========================================
  Files         185      185              
  Lines       42494    42510      +16     
==========================================
+ Hits        36319    36346      +27     
+ Misses       6175     6164      -11
Impacted Files Coverage Δ
scapy/modules/p0f.py 63.97% <60%> (-0.58%) ⬇️
scapy/layers/ntp.py 91.51% <0%> (-0.27%) ⬇️
scapy/sendrecv.py 85.03% <0%> (+0.17%) ⬆️
scapy/layers/inet6.py 88.23% <0%> (+0.23%) ⬆️
scapy/arch/windows/__init__.py 71.99% <0%> (+0.31%) ⬆️
scapy/automaton.py 82.36% <0%> (+0.55%) ⬆️
scapy/arch/pcapdnet.py 58.09% <0%> (+0.71%) ⬆️
scapy/pton_ntop.py 97.26% <0%> (+8.21%) ⬆️

@codecov
Copy link

codecov bot commented Feb 9, 2019

Codecov Report

Merging #1839 into master will increase coverage by 0.04%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #1839      +/-   ##
==========================================
+ Coverage   85.46%   85.51%   +0.04%     
==========================================
  Files         185      185              
  Lines       42494    42510      +16     
==========================================
+ Hits        36319    36351      +32     
+ Misses       6175     6159      -16
Impacted Files Coverage Δ
scapy/modules/p0f.py 63.97% <60%> (-0.58%) ⬇️
scapy/layers/ntp.py 91.51% <0%> (-0.27%) ⬇️
scapy/layers/inet6.py 88.28% <0%> (+0.28%) ⬆️
scapy/arch/windows/__init__.py 71.99% <0%> (+0.31%) ⬆️
scapy/sendrecv.py 85.2% <0%> (+0.34%) ⬆️
scapy/automaton.py 82.36% <0%> (+0.55%) ⬆️
scapy/arch/pcapdnet.py 58.09% <0%> (+0.71%) ⬆️
scapy/layers/tls/record_sslv2.py 87.7% <0%> (+1.67%) ⬆️
scapy/pton_ntop.py 97.26% <0%> (+8.21%) ⬆️

@guedou
Copy link
Member

guedou commented Feb 13, 2019

Thanks for your PR.

Could you implement the selection logic in a function to remove the redundant code?

For example:

def p0f_select_path(filename):
    path = "/etc/p0f/%s" % filename
    for directory in ["/usr/share/p0f/", "/opt/local/"]:
        if os.path.exists("%s/%s" % (directory, filename)):
            path = "%s/%s" % (directory, filename)
    return path

@p-l-
Copy link
Member

p-l- commented Feb 14, 2019

Also, I think files in /etc/p0f should be used first if they exist, then /usr/share/p0f, then /opt/local.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants