Re: [tcpdump-workers] [PATCH]: [DCCP]: support for variable-length
checked in.
tx for your submission.
/hannes
Gerrit Renker wrote:
> This introduces support for variable-length checksum in
> DCCP, as it is specified in section 9 of RFC 4340.
>
> Previously tcpdump was only able to validate full-coverage
> checksums, this patch verifies checksums in accordance with
> the CsCov header field (sec. 5.3).
>
> The patch has been tested and verified on DCCPv4 and DCCPv6
> connections, checksums were manually validated, and traces
> can be supplied.
>
>
>
> In addition, the patch also
> * removes reduplicated code in dccp6_cksum (which
> just repeated the code of in_cksum)
> * fixes a bug in dccp.h -- the fields of CCVAL and
> CSCOV were swapped (cf. section 5.1 of RFC 4340)
> * fixes a typo in the display of incorrect checksums
> (these were printed as `cksum xDEAD (incorrect (-> xBEEF)',
>it now will print `cksum xDEAD (incorrect -> xBEEF)'
>
> I would like to see this merged as a Linux kernel patch already
> exists for DCCP partial checksum coverage support.
>
> Regards
> Gerrit Renker
>
>
> dccp.h |4 ++--
> print-dccp.c | 34 --
> 2 files changed, 18 insertions(+), 20 deletions(-)
>
>
>
>
> diff --git a/dccp.h b/dccp.h
> index 1afa8c0..8585060 100644
> --- a/dccp.h
> +++ b/dccp.h
> @@ -36,8 +36,8 @@ struct dccp_hdr {
> } dccph_xtrs;
> };
>
> -#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov) & 0x0F)
> -#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov >> 4) & 0x0F)
> +#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov >> 4) & 0xF)
> +#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov) & 0xF)
>
> #define DCCPH_X(dh) ((dh)->dccph_xtrs.dccph_xtr & 1)
> #define DCCPH_TYPE(dh) (((dh)->dccph_xtrs.dccph_xtr >> 1) & 0xF)
> diff --git a/print-dccp.c b/print-dccp.c
> index e6bfca6..d19c500 100644
> --- a/print-dccp.c
> +++ b/print-dccp.c
> @@ -60,9 +60,20 @@ static const char *dccp_feature_nums[] =
> "check data checksum",
> };
>
> +static inline int dccp_csum_coverage(const struct dccp_hdr* dh, u_int len)
> +{
> + int cov;
> +
> + if (DCCPH_CSCOV(dh) == 0)
> + return len;
> + cov = (dh->dccph_doff + DCCPH_CSCOV(dh) - 1) * sizeof(u_int32_t);
> + return (cov > len)? len : cov;
> +}
> +
> static int dccp_cksum(const struct ip *ip,
> const struct dccp_hdr *dh, u_int len)
> {
> + int cov = dccp_csum_coverage(dh, len);
> union phu {
> struct phdr {
> u_int32_t src;
> @@ -86,15 +97,15 @@ static int dccp_cksum(const struct ip *i
> phu.ph.dst = ip_finddst(ip);
>
> sp = &phu.pa[0];
> - return in_cksum((u_short *)dh, len,
> sp[0]+sp[1]+sp[2]+sp[3]+sp[4]+sp[5]);
> + return in_cksum((u_short *)dh, cov,
> sp[0]+sp[1]+sp[2]+sp[3]+sp[4]+sp[5]);
> }
>
> #ifdef INET6
> static int dccp6_cksum(const struct ip6_hdr *ip6, const struct dccp_hdr *dh,
> u_int len)
> {
> size_t i;
> - const u_int16_t *sp;
> - u_int32_t sum;
> + u_int32_t sum = 0;
> + int cov = dccp_csum_coverage(dh, len);
> union {
> struct {
> struct in6_addr ph_src;
> @@ -113,23 +124,10 @@ static int dccp6_cksum(const struct ip6_
> phu.ph.ph_len = htonl(len);
> phu.ph.ph_nxt = IPPROTO_DCCP;
>
> - sum = 0;
> for (i = 0; i < sizeof(phu.pa) / sizeof(phu.pa[0]); i++)
> sum += phu.pa[i];
>
> - sp = (const u_int16_t *)dh;
> -
> - for (i = 0; i < (len & ~1); i += 2)
> - sum += *sp++;
> -
> - if (len & 1)
> - sum += htons((*(const u_int8_t *)sp) << 8);
> -
> - while (sum > 0x)
> - sum = (sum & 0x) + (sum >> 16);
> - sum = ~sum & 0x;
> -
> - return (sum);
> + return in_cksum((u_short *)dh, cov, sum);
> }
> #endif
>
> @@ -288,7 +286,7 @@ #ifdef INET6
> dccp_sum = EXTRACT_16BITS(&dh->dccph_checksum);
> printf("cksum 0x%04x", dccp_sum);
> if (sum != 0) {
> - (void)printf(" (incorrect (-> 0x%04x),
> ",in_cksum_shouldbe(dccp_sum, sum));
> + (void)printf(" (incorrect -> 0x%04x),
> ",in_cksum_shouldbe(dccp_sum, sum));
> } else
> (void)printf(" (correct), ");
> }
>
>
>
>
> -
> This is the tcpdump-workers list.
> Visit https://cod.sandelman.ca/ to unsubscribe.
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH]: [DCCP]: support for variable-length checksums
On 10/31/06, Gerrit Renker <[EMAIL PROTECTED]> wrote: This introduces support for variable-length checksum in DCCP, as it is specified in section 9 of RFC 4340. Previously tcpdump was only able to validate full-coverage checksums, this patch verifies checksums in accordance with the CsCov header field (sec. 5.3). The patch has been tested and verified on DCCPv4 and DCCPv6 connections, checksums were manually validated, and traces can be supplied. In addition, the patch also * removes reduplicated code in dccp6_cksum (which just repeated the code of in_cksum) * fixes a bug in dccp.h -- the fields of CCVAL and CSCOV were swapped (cf. section 5.1 of RFC 4340) * fixes a typo in the display of incorrect checksums (these were printed as `cksum xDEAD (incorrect (-> xBEEF)', it now will print `cksum xDEAD (incorrect -> xBEEF)' I would like to see this merged as a Linux kernel patch already exists for DCCP partial checksum coverage support. I would agree with this going ahead and in particular I remember the CCVAL bug and then forgetting to fix it... -- Ian McDonald Web: http://wand.net.nz/~iam4 Blog: http://imcdnzl.blogspot.com WAND Network Research Group Department of Computer Science University of Waikato New Zealand - This is the tcpdump-workers list. Visit https://cod.sandelman.ca/ to unsubscribe.
[tcpdump-workers] [PATCH]: [DCCP]: support for variable-length checksums
This introduces support for variable-length checksum in
DCCP, as it is specified in section 9 of RFC 4340.
Previously tcpdump was only able to validate full-coverage
checksums, this patch verifies checksums in accordance with
the CsCov header field (sec. 5.3).
The patch has been tested and verified on DCCPv4 and DCCPv6
connections, checksums were manually validated, and traces
can be supplied.
In addition, the patch also
* removes reduplicated code in dccp6_cksum (which
just repeated the code of in_cksum)
* fixes a bug in dccp.h -- the fields of CCVAL and
CSCOV were swapped (cf. section 5.1 of RFC 4340)
* fixes a typo in the display of incorrect checksums
(these were printed as `cksum xDEAD (incorrect (-> xBEEF)',
it now will print `cksum xDEAD (incorrect -> xBEEF)'
I would like to see this merged as a Linux kernel patch already
exists for DCCP partial checksum coverage support.
Regards
Gerrit Renker
dccp.h |4 ++--
print-dccp.c | 34 --
2 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/dccp.h b/dccp.h
index 1afa8c0..8585060 100644
--- a/dccp.h
+++ b/dccp.h
@@ -36,8 +36,8 @@ struct dccp_hdr {
} dccph_xtrs;
};
-#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov) & 0x0F)
-#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov >> 4) & 0x0F)
+#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov >> 4) & 0xF)
+#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov) & 0xF)
#define DCCPH_X(dh) ((dh)->dccph_xtrs.dccph_xtr & 1)
#define DCCPH_TYPE(dh) (((dh)->dccph_xtrs.dccph_xtr >> 1) & 0xF)
diff --git a/print-dccp.c b/print-dccp.c
index e6bfca6..d19c500 100644
--- a/print-dccp.c
+++ b/print-dccp.c
@@ -60,9 +60,20 @@ static const char *dccp_feature_nums[] =
"check data checksum",
};
+static inline int dccp_csum_coverage(const struct dccp_hdr* dh, u_int len)
+{
+ int cov;
+
+ if (DCCPH_CSCOV(dh) == 0)
+ return len;
+ cov = (dh->dccph_doff + DCCPH_CSCOV(dh) - 1) * sizeof(u_int32_t);
+ return (cov > len)? len : cov;
+}
+
static int dccp_cksum(const struct ip *ip,
const struct dccp_hdr *dh, u_int len)
{
+ int cov = dccp_csum_coverage(dh, len);
union phu {
struct phdr {
u_int32_t src;
@@ -86,15 +97,15 @@ static int dccp_cksum(const struct ip *i
phu.ph.dst = ip_finddst(ip);
sp = &phu.pa[0];
- return in_cksum((u_short *)dh, len, sp[0]+sp[1]+sp[2]+sp[3]+sp[4]+sp[5]);
+ return in_cksum((u_short *)dh, cov, sp[0]+sp[1]+sp[2]+sp[3]+sp[4]+sp[5]);
}
#ifdef INET6
static int dccp6_cksum(const struct ip6_hdr *ip6, const struct dccp_hdr *dh, u_int len)
{
size_t i;
- const u_int16_t *sp;
- u_int32_t sum;
+ u_int32_t sum = 0;
+ int cov = dccp_csum_coverage(dh, len);
union {
struct {
struct in6_addr ph_src;
@@ -113,23 +124,10 @@ static int dccp6_cksum(const struct ip6_
phu.ph.ph_len = htonl(len);
phu.ph.ph_nxt = IPPROTO_DCCP;
- sum = 0;
for (i = 0; i < sizeof(phu.pa) / sizeof(phu.pa[0]); i++)
sum += phu.pa[i];
- sp = (const u_int16_t *)dh;
-
- for (i = 0; i < (len & ~1); i += 2)
- sum += *sp++;
-
- if (len & 1)
- sum += htons((*(const u_int8_t *)sp) << 8);
-
- while (sum > 0x)
- sum = (sum & 0x) + (sum >> 16);
- sum = ~sum & 0x;
-
- return (sum);
+ return in_cksum((u_short *)dh, cov, sum);
}
#endif
@@ -288,7 +286,7 @@ #ifdef INET6
dccp_sum = EXTRACT_16BITS(&dh->dccph_checksum);
printf("cksum 0x%04x", dccp_sum);
if (sum != 0) {
-(void)printf(" (incorrect (-> 0x%04x), ",in_cksum_shouldbe(dccp_sum, sum));
+(void)printf(" (incorrect -> 0x%04x), ",in_cksum_shouldbe(dccp_sum, sum));
} else
(void)printf(" (correct), ");
}
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH] DCCP support - revised
(Noise inserted here to trick the duplicate-message dissector into letting this message pass; the original was sent from the wrong address.) Ian McDonald wrote: Thanks for putting this in - please credit Nishida-san as well. Done. - This is the tcpdump-workers list. Visit https://lists.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH] DCCP support - revised
On 20/09/05, Guy Harris <[EMAIL PROTECTED]> wrote: > Ian McDonald wrote: > > > Please find attached a slightly revised patch to tidyup a bug with > > seqno/ackno processing. > > Checked into the main and x.9 branches. I got rid of the dh_end > variable, and added RCS IDs. > > I gave you and Arnaldo credit in the CREDITS file; should I put > Yoshifumi Nishida in there as well? > - Thanks for putting this in - please credit Nishida-san as well. - This is the tcpdump-workers list. Visit https://lists.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH] DCCP support - revised
Ian McDonald wrote: Please find attached a slightly revised patch to tidyup a bug with seqno/ackno processing. Checked into the main and x.9 branches. I got rid of the dh_end variable, and added RCS IDs. I gave you and Arnaldo credit in the CREDITS file; should I put Yoshifumi Nishida in there as well? - This is the tcpdump-workers list. Visit https://lists.sandelman.ca/ to unsubscribe.
[tcpdump-workers] [PATCH] DCCP support - revised
Guy,
Please find attached a slightly revised patch to tidyup a bug with
seqno/ackno processing.
Regards,
Ian
On 06/09/05, Ian McDonald <[EMAIL PROTECTED]> wrote:
> Guy,
>
> Comments inline.
>
> > "dh_end" still isn't being used, as per my original comment:
> >
> > > In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2()
> > > calls, and the "dh_end" variable isn't used, so no check appears to be
> > > done to prevent the print routine from running past the end of the
> > > captured data in the packet.
> >
> > It might not need to be used - if there are a sufficient set of
> > TCHECK/TTEST-family calls, checking against the captured data length is
> > being done, and if all checks for the end of the packet (rather than the
> > end of the captured data length) are done by checking against "len", it
> > presumably wouldn't need to be - but if it's not used, it shouldn't exist.
>
> I am checking explicitly at each set with the TCHECK/TTEST macros. I
> have tested this with deliberately short packets and my old code
> segfaulted and the new one doesn't. I haven't used dh_end because I
> check each one before use. I believe this to be robust now - I spent
> quite a few hours testing this after your orginal comments :-)
>
> >
> > Also:
> >
> > 1) FILES needs to be modified to include dccp.h and print-dccp.c;
>
> Done
> >
> > 2) INSTALL should be, too;
>
> Done
> >
> > 3) I've attached a patch to clean up some compiler warnings on OS X
> > 10.3.9 (which probably show up on at least some other platforms).
> >
>
> I've put these in place and also more picked up by using -Wall on gcc
> (which I should have done in the first place).
>
> I've also put in one warning picked up by Matthew Luckie and at his
> suggestion also removed register keyword against variables and I'll
> let the compiler work it out.
>
> Please find attached new patch.
>
> Regards,
>
> Ian
>
>
>
diff -urN tcpdump-2005.08.22/dccp.h dccpdump/dccp.h
--- tcpdump-2005.08.22/dccp.h 1970-01-01 12:00:00.0 +1200
+++ dccpdump/dccp.h 2005-09-14 14:18:54.0 +1200
@@ -0,0 +1,135 @@
+#ifndef __DCCP_HDR__
+#define __DCCP_HDR__
+/*
+ * Copyright (C) Arnaldo Carvalho de Melo 2004
+ * Copyright (C) Ian McDonald 2005 <[EMAIL PROTECTED]>
+ * Copyright (C) Yoshifumi Nishida 2005
+ *
+ * This software may be distributed either under the terms of the
+ * BSD-style license that accompanies tcpdump or the GNU GPL version 2
+ */
+
+/**
+ * struct dccp_hdr - generic part of DCCP packet header
+ *
+ * @dccph_sport - Relevant port on the endpoint that sent this packet
+ * @dccph_dport - Relevant port on the other endpoint
+ * @dccph_doff - Data Offset from the start of the DCCP header, in 32-bit words
+ * @dccph_ccval - Used by the HC-Sender CCID
+ * @dccph_cscov - Parts of the packet that are covered by the Checksum field
+ * @dccph_checksum - Internet checksum, depends on dccph_cscov
+ * @dccph_x - 0 = 24 bit sequence number, 1 = 48
+ * @dccph_type - packet type, see DCCP_PKT_ prefixed macros
+ * @dccph_seq - sequence number high or low order 24 bits, depends on dccph_x
+ */
+struct dccp_hdr {
+ u_int16_t dccph_sport,
+ dccph_dport;
+ u_int8_tdccph_doff;
+ u_int8_tdccph_ccval_cscov;
+ u_int16_t dccph_checksum;
+ union {
+ u_int8_tdccph_xtr;
+ u_int32_t dccph_seq;
+ } dccph_xtrs;
+};
+
+#define DCCPH_CCVAL(dh)(((dh)->dccph_ccval_cscov) & 0x0F)
+#define DCCPH_CSCOV(dh)(((dh)->dccph_ccval_cscov >> 4) & 0x0F)
+
+#define DCCPH_X(dh)((dh)->dccph_xtrs.dccph_xtr & 1)
+#define DCCPH_TYPE(dh) (((dh)->dccph_xtrs.dccph_xtr >> 1) & 0xF)
+#define DCCPH_SEQ(dh) (((dh)->dccph_xtrs.dccph_seq) >> 8)
+
+/**
+ * struct dccp_hdr_ext - the low bits of a 48 bit seq packet
+ *
+ * @dccph_seq_low - low 24 bits of a 48 bit seq packet
+ */
+struct dccp_hdr_ext {
+ u_int32_t dccph_seq_low;
+};
+
+/**
+ * struct dccp_hdr_request - Conection initiation request header
+ *
+ * @dccph_req_service - Service to which the client app wants to connect
+ */
+struct dccp_hdr_request {
+ u_int32_t dccph_req_service;
+};
+
+/**
+ * struct dccp_hdr_ack_bits - acknowledgment bits common to most packets
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ */
+struct dccp_hdr_ack_bits {
+ u_int32_t dccph_ra;
+ u_int32_t dccph_ack_nr_low;
+};
+
+#define DCCPH_ACK(dh_ack) ((dh_ack)->dccph_ra >> 8)
+
+/**
+ * struct dccp_hdr_response - Conection initiation response header
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ * @dccph_resp_service - Echoes the Service Code on a received DCCP-Request
+ */
+struct dccp_hdr_response {
+ struct
Re: [tcpdump-workers] [PATCH] DCCP support
Guy,
Comments inline.
> "dh_end" still isn't being used, as per my original comment:
>
> > In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2()
> > calls, and the "dh_end" variable isn't used, so no check appears to be
> > done to prevent the print routine from running past the end of the
> > captured data in the packet.
>
> It might not need to be used - if there are a sufficient set of
> TCHECK/TTEST-family calls, checking against the captured data length is
> being done, and if all checks for the end of the packet (rather than the
> end of the captured data length) are done by checking against "len", it
> presumably wouldn't need to be - but if it's not used, it shouldn't exist.
I am checking explicitly at each set with the TCHECK/TTEST macros. I
have tested this with deliberately short packets and my old code
segfaulted and the new one doesn't. I haven't used dh_end because I
check each one before use. I believe this to be robust now - I spent
quite a few hours testing this after your orginal comments :-)
>
> Also:
>
> 1) FILES needs to be modified to include dccp.h and print-dccp.c;
Done
>
> 2) INSTALL should be, too;
Done
>
> 3) I've attached a patch to clean up some compiler warnings on OS X
> 10.3.9 (which probably show up on at least some other platforms).
>
I've put these in place and also more picked up by using -Wall on gcc
(which I should have done in the first place).
I've also put in one warning picked up by Matthew Luckie and at his
suggestion also removed register keyword against variables and I'll
let the compiler work it out.
Please find attached new patch.
Regards,
Ian
diff -urN tcpdump-2005.08.22/dccp.h dccpdump/dccp.h
--- tcpdump-2005.08.22/dccp.h 1970-01-01 12:00:00.0 +1200
+++ dccpdump/dccp.h 2005-09-06 10:24:24.276683776 +1200
@@ -0,0 +1,136 @@
+#ifndef __DCCP_HDR__
+#define __DCCP_HDR__
+/*
+ * Copyright (C) Arnaldo Carvalho de Melo 2004
+ * Copyright (C) Ian McDonald 2005 <[EMAIL PROTECTED]>
+ * Copyright (C) Yoshifumi Nishida 2005
+ *
+ * This software may be distributed either under the terms of the
+ * BSD-style license that accompanies tcpdump or the GNU GPL version 2
+ */
+
+/**
+ * struct dccp_hdr - generic part of DCCP packet header
+ *
+ * @dccph_sport - Relevant port on the endpoint that sent this packet
+ * @dccph_dport - Relevant port on the other endpoint
+ * @dccph_doff - Data Offset from the start of the DCCP header, in 32-bit words
+ * @dccph_ccval - Used by the HC-Sender CCID
+ * @dccph_cscov - Parts of the packet that are covered by the Checksum field
+ * @dccph_checksum - Internet checksum, depends on dccph_cscov
+ * @dccph_x - 0 = 24 bit sequence number, 1 = 48
+ * @dccph_type - packet type, see DCCP_PKT_ prefixed macros
+ * @dccph_seq - sequence number high or low order 24 bits, depends on dccph_x
+ */
+struct dccp_hdr {
+ u_int16_t dccph_sport,
+ dccph_dport;
+ u_int8_t dccph_doff;
+ u_int8_t dccph_ccval_cscov;
+ u_int16_t dccph_checksum;
+ union {
+ u_int8_t dccph_xtr;
+ u_int32_t dccph_seq;
+ } dccph_xtrs;
+};
+
+#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov) & 0x0F)
+#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov >> 4) & 0x0F)
+
+#define DCCPH_X(dh) ((dh)->dccph_xtrs.dccph_xtr & 1)
+#define DCCPH_TYPE(dh) (((dh)->dccph_xtrs.dccph_xtr >> 1) & 0xF)
+
+#define DCCPH_SEQ(dh) (((dh)->dccph_xtrs.dccph_seq >> 8) & 0x0FFF)
+
+/**
+ * struct dccp_hdr_ext - the low bits of a 48 bit seq packet
+ *
+ * @dccph_seq_low - low 24 bits of a 48 bit seq packet
+ */
+struct dccp_hdr_ext {
+ u_int32_t dccph_seq_low;
+};
+
+/**
+ * struct dccp_hdr_request - Conection initiation request header
+ *
+ * @dccph_req_service - Service to which the client app wants to connect
+ */
+struct dccp_hdr_request {
+ u_int32_t dccph_req_service;
+};
+
+/**
+ * struct dccp_hdr_ack_bits - acknowledgment bits common to most packets
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ */
+struct dccp_hdr_ack_bits {
+ u_int32_t dccph_ra;
+ u_int32_t dccph_ack_nr_low;
+};
+
+#define DCCPH_ACK(dh_ack) (((dh_ack)->dccph_ra >> 8) & 0x0FFF)
+
+/**
+ * struct dccp_hdr_response - Conection initiation response header
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ * @dccph_resp_service - Echoes the Service Code on a received DCCP-Request
+ */
+struct dccp_hdr_response {
+ struct dccp_hdr_ack_bits dccph_resp_ack;
+ u_int32_t dccph_resp_service;
+};
+
+static inline struct dccp_hdr_data *dccp_hdr_data(struct dccp_hdr *hdrg)
+{
+ const int ext = DCCPH_X(hdrg) ? sizeof(struct dccp_hdr_ext) : 0;
+
+ return (struct dccp_hdr_data *)(((u_char *)hdrg) + sizeof(hdrg) + ext);
+}
+
+/**
+ * struct dccp_hdr_reset - Unconditionally shut down a connection
+ *
+ * @dccph_reset_service - Echoes the Service Code on a received DCCP-R
Re: [tcpdump-workers] [PATCH] DCCP support
Ian McDonald wrote:
Please find attached a reworked patch which I believe addresses all of
Guy's original concerns and also adds options processing and other
enhancements.
"dh_end" still isn't being used, as per my original comment:
In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2()
calls, and the "dh_end" variable isn't used, so no check appears to be
done to prevent the print routine from running past the end of the
captured data in the packet.
It might not need to be used - if there are a sufficient set of
TCHECK/TTEST-family calls, checking against the captured data length is
being done, and if all checks for the end of the packet (rather than the
end of the captured data length) are done by checking against "len", it
presumably wouldn't need to be - but if it's not used, it shouldn't exist.
Also:
1) FILES needs to be modified to include dccp.h and print-dccp.c;
2) INSTALL should be, too;
3) I've attached a patch to clean up some compiler warnings on OS X
10.3.9 (which probably show up on at least some other platforms).
*** print-dccp.cMon Sep 5 11:32:53 2005
--- print-dccp.c.NEWMon Sep 5 11:34:04 2005
***
*** 187,193
return sizeof(struct dccp_hdr_reset);
}
! static int dccp_print_option(u_char *option);
/**
* dccp_print - show dccp packet
--- 187,193
return sizeof(struct dccp_hdr_reset);
}
! static int dccp_print_option(const u_char *option);
/**
* dccp_print - show dccp packet
***
*** 205,213
const u_char *cp;
const void *dh_end;
u_short sport, dport;
! int hlen;
u_int extlen = 0;
- u_int64_t seq;
dh = (const struct dccp_hdr *)bp;
dh_end = (const u_char *)dh + len;
--- 205,212
const u_char *cp;
const void *dh_end;
u_short sport, dport;
! u_int hlen;
u_int extlen = 0;
dh = (const struct dccp_hdr *)bp;
dh_end = (const u_char *)dh + len;
***
*** 381,388
TCHECK(*cp);
optlen = dccp_print_option(cp);
if (!optlen) goto trunc2;
hlen -= optlen;
- if (hlen <= 0) break;
cp += optlen;
printf(", ");
}
--- 380,387
TCHECK(*cp);
optlen = dccp_print_option(cp);
if (!optlen) goto trunc2;
+ if (hlen <= optlen) break;
hlen -= optlen;
cp += optlen;
printf(", ");
}
***
*** 395,401
return;
}
! static int dccp_print_option(u_char *option)
{
u_int8_t optlen, i;
u_int32_t *ts;
--- 394,400
return;
}
! static int dccp_print_option(const u_char *option)
{
u_int8_t optlen, i;
u_int32_t *ts;
***
*** 492,498
}
break;
case 44:
! printf("data_checksum %x");
for (i = 0; i < optlen -2; i ++) printf("%02x", *(option +2 +
i));
break;
default :
--- 491,497
}
break;
case 44:
! printf("data_checksum ");
for (i = 0; i < optlen -2; i ++) printf("%02x", *(option +2 +
i));
break;
default :
-
This is the tcpdump-workers list.
Visit https://lists.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH] DCCP support
Hi there,
Please find attached a reworked patch which I believe addresses all of
Guy's original concerns and also adds options processing and other
enhancements.
It has been tested on 32 and 64 bit, little and big endian but hasn't
been tested on Darwin - I have however done what was asked.
Can you please accept this into the tcpdump program or advise what
else needs to be done?
More information on DCCP can be obtained at http://wlug.org.nz/DCCP
Regards,
Ian
On 03/08/05, Guy Harris <[EMAIL PROTECTED]> wrote:
> Ian McDonald wrote:
>
> > +#define __LITTLE_ENDIAN_BITFIELD
> > +
> > +/**
> > + * struct dccp_hdr - generic part of DCCP packet header
> > + *
> > + * @dccph_sport - Relevant port on the endpoint that sent this packet
> > + * @dccph_dport - Relevant port on the other endpoint
> > + * @dccph_doff - Data Offset from the start of the DCCP header, in 32-bit
> > words
> > + * @dccph_ccval - Used by the HC-Sender CCID
> > + * @dccph_cscov - Parts of the packet that are covered by the Checksum
> > field
> > + * @dccph_checksum - Internet checksum, depends on dccph_cscov
> > + * @dccph_x - 0 = 24 bit sequence number, 1 = 48
> > + * @dccph_type - packet type, see DCCP_PKT_ prefixed macros
> > + * @dccph_seq - sequence number high or low order 24 bits, depends on
> > dccph_x
> > + */
> > +struct dccp_hdr {
> > + u_int16_t dccph_sport,
> > + dccph_dport;
> > + u_int8_tdccph_doff;
> > +#if defined(__LITTLE_ENDIAN_BITFIELD)
> > + u_int8_tdccph_cscov:4,
> > + dccph_ccval:4;
> > +#elif defined(__BIG_ENDIAN_BITFIELD)
> > + u_int8_tdccph_ccval:4,
> > + dccph_cscov:4;
> > +#else
> > +#error "Adjust your defines"
> > +#endif
>
> The fact that __LITTLE_ENDIAN_BITFIELD is predefined means that the code
> assumes that bitfields are stored in an order such that in
>
> u_int8_tdccph_cscov:4,
> dccph_ccval:4;
>
> the low-order 4 bits of the byte are dccph_cscov and the high-order 4
> bits are dccph_ccval.
>
> The C language doesn't specify the order of bitfields in the bits of a
> structure, so that won't work.
>
> If I remove that #define, I get the errors
>
> dccp.h:35:2: #error "Adjust your defines"
> dccp.h:49:2: #error "Adjust your defines"
> dccp.h:114: error: structure has no member named `dccph_x'
> print-dccp.c:57:2: #error "Adjust your defines"
>
> Unfortunately, the suggestion made by the errors doesn't help:
>
> Guy-Harris-Computer.local$ ls -l /usr/include/asm/byteorder.h
> ls: /usr/include/asm/byteorder.h: No such file or directory
> Guy-Harris-Computer.local$ ls -ld /usr/include/asm
> ls: /usr/include/asm: No such file or directory
>
> because:
>
> Guy-Harris-Computer.local$ uname -sr
> Darwin 7.9.0
>
> this ain't Linux.
>
> Try, instead:
>
> u_int8_tdccph_ccval_cscov;
>
> ...
>
> #define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov) & 0x0F)
> #define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov >> 4) & 0x0F)
>
> And, for the Res, Type, X, and Sequence number fields, I'd declare the
> first 4 bytes as a u_int32_t, extract it with EXTRACT_32BITS(), and
> define macros that take a 32-bit value and extract the Res, Type, X, and
> Sequence Number fields from it. I'd do something similar with the Ack
> sequence numbers.
>
> In addition, zero-length arrays such as
>
> u_int32_t dccph_options[0];
>
> are not supported by all C compilers people use to compile tcpdump;
> either make it an array of 1 element (and subtract sizeof(u_int32_t)
> from anything based on the size of the structure), or leave that field
> out (and, if code is written that looks at options, add the length of
> the header to a pointer to the start of the packet and use that pointer
> as a pointer to the options), and do similar things in other places
> where there are zero-length arrays.
>
> (Also, remove the include of ; the assert macro isn't used.)
>
> In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2()
> calls, and the "dh_end" variable isn't used, so no check appears to be
> done to prevent the print routine from running past the end of the
> captured data in the packet.
diff -urN tcpdump-2005.08.22/dccp.h dccpdump/dccp.h
--- tcpdump-2005.08.22/dccp.h 1970-01-01 12:00:00.0 +1200
+++ dccpdump/dccp.h 2005-09-05 11:49:48.0 +1200
@@ -0,0 +1,136 @@
+#ifndef __DCCP_HDR__
+#define __DCCP_HDR__
+/*
+ * Copyright (C) Arnaldo Carvalho de Melo 2004
+ * Copyright (C) Ian McDonald 2005 <[EMAIL PROTECTED]>
+ * Copyright (C) Yoshifumi Nishida 2005
+ *
+ * This software may be distributed either under the terms of the
+ * BSD-style license that accompanies tcpdump or the GNU GPL version 2
+ */
+
+/**
+ * struct dccp_hdr - generic part of DCCP packet header
+ *
+ * @dccph_sport - Relevant port on the endpoint th
Re: [tcpdump-workers] [PATCH] DCCP support
Guy, Comments in line. On 03/08/05, Guy Harris <[EMAIL PROTECTED]> wrote: > > The fact that __LITTLE_ENDIAN_BITFIELD is predefined means that the code > assumes that bitfields are stored in an order such that in > > u_int8_tdccph_cscov:4, > dccph_ccval:4; > Will investigate further.. > In addition, zero-length arrays such as > > u_int32_t dccph_options[0]; > > are not supported by all C compilers people use to compile tcpdump; Point noted > (Also, remove the include of ; the assert macro isn't used.) > Point noted > In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2() > calls, and the "dh_end" variable isn't used, so no check appears to be > done to prevent the print routine from running past the end of the > captured data in the packet. Will check what these do. I appreciate all the feedback. I have received further protocol support from Nishida-san of Sony Japan so will look to incorporate your comments and his changes before resubmitting. Regards, Ian - This is the tcpdump-workers list. Visit https://lists.sandelman.ca/ to unsubscribe.
Re: [tcpdump-workers] [PATCH] DCCP support
Ian McDonald wrote:
+#define __LITTLE_ENDIAN_BITFIELD
+
+/**
+ * struct dccp_hdr - generic part of DCCP packet header
+ *
+ * @dccph_sport - Relevant port on the endpoint that sent this packet
+ * @dccph_dport - Relevant port on the other endpoint
+ * @dccph_doff - Data Offset from the start of the DCCP header, in 32-bit words
+ * @dccph_ccval - Used by the HC-Sender CCID
+ * @dccph_cscov - Parts of the packet that are covered by the Checksum field
+ * @dccph_checksum - Internet checksum, depends on dccph_cscov
+ * @dccph_x - 0 = 24 bit sequence number, 1 = 48
+ * @dccph_type - packet type, see DCCP_PKT_ prefixed macros
+ * @dccph_seq - sequence number high or low order 24 bits, depends on dccph_x
+ */
+struct dccp_hdr {
+ u_int16_t dccph_sport,
+ dccph_dport;
+ u_int8_tdccph_doff;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ u_int8_tdccph_cscov:4,
+ dccph_ccval:4;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+ u_int8_tdccph_ccval:4,
+ dccph_cscov:4;
+#else
+#error "Adjust your defines"
+#endif
The fact that __LITTLE_ENDIAN_BITFIELD is predefined means that the code
assumes that bitfields are stored in an order such that in
u_int8_tdccph_cscov:4,
dccph_ccval:4;
the low-order 4 bits of the byte are dccph_cscov and the high-order 4
bits are dccph_ccval.
The C language doesn't specify the order of bitfields in the bits of a
structure, so that won't work.
If I remove that #define, I get the errors
dccp.h:35:2: #error "Adjust your defines"
dccp.h:49:2: #error "Adjust your defines"
dccp.h:114: error: structure has no member named `dccph_x'
print-dccp.c:57:2: #error "Adjust your defines"
Unfortunately, the suggestion made by the errors doesn't help:
Guy-Harris-Computer.local$ ls -l /usr/include/asm/byteorder.h
ls: /usr/include/asm/byteorder.h: No such file or directory
Guy-Harris-Computer.local$ ls -ld /usr/include/asm
ls: /usr/include/asm: No such file or directory
because:
Guy-Harris-Computer.local$ uname -sr
Darwin 7.9.0
this ain't Linux.
Try, instead:
u_int8_tdccph_ccval_cscov;
...
#define DCCPH_CCVAL(dh) (((dh)->dccph_ccval_cscov) & 0x0F)
#define DCCPH_CSCOV(dh) (((dh)->dccph_ccval_cscov >> 4) & 0x0F)
And, for the Res, Type, X, and Sequence number fields, I'd declare the
first 4 bytes as a u_int32_t, extract it with EXTRACT_32BITS(), and
define macros that take a 32-bit value and extract the Res, Type, X, and
Sequence Number fields from it. I'd do something similar with the Ack
sequence numbers.
In addition, zero-length arrays such as
u_int32_t dccph_options[0];
are not supported by all C compilers people use to compile tcpdump;
either make it an array of 1 element (and subtract sizeof(u_int32_t)
from anything based on the size of the structure), or leave that field
out (and, if code is written that looks at options, add the length of
the header to a pointer to the start of the packet and use that pointer
as a pointer to the options), and do similar things in other places
where there are zero-length arrays.
(Also, remove the include of ; the assert macro isn't used.)
In addition, there are no TCHECK(), TCHECK2(), TTEST(), or TTEST2()
calls, and the "dh_end" variable isn't used, so no check appears to be
done to prevent the print routine from running past the end of the
captured data in the packet.
-
This is the tcpdump-workers list.
Visit https://lists.sandelman.ca/ to unsubscribe.
[tcpdump-workers] [PATCH] DCCP support
Hi there,
I am submitting a patch to add support that I have worked on and
Arnaldo Carvalho de Melo has worked on. This patch has been written
against the nightly build of July 31, 2005.
I looked at the instructions for submitting patches and it said to use
SourceForge but nobody has submitted there for a while by the looks so
I am submitting to the list. Please advise if I should send elsewhere.
This is the first stage of DCCP packet analysis. I would like to add
support for CCID and option processing but I want to do the "release
early, release often" thing.
More details about DCCP can be obtained from here:
http://www.icir.org/kohler/dcp/
or here:
http://wand.cs.waikato.ac.nz/~iam4/dccp/index.html
Regards,
Ian
diff -uprN tcpdump-2005.07.31/dccp.h newdump/dccp.h
--- tcpdump-2005.07.31/dccp.h 1970-01-01 12:00:00.0 +1200
+++ newdump/dccp.h 2005-08-02 11:49:51.0 +1200
@@ -0,0 +1,186 @@
+#ifndef __DCCP_HDR__
+#define __DCCP_HDR__
+/*
+ * Copyright (C) Arnaldo Carvalho de Melo 2004
+ * Copyright (C) Ian McDonald 2005
+ *
+ * This software may be distributed either under the terms of the
+ * BSD-style license that accompanies tcpdump or the GNU GPL version 2
+ */
+
+#define __LITTLE_ENDIAN_BITFIELD
+
+/**
+ * struct dccp_hdr - generic part of DCCP packet header
+ *
+ * @dccph_sport - Relevant port on the endpoint that sent this packet
+ * @dccph_dport - Relevant port on the other endpoint
+ * @dccph_doff - Data Offset from the start of the DCCP header, in 32-bit words
+ * @dccph_ccval - Used by the HC-Sender CCID
+ * @dccph_cscov - Parts of the packet that are covered by the Checksum field
+ * @dccph_checksum - Internet checksum, depends on dccph_cscov
+ * @dccph_x - 0 = 24 bit sequence number, 1 = 48
+ * @dccph_type - packet type, see DCCP_PKT_ prefixed macros
+ * @dccph_seq - sequence number high or low order 24 bits, depends on dccph_x
+ */
+struct dccp_hdr {
+ u_int16_t dccph_sport,
+ dccph_dport;
+ u_int8_t dccph_doff;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ u_int8_t dccph_cscov:4,
+ dccph_ccval:4;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+ u_int8_t dccph_ccval:4,
+ dccph_cscov:4;
+#else
+#error "Adjust your defines"
+#endif
+ u_int16_t dccph_checksum;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ u_int32_t dccph_x:1,
+ dccph_type:4,
+ dccph_reserved:3,
+ dccph_seq:24;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+ u_int32_t dccph_reserved:3,
+ dccph_type:4,
+ dccph_x:1,
+ dccph_seq:24;
+#else
+#error "Adjust your defines"
+#endif
+ u_int32_t dccph_options[0];
+};
+
+/**
+ * struct dccp_hdr_ext - the low bits of a 48 bit seq packet
+ *
+ * @dccph_seq_low - low 24 bits of a 48 bit seq packet
+ */
+struct dccp_hdr_ext {
+ u_int32_t dccph_seq_low;
+ u_int32_t dccph_options[0];
+};
+
+/**
+ * struct dccp_hdr_request - Conection initiation request header
+ *
+ * @dccph_req_service - Service to which the client app wants to connect
+ * @dccph_req_options - list of options (must be a multiple of 32 bits
+ */
+struct dccp_hdr_request {
+ u_int32_t dccph_req_service;
+ u_int32_t dccph_req_options[0];
+};
+
+/**
+ * struct dccp_hdr_ack_bits - acknowledgment bits common to most packets
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ */
+struct dccp_hdr_ack_bits {
+ u_int32_t dccph_reserved1:8,
+ dccph_ack_nr_high:24;
+ u_int32_t dccph_ack_nr_low;
+ u_int32_t dccph_options[0];
+};
+
+/**
+ * struct dccp_hdr_response - Conection initiation response header
+ *
+ * @dccph_resp_ack_nr_high - 48 bit ack number high order bits, contains GSR
+ * @dccph_resp_ack_nr_low - 48 bit ack number low order bits, contains GSR
+ * @dccph_resp_service - Echoes the Service Code on a received DCCP-Request
+ * @dccph_resp_options - list of options (must be a multiple of 32 bits
+ */
+struct dccp_hdr_response {
+ struct dccp_hdr_ack_bits dccph_resp_ack;
+ u_int32_t dccph_resp_service;
+ u_int32_t dccph_resp_options[0];
+};
+
+/**
+ * struct dccp_hdr_data - DataAck and Ack packets
+ *
+ * @dccph_data_options - list of options (must be a multiple of 32 bits
+ */
+struct dccp_hdr_data {
+ struct dccp_hdr_ack_bits dccph_data_ack;
+ u_int32_t dccph_data_options[0];
+};
+
+static inline struct dccp_hdr_data *dccp_hdr_data(struct dccp_hdr *hdrg)
+{
+ const int ext = hdrg->dccph_x ? sizeof(struct dccp_hdr_ext) : 0;
+
+ return (struct dccp_hdr_data *)(((u_char *)hdrg) + sizeof(hdrg) + ext);
+}
+
+#define dccp_hdr_ack(hdrg) dccp_hdr_data(hdrg)
+
+/**
+ * struct dccp_hdr_close - Conection CloseReq and Close packets
+ *
+ * @dccph_close_options - list of options (must be a multiple of 32 bits
+ */
+struct dccp_hdr_close {
+ struct dccp_hdr_ack_bits dccph_close_ack;
+ u_int32_t dccph_close_options[0];
+};
+
+/**
+ * struct dccp_hdr_reset - Unconditionally shut down a connection
+ *
+ * @dccph_reset_service - Echoes the Service Code on a received DCCP-Request
+ * @dccph_reset_options - list of options (m
