Skip to content

Commit 018b463

Browse files
committed
simplify tcp flag checks, fix stream_follower
On various places was used simple comparison for checking state of flags. tcp.flags() == (TCP::SYN | TCP::ACK) This is not what you want usually, because this check is false in case that another flag is set also. Correct check for syn-ack packet should be: (tcp.flags() & (TCP::SYN | TCP::ACK)) == (TCP::SYN | TCP::ACK) To simplify this kind of check, add new has_flags method: bool TCP::has_flags(small_uint<12> check_flags) const
1 parent 74e3d90 commit 018b463

File tree

8 files changed

+31
-11
lines changed

8 files changed

+31
-11
lines changed

examples/portscan.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ bool Scanner::callback(PDU& pdu) {
116116
cout << "Port: " << setw(5) << tcp.sport() << " closed\n";
117117
}
118118
// Is SYN flag on? Then port is open!
119-
else if(tcp.flags() == (TCP::SYN | TCP::ACK)) {
119+
else if(tcp.has_flags(TCP::SYN | TCP::ACK)) {
120120
cout << "Port: " << setw(5) << tcp.sport() << " open\n";
121121
}
122122
}

examples/tcp_connection_close.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class tcp_connection_closer {
6868
const IP& ip = pdu.rfind_pdu<IP>();
6969
const TCP& tcp = pdu.rfind_pdu<TCP>();
7070
// We'll only close a connection when seeing a SYN|ACK
71-
if (tcp.flags() == (TCP::SYN | TCP::ACK)) {
71+
if (tcp.has_flags(TCP::SYN | TCP::ACK)) {
7272
// Create an ethernet header flipping the addresses
7373
EthernetII packet(eth.src_addr(), eth.dst_addr());
7474
// Do the same for IP

include/tins/tcp.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,29 @@ class TINS_API TCP : public PDU {
288288
* \code
289289
* TCP tcp = ...;
290290
* if(tcp.flags() == (TCP::SYN | TCP::ACK)) {
291-
* // It's a SYN+ACK!
291+
* // It's a SYN+ACK, but not SYN+ACK+ECN!
292292
* }
293293
* \endcode
294294
*
295295
* \return The value of the flags field.
296296
*/
297297
small_uint<12> flags() const;
298+
299+
/**
300+
* \brief Check if the given flags are set.
301+
*
302+
* \code
303+
* TCP tcp = ...;
304+
* if(tcp.has_flags(TCP::SYN | TCP::ACK)) {
305+
* // It's a SYN+ACK, but it also possible that other flags are set!
306+
* // it is equivalent to: (tpc.flags() & (TCP::SYN | TCP::ACK)) == (TCP::SYN | TCP::ACK)
307+
* }
308+
* \endcode
309+
*
310+
* \param check_flags
311+
* \return true if all check_flags are set
312+
*/
313+
bool has_flags(small_uint<12> check_flags) const;
298314

299315
/* Setters */
300316

src/tcp.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,10 @@ small_uint<12> TCP::flags() const {
257257
return (header_.res1 << 8) | header_.flags_8;
258258
}
259259

260+
bool TCP::has_flags(small_uint<12> check_flags) const {
261+
return (flags() & check_flags) == check_flags;
262+
}
263+
260264
void TCP::set_flag(Flags tcp_flag, small_uint<1> value) {
261265
switch (tcp_flag) {
262266
case FIN:

src/tcp_ip/flow.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,19 @@ void Flow::advance_sequence(uint32_t seq) {
127127
}
128128

129129
void Flow::update_state(const TCP& tcp) {
130-
if ((tcp.flags() & TCP::FIN) != 0) {
130+
if (tcp.has_flags(TCP::FIN)) {
131131
state_ = FIN_SENT;
132132
}
133-
else if ((tcp.flags() & TCP::RST) != 0) {
133+
else if (tcp.has_flags(TCP::RST)) {
134134
state_ = RST_SENT;
135135
}
136-
else if (state_ == SYN_SENT && (tcp.flags() & TCP::ACK) != 0) {
136+
else if (state_ == SYN_SENT && tcp.has_flags(TCP::ACK)) {
137137
#ifdef TINS_HAVE_ACK_TRACKER
138138
ack_tracker_ = AckTracker(tcp.ack_seq());
139139
#endif // TINS_HAVE_ACK_TRACKER
140140
state_ = ESTABLISHED;
141141
}
142-
else if (state_ == UNKNOWN && (tcp.flags() & TCP::SYN) != 0) {
142+
else if (state_ == UNKNOWN && tcp.has_flags(TCP::SYN)) {
143143
// This is the server's state, sending it's first SYN|ACK
144144
#ifdef TINS_HAVE_ACK_TRACKER
145145
ack_tracker_ = AckTracker(tcp.ack_seq());

src/tcp_ip/stream.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Stream::Stream(PDU& packet, const timestamp_type& ts)
6161
}
6262
const TCP& tcp = packet.rfind_pdu<TCP>();
6363
// If this is not the first packet of a stream (SYN), then it's a partial stream
64-
is_partial_stream_ = tcp.flags() != TCP::SYN;
64+
is_partial_stream_ = !tcp.has_flags(TCP::SYN);
6565
}
6666

6767
void Stream::process_packet(PDU& packet, const timestamp_type& ts) {

src/tcp_ip/stream_follower.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void StreamFollower::process_packet(PDU& packet, const timestamp_type& ts) {
8484
if (iter == streams_.end()) {
8585
// Start tracking if they're either SYNs or they contain data (attach
8686
// to an already running flow).
87-
if (tcp->flags() == TCP::SYN || (attach_to_flows_ && tcp->find_pdu<RawPDU>() != 0)) {
87+
if (tcp->has_flags(TCP::SYN) || (attach_to_flows_ && tcp->find_pdu<RawPDU>() != 0)) {
8888
iter = streams_.insert(make_pair(identifier, Stream(packet, ts))).first;
8989
iter->second.setup_flows_callbacks();
9090
if (on_new_connection_) {
@@ -93,7 +93,7 @@ void StreamFollower::process_packet(PDU& packet, const timestamp_type& ts) {
9393
else {
9494
throw callback_not_set();
9595
}
96-
if (tcp->flags() != TCP::SYN) {
96+
if (!tcp->has_flags(TCP::SYN)) {
9797
// assume the connection is established
9898
iter->second.client_flow().state(Flow::ESTABLISHED);
9999
iter->second.server_flow().state(Flow::ESTABLISHED);

src/tcp_stream.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ bool TCPStream::generic_process(uint32_t& my_seq,
230230

231231
bool TCPStream::update(IP* ip, TCP* tcp) {
232232
if (!syn_ack_sent_) {
233-
if (tcp->flags() == (TCP::SYN | TCP::ACK)) {
233+
if (tcp->has_flags(TCP::SYN | TCP::ACK)) {
234234
server_seq_ = tcp->seq() + 1;
235235
client_seq_ = tcp->ack_seq();
236236
syn_ack_sent_ = true;

0 commit comments

Comments
 (0)