[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

2013-10-11 Thread Robin Sommer (JIRA) 

[ 
https://bro-tracker.atlassian.net/browse/BIT-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14211#comment-14211
 ] 

Robin Sommer commented on BIT-1045:
---

Going through, I see number of places where I'd argue it's actually a 
programming/logic error that's not something that can be directly/just 
triggered by crafted network traffic. Examples are the RefCnt() checks in 
~ConnectionTimer() and the indent_level check in ODesc. I'm inclined to leave 
them as they were, with the argument being that those kinds of error actually 
*are* best to trigger an abort. E.g, if the reference counting goes awry, 
pretty much all bets are off anyways, and I'd rather have Bro terminate than 
trying to continue. 

So I think the guideline should be avoiding internal errors that happen 
*directly* because of broken network input; not because of (for lack of a 
better term) infrastructure problems in other parts of Bro. (Although I'm 
sure as I go further, I'll find more cases where that definition is  ambiguous  
as well.)

What's your opinion on cases like the above? 

What I could do is go through your diffs and adapt with the above in mind, and 
then we can do another iteration and see if/where we agree. 


 Review usage of InternalError when parsing network traffic
 --

 Key: BIT-1045
 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
 Project: Bro Issue Tracker
  Issue Type: Task
  Components: Bro
Affects Versions: git/master, 2.1
Reporter: Vlad Grigorescu
Assignee: Robin Sommer

 Creating issue for tracking purposes.
 Reporter-InternalError denotes a fatal error, and will cause Bro to stop. 
 Calling this function when parsing network traffic creates the possibility 
 for an attacker using a packet of death, which could stop Bro.
 I suspect that in most cases, a weird should be generated instead, and Bro 
 should just move on to the next packet. A quick grep shows some likely 
 candidates for incorrect use of InternalError:
 src/Sessions.cc:  reporter-InternalError(Bad IP protocol 
 version in DoNextInnerPacket);
 src/Sessions.cc:  reporter-InternalError(fragment block not in 
 dictionary);
 src/Sessions.cc:  reporter-InternalError(fragment block 
 missing);
 src/Sessions.cc:  reporter-InternalError(unknown 
 transport protocol);
 src/Frag.cc:  reporter-InternalError(bad IP version in fragment 
 reassembly);
 src/IP.cc:reporter-InternalError(IPv6_HdrChain::Init with 
 truncated IP header);
 src/IP.cc:reporter-InternalError(IPv6_Hdr_Chain bad 
 header %d, type);
 src/IP.h: reporter-InternalError(bad IP version in 
 IP_Hdr ctor);
 src/RSH.cc:   reporter-InternalError(multiple rsh client names);
 src/RSH.cc:   reporter-InternalError(multiple rsh initial client 
 names);
 src/POP3.cc:  reporter-InternalError(command not known);
 src/Rlogin.cc:reporter-InternalError(multiple rlogin client 
 names);
 src/ICMP.cc:  reporter-InternalError(unexpected IP proto in 
 ICMP analyzer: %d,
 src/ICMP.cc:  reporter-InternalError(unexpected next protocol in 
 ICMP::DeliverPacket());
 src/SMB.cc:   reporter-InternalError(command mismatch for 
 ParseTransaction);
 src/HTTP.cc:  reporter-InternalError(unrecognized HTTP message 
 event);
 src/HTTP.cc:  reporter-InternalError(HTTP ParseRequest failed);
 src/DPM.cc:   reporter-InternalError(unknown protocol);
 src/RPC.cc:   reporter-InternalError(RPC underflow);
 src/RPC.cc:   reporter-InternalError(RPC resync: skipping 
 over data failed);
 src/RPC.cc:   
 reporter-InternalError(inconsistent RPC record marker extraction);



--
This message was sent by Atlassian JIRA
(v6.1-OD-09-WN#6144)
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev

[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

2013-10-10 Thread Jon Siwek (JIRA) 

 [ 
https://bro-tracker.atlassian.net/browse/BIT-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jon Siwek updated BIT-1045:
---

Assignee: Robin Sommer

 Review usage of InternalError when parsing network traffic
 --

 Key: BIT-1045
 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
 Project: Bro Issue Tracker
  Issue Type: Task
  Components: Bro
Affects Versions: git/master, 2.1
Reporter: Vlad Grigorescu
Assignee: Robin Sommer

 Creating issue for tracking purposes.
 Reporter-InternalError denotes a fatal error, and will cause Bro to stop. 
 Calling this function when parsing network traffic creates the possibility 
 for an attacker using a packet of death, which could stop Bro.
 I suspect that in most cases, a weird should be generated instead, and Bro 
 should just move on to the next packet. A quick grep shows some likely 
 candidates for incorrect use of InternalError:
 src/Sessions.cc:  reporter-InternalError(Bad IP protocol 
 version in DoNextInnerPacket);
 src/Sessions.cc:  reporter-InternalError(fragment block not in 
 dictionary);
 src/Sessions.cc:  reporter-InternalError(fragment block 
 missing);
 src/Sessions.cc:  reporter-InternalError(unknown 
 transport protocol);
 src/Frag.cc:  reporter-InternalError(bad IP version in fragment 
 reassembly);
 src/IP.cc:reporter-InternalError(IPv6_HdrChain::Init with 
 truncated IP header);
 src/IP.cc:reporter-InternalError(IPv6_Hdr_Chain bad 
 header %d, type);
 src/IP.h: reporter-InternalError(bad IP version in 
 IP_Hdr ctor);
 src/RSH.cc:   reporter-InternalError(multiple rsh client names);
 src/RSH.cc:   reporter-InternalError(multiple rsh initial client 
 names);
 src/POP3.cc:  reporter-InternalError(command not known);
 src/Rlogin.cc:reporter-InternalError(multiple rlogin client 
 names);
 src/ICMP.cc:  reporter-InternalError(unexpected IP proto in 
 ICMP analyzer: %d,
 src/ICMP.cc:  reporter-InternalError(unexpected next protocol in 
 ICMP::DeliverPacket());
 src/SMB.cc:   reporter-InternalError(command mismatch for 
 ParseTransaction);
 src/HTTP.cc:  reporter-InternalError(unrecognized HTTP message 
 event);
 src/HTTP.cc:  reporter-InternalError(HTTP ParseRequest failed);
 src/DPM.cc:   reporter-InternalError(unknown protocol);
 src/RPC.cc:   reporter-InternalError(RPC underflow);
 src/RPC.cc:   reporter-InternalError(RPC resync: skipping 
 over data failed);
 src/RPC.cc:   
 reporter-InternalError(inconsistent RPC record marker extraction);



--
This message was sent by Atlassian JIRA
(v6.1-OD-09-WN#6144)
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev

[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

2013-07-29 Thread Robin Sommer (JIRA) 

[ 
https://bro-tracker.atlassian.net/browse/BIT-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13400#comment-13400
 ] 

Robin Sommer commented on BIT-1045:
---





Ack, InternalError() is not something that external input should be
able to trigger. I already removed a number of these over time, but
never looked systematically for them.


Agreed, though sometimes they aren't about the traffic but about a
logic error in decoding it; it would be good to still differentiate
those cases from a broken packet, however indeed without aborting.



 Review usage of InternalError when parsing network traffic
 --

 Key: BIT-1045
 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
 Project: Bro Issue Tracker
  Issue Type: Task
  Components: Bro
Affects Versions: git/master, 2.1
Reporter: Vlad Grigorescu

 Creating issue for tracking purposes.
 Reporter-InternalError denotes a fatal error, and will cause Bro to stop. 
 Calling this function when parsing network traffic creates the possibility 
 for an attacker using a packet of death, which could stop Bro.
 I suspect that in most cases, a weird should be generated instead, and Bro 
 should just move on to the next packet. A quick grep shows some likely 
 candidates for incorrect use of InternalError:
 src/Sessions.cc:  reporter-InternalError(Bad IP protocol 
 version in DoNextInnerPacket);
 src/Sessions.cc:  reporter-InternalError(fragment block not in 
 dictionary);
 src/Sessions.cc:  reporter-InternalError(fragment block 
 missing);
 src/Sessions.cc:  reporter-InternalError(unknown 
 transport protocol);
 src/Frag.cc:  reporter-InternalError(bad IP version in fragment 
 reassembly);
 src/IP.cc:reporter-InternalError(IPv6_HdrChain::Init with 
 truncated IP header);
 src/IP.cc:reporter-InternalError(IPv6_Hdr_Chain bad 
 header %d, type);
 src/IP.h: reporter-InternalError(bad IP version in 
 IP_Hdr ctor);
 src/RSH.cc:   reporter-InternalError(multiple rsh client names);
 src/RSH.cc:   reporter-InternalError(multiple rsh initial client 
 names);
 src/POP3.cc:  reporter-InternalError(command not known);
 src/Rlogin.cc:reporter-InternalError(multiple rlogin client 
 names);
 src/ICMP.cc:  reporter-InternalError(unexpected IP proto in 
 ICMP analyzer: %d,
 src/ICMP.cc:  reporter-InternalError(unexpected next protocol in 
 ICMP::DeliverPacket());
 src/SMB.cc:   reporter-InternalError(command mismatch for 
 ParseTransaction);
 src/HTTP.cc:  reporter-InternalError(unrecognized HTTP message 
 event);
 src/HTTP.cc:  reporter-InternalError(HTTP ParseRequest failed);
 src/DPM.cc:   reporter-InternalError(unknown protocol);
 src/RPC.cc:   reporter-InternalError(RPC underflow);
 src/RPC.cc:   reporter-InternalError(RPC resync: skipping 
 over data failed);
 src/RPC.cc:   
 reporter-InternalError(inconsistent RPC record marker extraction);

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://bro-tracker.atlassian.net/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev

[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

2013-07-29 Thread Vern Paxson (JIRA) 

[ 
https://bro-tracker.atlassian.net/browse/BIT-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13401#comment-13401
 ] 

Vern Paxson commented on BIT-1045:
--


In line with what you frame, the history behind these is that they're meant
to reflect should-never-happen situations; not weird activity, but apparent
internal inconsistencies in Bro's processing/execution.  So they don't really
fit with the notion of weird.  (Of course it's definitely possible there's
some mission-creep and InternalError is misused when Weird really is the
right notion.)

That said, for sure I agree that we don't want to give adversaries a way
to tickle a Bro crash.  So ideally the solution here would be to come up
with something similar to the weird/notice framework, but that expicitly
captures the notion that Bro-is-confused rather than
something-happened-on-the-network.

Vern


 Review usage of InternalError when parsing network traffic
 --

 Key: BIT-1045
 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
 Project: Bro Issue Tracker
  Issue Type: Task
  Components: Bro
Affects Versions: git/master, 2.1
Reporter: Vlad Grigorescu

 Creating issue for tracking purposes.
 Reporter-InternalError denotes a fatal error, and will cause Bro to stop. 
 Calling this function when parsing network traffic creates the possibility 
 for an attacker using a packet of death, which could stop Bro.
 I suspect that in most cases, a weird should be generated instead, and Bro 
 should just move on to the next packet. A quick grep shows some likely 
 candidates for incorrect use of InternalError:
 src/Sessions.cc:  reporter-InternalError(Bad IP protocol 
 version in DoNextInnerPacket);
 src/Sessions.cc:  reporter-InternalError(fragment block not in 
 dictionary);
 src/Sessions.cc:  reporter-InternalError(fragment block 
 missing);
 src/Sessions.cc:  reporter-InternalError(unknown 
 transport protocol);
 src/Frag.cc:  reporter-InternalError(bad IP version in fragment 
 reassembly);
 src/IP.cc:reporter-InternalError(IPv6_HdrChain::Init with 
 truncated IP header);
 src/IP.cc:reporter-InternalError(IPv6_Hdr_Chain bad 
 header %d, type);
 src/IP.h: reporter-InternalError(bad IP version in 
 IP_Hdr ctor);
 src/RSH.cc:   reporter-InternalError(multiple rsh client names);
 src/RSH.cc:   reporter-InternalError(multiple rsh initial client 
 names);
 src/POP3.cc:  reporter-InternalError(command not known);
 src/Rlogin.cc:reporter-InternalError(multiple rlogin client 
 names);
 src/ICMP.cc:  reporter-InternalError(unexpected IP proto in 
 ICMP analyzer: %d,
 src/ICMP.cc:  reporter-InternalError(unexpected next protocol in 
 ICMP::DeliverPacket());
 src/SMB.cc:   reporter-InternalError(command mismatch for 
 ParseTransaction);
 src/HTTP.cc:  reporter-InternalError(unrecognized HTTP message 
 event);
 src/HTTP.cc:  reporter-InternalError(HTTP ParseRequest failed);
 src/DPM.cc:   reporter-InternalError(unknown protocol);
 src/RPC.cc:   reporter-InternalError(RPC underflow);
 src/RPC.cc:   reporter-InternalError(RPC resync: skipping 
 over data failed);
 src/RPC.cc:   
 reporter-InternalError(inconsistent RPC record marker extraction);

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://bro-tracker.atlassian.net/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev

[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

2013-07-28 Thread Vlad Grigorescu (JIRA) 
Vlad Grigorescu created BIT-1045:


 Summary: Review usage of InternalError when parsing network traffic
 Key: BIT-1045
 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
 Project: Bro Issue Tracker
  Issue Type: Task
  Components: Bro
Affects Versions: 2.1, git/master
Reporter: Vlad Grigorescu


Creating issue for tracking purposes.

Reporter-InternalError denotes a fatal error, and will cause Bro to stop. 
Calling this function when parsing network traffic creates the possibility for 
an attacker using a packet of death, which could stop Bro.

I suspect that in most cases, a weird should be generated instead, and Bro 
should just move on to the next packet. A quick grep shows some likely 
candidates for incorrect use of InternalError:

src/Sessions.cc:reporter-InternalError(Bad IP protocol 
version in DoNextInnerPacket);
src/Sessions.cc:reporter-InternalError(fragment block not in 
dictionary);
src/Sessions.cc:reporter-InternalError(fragment block 
missing);
src/Sessions.cc:reporter-InternalError(unknown 
transport protocol);
src/Frag.cc:reporter-InternalError(bad IP version in fragment 
reassembly);
src/IP.cc:  reporter-InternalError(IPv6_HdrChain::Init with 
truncated IP header);
src/IP.cc:  reporter-InternalError(IPv6_Hdr_Chain bad 
header %d, type);
src/IP.h:   reporter-InternalError(bad IP version in 
IP_Hdr ctor);
src/RSH.cc: reporter-InternalError(multiple rsh client names);
src/RSH.cc: reporter-InternalError(multiple rsh initial client 
names);
src/POP3.cc:reporter-InternalError(command not known);
src/Rlogin.cc:  reporter-InternalError(multiple rlogin client names);
src/ICMP.cc:reporter-InternalError(unexpected IP proto in 
ICMP analyzer: %d,
src/ICMP.cc:reporter-InternalError(unexpected next protocol in 
ICMP::DeliverPacket());
src/SMB.cc: reporter-InternalError(command mismatch for 
ParseTransaction);
src/HTTP.cc:reporter-InternalError(unrecognized HTTP message 
event);
src/HTTP.cc:reporter-InternalError(HTTP ParseRequest failed);
src/DPM.cc: reporter-InternalError(unknown protocol);
src/RPC.cc: reporter-InternalError(RPC underflow);
src/RPC.cc: reporter-InternalError(RPC resync: skipping 
over data failed);
src/RPC.cc: 
reporter-InternalError(inconsistent RPC record marker extraction);

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://bro-tracker.atlassian.net/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev