Closed kruegger closed 6 years ago
@kruegger this looks great! thanks so much! Sorry about the delay, just catching up after vacation. The test is failing for me:
$ rebar3 do clean, compile, eunit
===> Cleaning out pkt...
===> Verifying dependencies...
===> Compiling pkt
===> Verifying dependencies...
===> Compiling pkt
===> Performing EUnit tests...
......................
Pending:
undefined
%% Unknown error: {abort,
{generator_failed,
{{pkt_802_1x_tests,codec_test_,0},
{error,function_clause,
[{pkt_ether,type,
[34958],
[{file,
"/home/msantos/src/erlang/pkt/_build/test/lib/pkt/src/pkt_ether.erl"},
{line,37}]},
{pkt,decapsulate_next,2,
[{file,
"/home/msantos/src/erlang/pkt/_build/test/lib/pkt/src/pkt.erl"},
{line,122}]},
{pkt_802_1x_tests,decode,0,
[{file,
"/home/msantos/src/erlang/pkt/_build/test/lib/pkt/test/pkt_802_1x_tests.erl"},
{line,21}]},
{pkt_802_1x_tests,codec_test_,0,
[{file,
"/home/msantos/src/erlang/pkt/_build/test/lib/pkt/test/pkt_802_1x_tests.erl"},
{line,8}]}]}}}}
Finished in ? seconds
25 tests, 0 failures, 3 cancelled
===> Error running tests
Seems like some boilerplate just needs to be added to pkt.erl
. There is also a LLC header in the test packet that needs to be accounted for.
diff --git a/src/pkt.erl b/src/pkt.erl
index abc1921..d9239f8 100644
--- a/src/pkt.erl
+++ b/src/pkt.erl
@@ -43,6 +43,7 @@
ether_type/1,
mpls/1,
'802.1q'/1,
+ '802.1x'/1,
llc/1,
arp/1,
rarp/1,
@@ -129,6 +130,9 @@ decapsulate_next({'802.1q', Data}, Headers) ->
decapsulate_next({'802.1qinq', Data}, Headers) ->
{Header, Payload} = '802.1q'(Data),
decapsulate_next({next(Header), Payload}, [Header|Headers]);
+decapsulate_next({'802.1x', Data}, Headers) ->
+ {Header, Payload} = '802.1x'(Data),
+ decapsulate_next({next(Header), Payload}, [Header|Headers]);
decapsulate_next({llc, Data}, Headers) ->
{Header, Payload} = llc(Data),
lists:reverse([Payload, Header|Headers]);
@@ -244,6 +248,7 @@ decode_next({Proto, Data}, Headers) when
Proto =:= linux_cooked;
Proto =:= null;
Proto =:= '802.1q';
+ Proto =:= '802.1x';
Proto =:= ipv6_ah;
Proto =:= ipv6_dstopts;
@@ -282,6 +287,7 @@ decode_next({ipv6_none, Data}, Headers) ->
decode_next({Proto, Data}, Headers) when
Proto =:= arp;
Proto =:= rarp;
+ Proto =:= llc;
Proto =:= lldp;
Proto =:= icmp;
Proto =:= icmp6;
@@ -302,6 +308,7 @@ next(#null{family = Family}) -> family(Family);
next(#linux_cooked{pro = Pro}) -> ether_type(Pro);
next(#ether{type = Type}) -> ether_type(Type);
next(#'802.1q'{type = Type}) -> ether_type(Type);
+next(#'802.1x'{type = Type}) -> ether_type(Type);
next(#ipv4{p = P}) -> ipproto(P);
next(#gre{type = Type}) -> ether_type(Type);
next(#ipv6{next = Next}) -> ipproto(Next);
@@ -349,6 +356,9 @@ llc(N) ->
'802.1q'(N) ->
pkt_802_1q:codec(N).
+'802.1x'(N) ->
+ pkt_802_1x:codec(N).
+
%% IPv4
ipv4(N) ->
pkt_ipv4:codec(N).
diff --git a/src/pkt_ether.erl b/src/pkt_ether.erl
index 77ea6e3..fd8c7a4 100644
--- a/src/pkt_ether.erl
+++ b/src/pkt_ether.erl
@@ -45,6 +45,8 @@ type(EtherType) when EtherType < 16#05DC -> llc;
type(?ETH_P_802_1Q) -> '802.1q';
%% 802.1ad (802.1q QinQ)
type(?ETH_P_802_1QinQ) -> '802.1qinq';
+%% 802.1X
+type(?ETH_P_802_1X) -> '802.1x';
%% MPLS_
type(?ETH_P_MPLS_UNI) -> mpls;
type(?ETH_P_MPLS_MULTI) -> mpls.
diff --git a/test/pkt_802_1x_tests.erl b/test/pkt_802_1x_tests.erl
index 9a293d9..c36bdb5 100644
--- a/test/pkt_802_1x_tests.erl
+++ b/test/pkt_802_1x_tests.erl
@@ -18,7 +18,7 @@ packet() ->
202,98,8,201,119,185,73,11,98,90,123,40,181,73,189,189,89,19,137>>.
decode() ->
- [_Ether, EAPoL, _Payload] = pkt:decapsulate(packet()),
+ [_Ether, EAPoL, _LLC, _Payload] = pkt:decapsulate(packet()),
?_assertEqual(
{'802.1x',2, 3, 127 },
EAPoL
@msantos Thanks for the comment and my apology for missing merge code which I thought I checked in last year.
On your comment on LLC handling and following code: @@ -302,6 +308,7 @@ next(#null{family = Family}) -> family(Family); next(#linux_cooked{pro = Pro}) -> ether_type(Pro); next(#ether{type = Type}) -> ether_type(Type); next(#'802.1q'{type = Type}) -> ether_type(Type); +next(#'802.1x'{type = Type}) -> ether_type(Type); next(#ipv4{p = P}) -> ipproto(P); next(#gre{type = Type}) -> ether_type(Type); next(#ipv6{next = Next}) -> ipproto(Next);
IMHO, I think, we shouldn't do this since the payload contains the message that requires additional decoders of various applications. The test PDU happens to be EAPoL-Key(type 16#0003) message which needs decoder. It is not cascaded other type of Layer 2 message.
What do you think?
Agreed, my mistake, thanks for the clarification. Merged, thank you!
Hi
I have added the 802.1x EAPoL PDU header support. Please, review and let me know.