Skip to content

p0f rewritten #1923

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

p0f rewritten #1923

wants to merge 9 commits into from

Conversation

hissssst
Copy link

@hissssst hissssst commented Mar 21, 2019 •
edited
Loading

  • It is currently written in python3, but I will definetely rewrite it in six
  • It's not much, because I don't want to find out that something went wrong in the beginning after writing many lines of code
  • There are some comments for the reviewer. They will be definetely rewritten in the later versions of this file

gpotter2
gpotter2 reviewed Mar 21, 2019
View reviewed changes
Copy link
Member

@gpotter2 gpotter2 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution !

However, there isn’t much left :/ all the actually useful functions have been removed :/ do you think you could make them work ?

The new parsing is however great !

@codecov
Copy link

codecov bot commented Mar 27, 2019 •
edited
Loading

Codecov Report

Merging #1923 into master will decrease coverage by 22.64%.
The diff coverage is 10%.

@@             Coverage Diff             @@
##           master    #1923       +/-   ##
===========================================
- Coverage   85.89%   63.24%   -22.65%     
===========================================
  Files         187      126       -61     
  Lines       42746    30506    -12240     
===========================================
- Hits        36716    19295    -17421     
- Misses       6030    11211     +5181
Impacted Files Coverage Δ
scapy/modules/p0f.py 13.54% <10%> (-51%) ⬇️
scapy/sessions.py 26.08% <0%> (-73.92%) ⬇️
scapy/plist.py 18.57% <0%> (-67.49%) ⬇️
scapy/pton_ntop.py 30.13% <0%> (-67.13%) ⬇️
scapy/layers/tls/crypto/hkdf.py 35.55% <0%> (-64.45%) ⬇️
scapy/pipetool.py 26.26% <0%> (-63.26%) ⬇️
scapy/main.py 18.87% <0%> (-56.62%) ⬇️
scapy/utils6.py 33.78% <0%> (-53.43%) ⬇️
scapy/sendrecv.py 32.14% <0%> (-52.81%) ⬇️
scapy/scapypipes.py 33.2% <0%> (-52.15%) ⬇️
... and 132 more

p-l-
p-l- reviewed Sep 17, 2019
View reviewed changes
Copy link
Member

@p-l- p-l- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!
I have a couple of suggestions.
BTW, since p0f v2 and v3 differ a lot, what would you think of renaming the existing module (maybe to something like p0fv2) so that we can have both?

scapy/modules/p0f.py
Only Python 3 compatible
"""
a = line.split(splitchar)[:n]
yield from a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yield from a
for elt in a:
yield elt

scapy/modules/p0f.py
"""
a = line.split(splitchar)[:n]
yield from a
yield from [default] * (n - len(a))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yield from [default] * (n - len(a))
for _ in range(n - len(a)):
yield default

scapy/modules/p0f.py
def lparse(line, n, default='', splitchar=':'):
"""
Function for nice parcing of 'a:b:c:d:e' lines
Only Python 3 compatible
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Only Python 3 compatible

scapy/modules/p0f.py
gen = p0fdb.tcp_correl(direction, p0f_out, olayout, quirks)
return max(list(gen), key=lambda x: sum(x[0]))[1]

if __name__ == '__main__':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should go in a separate .uts file rather than here.

@gpotter2 gpotter2 mentioned this pull request Mar 10, 2020
Closed
@guedou
Copy link
Member

guedou commented Mar 10, 2020

I added a reference to this PR in #399 and I am closing it due to no activity.

@guedou guedou closed this Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-l- p-l- p-l- left review comments

@gpotter2 gpotter2 gpotter2 left review comments

Assignees
No one assigned
Labels
needs rework/cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hissssst @guedou @p-l- @gpotter2