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.

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

RadioTap flags support + Dot11 FCS&improvements #1381

Merged
merged 2 commits into from
May 18, 2018
Merged

RadioTap flags support + Dot11 FCS&improvements #1381

merged 2 commits into from
May 18, 2018

Conversation

gpotter2
Copy link
Member

@gpotter2 gpotter2 commented Apr 30, 2018 •
edited
Loading

This PR:

  • fixes Signal strength (RadioTap.dbm_antsignal) not working #1379 by improving RadioTap fields support. This required the implementation of ReversePadField (RadioTap padding is reversed), which itself required a pkt._tmp_dissect_pos)
  • Supports FCS in Dot11
  • introducing pkt._tmp_dissect_pos. It is a variable which contains the size of the current layer, at the time that it is used, and can only be used during the dissection. It is very useful in length_from functions, as it gets what has been exactly dissected when the lambda gets executed. It is changing dynamically
  • merging contrib/ppi with dot11: the PPI() layer was implemented twice.

@gpotter2 gpotter2 force-pushed the dot11improvements branch from f5e10d7 to 0fbdb8e Compare April 30, 2018 12:36
@codecov-io
Copy link

codecov-io commented Apr 30, 2018 •
edited
Loading

Codecov Report

Merging #1381 into master will increase coverage by 0.07%.
The diff coverage is 90.62%.

@@            Coverage Diff             @@
##           master    #1381      +/-   ##
==========================================
+ Coverage    85.2%   85.27%   +0.07%     
==========================================
  Files         174      174              
  Lines       40378    40609     +231     
==========================================
+ Hits        34403    34630     +227     
- Misses       5975     5979       +4
Impacted Files Coverage Δ
scapy/contrib/ppi_geotag.py 90% <100%> (ø) ⬆️
scapy/packet.py 76.41% <100%> (+0.08%) ⬆️
scapy/contrib/ppi_cace.py 90.38% <100%> (-0.19%) ⬇️
scapy/fields.py 90.88% <100%> (+0.05%) ⬆️
scapy/layers/bluetooth4LE.py 89.58% <100%> (ø) ⬆️
scapy/layers/ppi.py 84.09% <100%> (ø)
scapy/layers/dot11.py 90.4% <86.04%> (-0.54%) ⬇️
scapy/layers/ntp.py 91.51% <0%> (-0.27%) ⬇️
scapy/asn1/ber.py 82.28% <0%> (+0.28%) ⬆️
... and 3 more

@gpotter2 gpotter2 force-pushed the dot11improvements branch 2 times, most recently from 50b9da7 to a5c7165 Compare April 30, 2018 14:57
@gpotter2 gpotter2 changed the title [WIP] RadioTap flags support + Dot11 FCS&improvements RadioTap flags support + Dot11 FCS&improvements Apr 30, 2018
@gpotter2 gpotter2 mentioned this pull request Apr 30, 2018
Closed
@gpotter2 gpotter2 force-pushed the dot11improvements branch from a5c7165 to 8986872 Compare April 30, 2018 15:46
@calebmadrigal
Copy link

This seems to be working to me (tested on both Ubuntu Linux and macOS 10.13). Any idea when we can get scapy 2.4.1? I'd love to use this as soon as possible in my project (https://github.com/calebmadrigal/trackerjacker).

guedou
guedou reviewed May 2, 2018
View reviewed changes
scapy/layers/dot11.py Outdated
# Copyright (C) Philippe Biondi <phil@secdev.org>
# This program is published under a GPLv2 license
# PPI author: <jellch@harris.com>
Copy link
Member

Choose a reason for hiding this comment

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

Original PPI author?

Copy link
Member Author

Choose a reason for hiding this comment

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

From the contributor modules

Copy link
Member

Choose a reason for hiding this comment

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

I meant to change "PPI author:" to "Original PPI author:".

@guedou
Copy link
Member

guedou commented May 2, 2018

I restarted the failing test.

@gpotter2 gpotter2 force-pushed the dot11improvements branch 2 times, most recently from 9018bdf to 43557a8 Compare May 4, 2018 17:13
p-l-
p-l- requested changes May 9, 2018
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.

I think PPI related stuff should be handled in a dedicated layer file, not in dot11, as it is not specific to dot11. What do you think?

scapy/contrib/ppi_geotag.py Outdated
@@ -26,7 +26,7 @@
import time
from scapy.packet import *
from scapy.fields import *
from scapy.contrib.ppi import PPIGenericFldHdr, addPPIType
from scapy.layers.dot11 import *
Copy link
Member

Choose a reason for hiding this comment

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

Could you limit what is imported here?

@gpotter2 gpotter2 force-pushed the dot11improvements branch 2 times, most recently from bcde1d2 to 9ff889f Compare May 9, 2018 14:32
@gpotter2
Copy link
Member Author

gpotter2 commented May 9, 2018

@p-l- Updated: moved PPI to ppi.py

@gpotter2 gpotter2 force-pushed the dot11improvements branch from 9ff889f to 476fe17 Compare May 10, 2018 10:48
@p-l-
Copy link
Member

p-l- commented May 11, 2018

Can you rebase against current master? We have a conflict due to removed tests.

@gpotter2 gpotter2 force-pushed the dot11improvements branch from 476fe17 to 8ba45b7 Compare May 11, 2018 11:59
guedou
guedou approved these changes May 15, 2018
View reviewed changes
Copy link
Member

@guedou guedou left a comment

Choose a reason for hiding this comment

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

Looks good. @p-l- are the changes OK?

@guedou
Copy link
Member

guedou commented May 16, 2018

@gpotter2 this PR conflicts.

@gpotter2 gpotter2 force-pushed the dot11improvements branch from 8ba45b7 to a1e676e Compare May 16, 2018 11:02
@gpotter2
Copy link
Member Author

Fixed conflicts

@gpotter2 gpotter2 force-pushed the dot11improvements branch from a1e676e to 1b8ad56 Compare May 17, 2018 19:34
@p-l- p-l- merged commit 5335768 into secdev:master May 18, 2018
@gpotter2 gpotter2 deleted the dot11improvements branch May 18, 2018 17:14
@calebmadrigal
Copy link

@gpotter2 Any ETA on a 2.4.1 release? It'd be great prior to DEF CON 2018! :)

@gpotter2
Copy link
Member Author

Oh, you’re not the first one wanting 2.4.1 for DEF CON :) I have created a project a few days ago, but I doubt it will be ready for August...

Why do you need 2.4.1 ? Can’t you just use the dev build ? You can even install it via pip with git (lookup)

@calebmadrigal
Copy link

@gpotter2 I basically just need the stuff in this PR. I could use a dev build for personal use, but I'd like a nice clean pip "scapy==2.4.1" for dependency management.

@gpotter2
Copy link
Member Author

gpotter2 commented Jun 29, 2018 •
edited
Loading

We still have stuff to check :/
https://github.com/secdev/scapy/projects/2

Other maintainers don’t have much time to check all of those, as currently preparing for BlackHat.

For DEF CON you should be able to use the dev release, we’ll try to provide 2.4.1 ASAP

@guedou
Copy link
Member

guedou commented Jun 30, 2018 via email

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- requested changes

@guedou guedou guedou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Signal strength (RadioTap.dbm_antsignal) not working
5 participants
@gpotter2 @codecov-io @calebmadrigal @guedou @p-l-