basic EtherCat layer #569

speakinghedge · 2017-03-19T20:30:04Z

This pull request adds basic support for Type 12 EtherCat messages (IEC 61158-4-12).

As the EtherCat protocol uses LittleEndian byte order - even for BitFields - this also adds LEBitFields support. There is a small regression test on top of the EtherCat-tests - but I don't know if the whole LEBitField implementation is mature enough...

gpotter2 · 2017-03-29T11:59:49Z

scapy/contrib/EtherCat.py

+        if pad_len > 0:
+
+            pad = Padding()
+            pad.load = '\x00' * pad_len


Please adapt your code to the new contributing rules:

That only means using b'\x0....' instead of '\x0....'

will do. there is also a bug in the frame padding (frames are to big).

done. padding is fixed & tested against real devices.

gpotter2 · 2017-03-29T12:04:49Z

scapy/fields.py

+        as we don't know the final size of the full bitfield we need to accumulate the data.
+        if we reach a field that ends at a octet boundary, we build the whole string
+
+        !!! NEVER MIX LEBitField with BitField !!!


What do you mean ?

Would it be possible to check that and raise an error ?

I think so and I will add an exception. It was not completely clear to me how to handle that kind of internal error as I was not able to derive some kind of rule by looking at the remaining code base 😄

edit: typo.

addressed by 3fd2b76, adds explicit check for lower/upper layer sharing LEBitField type

p-l- · 2017-04-18T20:27:05Z

Hi,

I'm not sure what you mean by BitFields being LE, but if I understand correctly, you can use regular BitFields and set negative lengths.

speakinghedge · 2017-04-19T20:09:12Z

Hi,

negative size works only on BitFields using 16 or 32 bit length.

