[netsniff-ng] Re: [RFC 6/7] trafgen: l7: Add DNS header generation API
On 2017-03-14 at 20:15:45 +0100, Vadim Kochan wrote: > On Tue, Mar 14, 2017 at 12:21 PM, Tobias Klauser wrote: > > On 2017-01-30 at 09:33:29 +0100, Vadim Kochan wrote: > >> Add trafgen_l7.c module with DNS proto header generation with > >> support of filling DNS query/answer/authority/additional sections > >> as sub headers. > >> > >> Introcuded new concept as 'sub header' which is needed to easy handle > >> DNS sections which might be added on-demand, and to simplify using > >> sub-header as regular header with a fields, offset, etc. There is a > >> parent header which contains array of pointers of sub-headers, and the > >> array is ordered as they are located in the parent header. The > >> sub-headers mostly encapsulated by the parent header which 'knows' > >> the semantic of them. The new proto_hdr->push_sub_header(...) callback > >> was added to tell the parent header to push the sub-header's fields, > >> sub-header also may have proto_ops which must be filled by the parent. > >> This sub-header concept might be used in the future if it will be needed > >> to support DHCP, WLAN headers. > >> > >> There are 4 kinds of DNS sub-headers - query, answer, authority, > >> additional. 'id' of each sub-header is used to only differentiate these > >> types of sections. These sections have strict order inside DNS header, > >> and there was added the proto_hdr_move_sub_header(...) to sort them in > >> required order. > > > > Might be a bit of a naive question: But wouldn't it be possible to > > enforce the sub-header order through the parser (i.e. only allow > > trafgen scripts which specify the respective sections in the right > > order? This would safe us from doing the whole header sorting/moving > > dance which looks a bit sacry to me (memmove of payload especially). > > > > Well, I was thinking about this and decided to do so because: > > 1) I really think that there might be in the future libtrafgen > which will handle sub-headers > automatically (if it is needed by protocol header) both for > the application and trafgen tool (this > reason might be not so strict). Well, that's a bit hypothetical ;) And a don't quite see how this prevents us from going with the easier, less scary solution for now ;) > 2) sub-headers might be a generic way for other protocol headers > like DHCP, WLAN. We could enforce sub-header order there too, no? > Yes, the logic which does sub-headers moving is a bit scary as you > said, not sure if I can re-write it > to simplify it, will try. Ok, let's try that. I'm not strictly against the current solution and I see that there might be cases in the future where this would be benefitial. I'd just like to prevent us from introducing a lot of code with growing complexity which could be prevented if we were a bit more restrictive in terms of what the user can do. Thanks Tobias -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to netsniff-ng+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[netsniff-ng] Re: [RFC 6/7] trafgen: l7: Add DNS header generation API
On Tue, Mar 14, 2017 at 12:21 PM, Tobias Klauser wrote: > On 2017-01-30 at 09:33:29 +0100, Vadim Kochan wrote: >> Add trafgen_l7.c module with DNS proto header generation with >> support of filling DNS query/answer/authority/additional sections >> as sub headers. >> >> Introcuded new concept as 'sub header' which is needed to easy handle >> DNS sections which might be added on-demand, and to simplify using >> sub-header as regular header with a fields, offset, etc. There is a >> parent header which contains array of pointers of sub-headers, and the >> array is ordered as they are located in the parent header. The >> sub-headers mostly encapsulated by the parent header which 'knows' >> the semantic of them. The new proto_hdr->push_sub_header(...) callback >> was added to tell the parent header to push the sub-header's fields, >> sub-header also may have proto_ops which must be filled by the parent. >> This sub-header concept might be used in the future if it will be needed >> to support DHCP, WLAN headers. >> >> There are 4 kinds of DNS sub-headers - query, answer, authority, >> additional. 'id' of each sub-header is used to only differentiate these >> types of sections. These sections have strict order inside DNS header, >> and there was added the proto_hdr_move_sub_header(...) to sort them in >> required order. > > Might be a bit of a naive question: But wouldn't it be possible to > enforce the sub-header order through the parser (i.e. only allow > trafgen scripts which specify the respective sections in the right > order? This would safe us from doing the whole header sorting/moving > dance which looks a bit sacry to me (memmove of payload especially). > Well, I was thinking about this and decided to do so because: 1) I really think that there might be in the future libtrafgen which will handle sub-headers automatically (if it is needed by protocol header) both for the application and trafgen tool (this reason might be not so strict). 2) sub-headers might be a generic way for other protocol headers like DHCP, WLAN. Yes, the logic which does sub-headers moving is a bit scary as you said, not sure if I can re-write it to simplify it, will try. Regards, Vadim Kochan -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to netsniff-ng+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[netsniff-ng] Re: [RFC 6/7] trafgen: l7: Add DNS header generation API
On 2017-01-30 at 09:33:29 +0100, Vadim Kochan wrote: > Add trafgen_l7.c module with DNS proto header generation with > support of filling DNS query/answer/authority/additional sections > as sub headers. > > Introcuded new concept as 'sub header' which is needed to easy handle > DNS sections which might be added on-demand, and to simplify using > sub-header as regular header with a fields, offset, etc. There is a > parent header which contains array of pointers of sub-headers, and the > array is ordered as they are located in the parent header. The > sub-headers mostly encapsulated by the parent header which 'knows' > the semantic of them. The new proto_hdr->push_sub_header(...) callback > was added to tell the parent header to push the sub-header's fields, > sub-header also may have proto_ops which must be filled by the parent. > This sub-header concept might be used in the future if it will be needed > to support DHCP, WLAN headers. > > There are 4 kinds of DNS sub-headers - query, answer, authority, > additional. 'id' of each sub-header is used to only differentiate these > types of sections. These sections have strict order inside DNS header, > and there was added the proto_hdr_move_sub_header(...) to sort them in > required order. Might be a bit of a naive question: But wouldn't it be possible to enforce the sub-header order through the parser (i.e. only allow trafgen scripts which specify the respective sections in the right order? This would safe us from doing the whole header sorting/moving dance which looks a bit sacry to me (memmove of payload especially). A few minor comments inline below. > Actually there are only 2 proto_hdr's which describes 4 DNS sections - > query & rrecord, because rrecord covers another 3 - answer, auhority, > additional which have the same layout. > > Signed-off-by: Vadim Kochan > --- > trafgen/Makefile | 1 + > trafgen_l4.c | 32 ++ > trafgen_l7.c | 175 > +++ > trafgen_l7.h | 45 ++ > trafgen_proto.c | 128 > trafgen_proto.h | 18 +- > 6 files changed, 398 insertions(+), 1 deletion(-) > create mode 100644 trafgen_l7.c > create mode 100644 trafgen_l7.h > > diff --git a/trafgen/Makefile b/trafgen/Makefile > index 876ed93..95a31e0 100644 > --- a/trafgen/Makefile > +++ b/trafgen/Makefile > @@ -25,6 +25,7 @@ trafgen-objs = xmalloc.o \ > trafgen_l2.o \ > trafgen_l3.o \ > trafgen_l4.o \ > + trafgen_l7.o \ > trafgen_lexer.yy.o \ > trafgen_parser.tab.o \ > trafgen.o > diff --git a/trafgen_l4.c b/trafgen_l4.c > index 5a694b3..198d622 100644 > --- a/trafgen_l4.c > +++ b/trafgen_l4.c > @@ -80,6 +80,21 @@ static void udp_packet_finish(struct proto_hdr *hdr) > udp_csum_update(hdr); > } > > +static void udp_set_next_proto(struct proto_hdr *hdr, enum proto_id pid) > +{ > + uint16_t dport; > + > + switch (pid) { > + case PROTO_DNS: > + dport = 53; > + break; > + default: > + bug(); > + } > + > + proto_hdr_field_set_default_be16(hdr, UDP_DPORT, dport); > +} > + > static const struct proto_ops udp_proto_ops = { > .id = PROTO_UDP, > .layer = PROTO_L4, > @@ -87,6 +102,7 @@ static const struct proto_ops udp_proto_ops = { > .packet_update = udp_csum_update, > .packet_finish = udp_packet_finish, > .field_changed = udp_field_changed, > + .set_next_proto = udp_set_next_proto, > }; > > static struct proto_field tcp_fields[] = { > @@ -160,6 +176,21 @@ static void tcp_csum_update(struct proto_hdr *hdr) > hdr->is_csum_valid = true; > } > > +static void tcp_set_next_proto(struct proto_hdr *hdr, enum proto_id pid) > +{ > + uint16_t dport; > + > + switch (pid) { > + case PROTO_DNS: > + dport = 53; > + break; > + default: > + bug(); > + } > + > + proto_hdr_field_set_default_be16(hdr, TCP_DPORT, dport); > +} > + > static const struct proto_ops tcp_proto_ops = { > .id = PROTO_TCP, > .layer = PROTO_L4, > @@ -167,6 +198,7 @@ static const struct proto_ops tcp_proto_ops = { > .packet_update = tcp_csum_update, > .packet_finish = tcp_csum_update, > .field_changed = tcp_field_changed, > + .set_next_proto = tcp_set_next_proto, > }; > > static struct proto_field icmpv4_fields[] = { > diff --git a/trafgen_l7.c b/trafgen_l7.c > new file mode 100644 > index 000..1e82ccb > --- /dev/null > +++ b/trafgen_l7.c > @@ -0,0 +1,175 @@ > +/* > + * netsniff-ng - the packet sniffing beast > + * Subject to the GPL, version 2. > + */ > + > +#include > + > +#include "str.h" > +#include "xmalloc.h" > +#include "built_in.h" > +#include "trafgen_l7.h" > +#include "trafgen_proto.h" > + > +static struct proto_field dns_fields[]