quick example (number after the # is assigned to the field):

class BitFieldReverseTest(Packet):
    fields_desc = [
        BitField('a', 0, 16), # 0xabcd
        BitField('b', 0, -16), # 0xabcd
        BitField('c0', 0, -4), # 0xa
        BitField('c1', 0, -4), # 0xb
        BitField('c2', 0, -4), # 0xc
        BitField('c3', 0, -4), # 0xd
        BitField('d0', 0, -4), # 0x5
        BitField('d1', 0, -16), # 0x1234
        BitField('d2', 0, -4), # 0x6
    ]

gives (added some spaces to separate fields)

ABCD CDAB A B C D 5 3412 6

As EtherCat uses some 2-11-3-sized-fields negative field length was no option as this is not handled by the current implementation. Even worse: it silently returns the unchanged value - maybe an Exception should be thrown here as this is not the expected behavior (as shown by your comment too).

Open question: what is the effort to add the ability to handle non 16/32 bit sized fields with the stock-BitField class....

codecov-io · 2017-06-13T22:08:08Z

Codecov Report

Merging #569 into master will increase coverage by 0.12%.
The diff coverage is 97.69%.

@@            Coverage Diff            @@
##           master    #569      +/-   ##
=========================================
+ Coverage   83.68%   83.8%   +0.12%     
=========================================
  Files         159     160       +1     
  Lines       38120   38337     +217     
=========================================
+ Hits        31899   32129     +230     
+ Misses       6221    6208      -13

Impacted Files	Coverage Δ
scapy/fields.py	`90.87% <ø> (+0.17%)`	⬆️
scapy/contrib/ethercat.py	`97.69% <97.69%> (ø)`
scapy/layers/inet6.py	`78.2% <0%> (ø)`	⬆️
scapy/asn1/ber.py	`82.28% <0%> (+0.28%)`	⬆️
scapy/layers/tls/basefields.py	`80.27% <0%> (+0.68%)`	⬆️
scapy/layers/tls/record.py	`92.08% <0%> (+0.87%)`	⬆️
scapy/layers/tls/record_sslv2.py	`88.82% <0%> (+1.67%)`	⬆️
scapy/layers/tls/keyexchange.py	`84.76% <0%> (+1.71%)`	⬆️

guedou · 2017-12-16T16:50:40Z

@speakinghedge could you rebase you changes ? Thanks.

speakinghedge · 2017-12-21T22:40:41Z

did. also reduced the plethora of commits to some meaningful ones...

p-l- · 2018-01-15T09:06:17Z

scapy/contrib/EtherCat.uts

@@ -0,0 +1,256 @@
+from enum import test


What is this?

fixed in ae0317c (I was fooled by pycharm. again. sorry.)

p-l- · 2018-01-15T09:07:52Z

Please rebase against current master as we now have Python 3 tests. Also , do not use _ in your tests as they break because of that.

speakinghedge · 2018-01-20T16:43:10Z

underscores removed in 49fd726.

maybe this should be added to your contribution rules.

speakinghedge · 2018-01-20T18:37:52Z

I will take care for the Python 3 related issues.

guedou · 2018-01-24T10:27:26Z

@speakinghedge #1090 updates the documentation. Thanks for the idea.

speakinghedge · 2018-02-07T07:30:45Z

I was really busy - will start working on this later today.

speakinghedge · 2018-02-12T19:57:07Z

I was really busy - will start working on this later today.

...6 days ago. naa. that human timescale is wrong.

guedou · 2018-02-23T07:46:28Z

@speakinghedge I restarted the failing osx test and it is now ok. Could you rename your files to ethercat.py and ethertypes.uts ?

gpotter2 · 2018-02-23T11:22:46Z

You certainly mean ethercat.uts

speakinghedge · 2018-02-23T15:42:51Z

@speakinghedge I restarted the failing osx test and it is now ok. Could you rename your files to ethercat.py and ethertypes.uts ?

done in c4b5843.

how could this survive that long? 😃

guedou · 2018-02-24T08:49:43Z

scapy/contrib/ethercat.uts

+assert(len(frm) == 60)
+frm = Ether(frm.do_build())
+assert(frm[EtherCat].length == 10)
+frm = Ether()/EtherCat()/Raw('012345678901234567890123456789012345678901234567890123456789')


You need to write:

frm = Ether()/EtherCat()/Raw(b'012345678901234567890123456789012345678901234567890123456789')

The same comment apply to other part of your code.

guedou · 2018-02-24T08:50:07Z

Sorry, I have more requests:

please remove the new fields from fields.py and move them to ethercat.py as they are specific to your changes
rebase against master. Your tests will fail on Python3.

p-l- · 2018-02-25T20:54:19Z

scapy/fields.py

@@ -1121,6 +1121,7 @@ def any2i(self, pkt, x):
    def i2repr(self, pkt, x):
        return _EnumField.i2repr(self, pkt, x)

+


Could you please remove this useless change to preserve this file's code history? Thanks!

as I touched that part of the code (by adding new fields but later on I moved them to another location) I honored the boy scout rule and added a newline between the two classes to follow pep8. but I see your point - 3da7f3f

dsheets · 2018-04-01T11:27:32Z

What happened to this PR? I am very interested in this functionality.

gpotter2 · 2018-04-01T11:52:18Z

It seems that the PR moved to https://github.com/speakinghedge/scapy/tree/EtherCat-support from speakinghedge/master, and he did not reopen the PR

speakinghedge · 2018-04-03T18:46:42Z

Sorry - I lost somehow track - this 'close and move' didn't happen on purpose. Seems https://github.com/speakinghedge/scapy/tree/EtherCat-support is also in a bad state.

I will go and

create a new clean branch from master and add all EtherCat related changes on top and
open an new PR.

Side note: I'm not able to test this against real hardware anymore (as I switched the employer).

this is the cleaned up version of broken PR secdev#569 it incorporates all requested changes up to 3da7f3f sorry for the noise.

gpotter2 reviewed Mar 29, 2017

View reviewed changes

guedou added the conflicts label Dec 16, 2017

speakinghedge force-pushed the master branch from 181acb2 to 45f8892 Compare December 21, 2017 22:32

guedou removed the conflicts label Dec 22, 2017

p-l- reviewed Jan 15, 2018

View reviewed changes

speakinghedge force-pushed the master branch from 45f8892 to 49fd726 Compare January 20, 2018 16:39

guedou approved these changes Feb 23, 2018

View reviewed changes

guedou reviewed Feb 24, 2018

View reviewed changes

speakinghedge force-pushed the master branch from c4b5843 to cea0723 Compare February 24, 2018 13:40

p-l- requested changes Feb 25, 2018

View reviewed changes

speakinghedge closed this Mar 12, 2018

speakinghedge force-pushed the master branch from 3da7f3f to 24fe55a Compare March 12, 2018 10:17

speakinghedge mentioned this pull request Apr 3, 2018

EtherCat support #1313

Merged

speakinghedge added a commit to speakinghedge/scapy that referenced this pull request Apr 3, 2018

add EtherCat support

cb335e0

this is the cleaned up version of broken PR secdev#569 it incorporates all requested changes up to 3da7f3f sorry for the noise.

guedou pushed a commit to guedou/scapy-issues that referenced this pull request Apr 18, 2018

add EtherCat support

bc0a17c

this is the cleaned up version of broken PR secdev#569 it incorporates all requested changes up to 3da7f3f sorry for the noise.

rugeGerritsen mentioned this pull request Dec 22, 2020

btle-rf: Define previous RFU flags to create better Wireshark logs #3027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

basic EtherCat layer #569

basic EtherCat layer #569

speakinghedge commented Mar 19, 2017

gpotter2 Mar 29, 2017

speakinghedge Apr 19, 2017

speakinghedge Jun 13, 2017

gpotter2 Mar 29, 2017

speakinghedge Apr 19, 2017 •

edited

Loading

speakinghedge Jun 13, 2017

p-l- commented Apr 18, 2017

speakinghedge commented Apr 19, 2017

codecov-io commented Jun 13, 2017 •

edited

Loading

guedou commented Dec 16, 2017

speakinghedge commented Dec 21, 2017

p-l- Jan 15, 2018

speakinghedge Jan 20, 2018

p-l- commented Jan 15, 2018

speakinghedge commented Jan 20, 2018

speakinghedge commented Jan 20, 2018

guedou commented Jan 24, 2018

speakinghedge commented Feb 7, 2018

speakinghedge commented Feb 12, 2018

guedou commented Feb 23, 2018

gpotter2 commented Feb 23, 2018

speakinghedge commented Feb 23, 2018

guedou Feb 24, 2018

guedou commented Feb 24, 2018

p-l- Feb 25, 2018

speakinghedge Feb 28, 2018

dsheets commented Apr 1, 2018

gpotter2 commented Apr 1, 2018

speakinghedge commented Apr 3, 2018

		@@ -1121,6 +1121,7 @@ def any2i(self, pkt, x):
		def i2repr(self, pkt, x):
		return _EnumField.i2repr(self, pkt, x)

basic EtherCat layer #569

basic EtherCat layer #569

Conversation

speakinghedge commented Mar 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speakinghedge Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-l- commented Apr 18, 2017

speakinghedge commented Apr 19, 2017

codecov-io commented Jun 13, 2017 • edited Loading

Codecov Report

guedou commented Dec 16, 2017

speakinghedge commented Dec 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-l- commented Jan 15, 2018

speakinghedge commented Jan 20, 2018

speakinghedge commented Jan 20, 2018

guedou commented Jan 24, 2018

speakinghedge commented Feb 7, 2018

speakinghedge commented Feb 12, 2018

guedou commented Feb 23, 2018

gpotter2 commented Feb 23, 2018

speakinghedge commented Feb 23, 2018

Choose a reason for hiding this comment

guedou commented Feb 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsheets commented Apr 1, 2018

gpotter2 commented Apr 1, 2018

speakinghedge commented Apr 3, 2018

speakinghedge Apr 19, 2017 •

edited

Loading

codecov-io commented Jun 13, 2017 •

edited

Loading