Re: AUDIT_NETFILTER_PKT message format

2017-02-13 Thread Richard Guy Briggs 
On 2017-02-13 18:50, Paul Moore wrote:
> On Mon, Feb 13, 2017 at 3:50 PM, Richard Guy Briggs  wrote:
> > On 2017-02-13 12:57, Steve Grubb wrote:
> >> On Friday, February 10, 2017 5:54:45 PM EST Richard Guy Briggs wrote:
> >> > On 2017-02-10 17:39, Steve Grubb wrote:
> >> > > > The alternatives that I currently see are to drop packets for which
> >> > > > there is no local process ownership, or to leave the ownership fields
> >> > > > unset.>
> >> >
> >> > > What ownership fields are we talking about?
> >> >
> >> > The ones you want, auid, pid, ses.  Perhaps I'm using the wrong
> >> > terminology.  What technical term is there for the collection of subject
> >> > identifiers?
> >>
> >> Subject attributes.
> >
> > Ah ok, I'll try to remember to use that term...
> >
> > Now that you know what I'm talking about, can you go back and answer the
> > questions I had about packet "ownership" (which is really packet subject
> > attributes)?  If we have that information, how to we include it in the
> > message format?  And if we don't have it, do we ignore the packet, or do
> > we swing fields out, or do we set those fields to "unset" or do we use
> > an auxiliary record?
> 
> Packet "ownership" is likely going to be impossible to determine
> reliably since in some cases you can't even match a packet to a
> socket, let alone a process.  To back up a few messages in this
> thread, to Richard's list of things to potentially log:
> 
> > helpful action, hook
> 
> I haven't checked, but do we allow setting of an audit key in
> NETFILTER_PKT records?  It seems like that might be a good thing for
> the userspace tools and would likely make logging the action/hook
> unncessary.

Not that I am aware of.  That would be way useful if it were possible.
"AUDIT" is a netfilter target and you can set the type to "accept",
"drop" or "reject".  Similarly, having the sub-chain name would be
useful but that doesn't appear to be available either.  This is why I
used a "mark" in the testsuite to track packets.

> > useless?len
> 
> I don't see much point in this.
> 
> > helpful inif, outif, mark
> 
> Let's split this into two things: the interfaces and the mark.  I
> don't see much value in logging the mark, but I could see some value
> in logging the interface.

In fact, the mark I found to be a useful way to track which rule was
involved and I'd be pretty surprised if others don't try to do the same.

> > useless?smac, dmac, macproto
> 
> Probably useless in the majority of use cases.

How do we deal with the minority of cases where it could be quite useful?

> > helpful protocol family
> 
> I think we need some clarity on protocol logging; we've got "macproto"
> (I assume this is the ethertype, or similar), "protocol family" (I
> assume this to be a duplicate of ethertype, e.g. AF_INET), and "proto"
> (see below, I assume this to be TCP/UDP/etc.).

Sorry, you are right.  I know that field as "ethertype" which defines
the "protocol family" (network layer protocol, IPv4/6, etc...).  "proto"
is the transport layer protocol.  For some reason, I was thinking
"macproto" was the link layer type, but that's obvious from the media.

> > useless?truncated
> 
> Definitely useless.  Only keep this if we need it for some backwards
> compatibility.
> 
> > helpful saddr, daddr
> 
> Helpful.
> 
> > useless?ipid
> 
> Useless.
> 
> > helpful proto
> > helpful sport, dport
> 
> Assuming "proto" means the TCP/UDP/etc. then we should treat the
> proto/ports as one block; you can't log the ports without logging
> "proto".
> 
> > useless?frag
> > useless?truncated
> 
> Yes, useless.
> 
> > helpful icmptype, icmpcode
> 
> Similar to proto/port above.
> 
> > helpful secmark (I forgot to change it from "obj" to "secmark" in 
> > my patch).
> 
> We may also want to log the peer label if we are going to log the secmark.

Ok, noted.

> paul moore

- RGB

--
Richard Guy Briggs 
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AUDIT_NETFILTER_PKT message format

2017-02-13 Thread Paul Moore 
On Mon, Feb 13, 2017 at 3:50 PM, Richard Guy Briggs  wrote:
> On 2017-02-13 12:57, Steve Grubb wrote:
>> On Friday, February 10, 2017 5:54:45 PM EST Richard Guy Briggs wrote:
>> > On 2017-02-10 17:39, Steve Grubb wrote:
>> > > > The alternatives that I currently see are to drop packets for which
>> > > > there is no local process ownership, or to leave the ownership fields
>> > > > unset.>
>> >
>> > > What ownership fields are we talking about?
>> >
>> > The ones you want, auid, pid, ses.  Perhaps I'm using the wrong
>> > terminology.  What technical term is there for the collection of subject
>> > identifiers?
>>
>> Subject attributes.
>
> Ah ok, I'll try to remember to use that term...
>
> Now that you know what I'm talking about, can you go back and answer the
> questions I had about packet "ownership" (which is really packet subject
> attributes)?  If we have that information, how to we include it in the
> message format?  And if we don't have it, do we ignore the packet, or do
> we swing fields out, or do we set those fields to "unset" or do we use
> an auxiliary record?

Packet "ownership" is likely going to be impossible to determine
reliably since in some cases you can't even match a packet to a
socket, let alone a process.  To back up a few messages in this
thread, to Richard's list of things to potentially log:

> helpful action, hook

I haven't checked, but do we allow setting of an audit key in
NETFILTER_PKT records?  It seems like that might be a good thing for
the userspace tools and would likely make logging the action/hook
unncessary.

> useless?len

I don't see much point in this.

> helpful inif, outif, mark

Let's split this into two things: the interfaces and the mark.  I
don't see much value in logging the mark, but I could see some value
in logging the interface.

> useless?smac, dmac, macproto

Probably useless in the majority of use cases.

> helpful protocol family

I think we need some clarity on protocol logging; we've got "macproto"
(I assume this is the ethertype, or similar), "protocol family" (I
assume this to be a duplicate of ethertype, e.g. AF_INET), and "proto"
(see below, I assume this to be TCP/UDP/etc.).

> useless?truncated

Definitely useless.  Only keep this if we need it for some backwards
compatibility.

> helpful saddr, daddr

Helpful.

> useless?ipid

Useless.

> helpful proto
> helpful sport, dport

Assuming "proto" means the TCP/UDP/etc. then we should treat the
proto/ports as one block; you can't log the ports without logging
"proto".

> useless?frag
> useless?truncated

Yes, useless.

> helpful icmptype, icmpcode

Similar to proto/port above.

> helpful secmark (I forgot to change it from "obj" to "secmark" in my 
> patch).

We may also want to log the peer label if we are going to log the secmark.

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf-next] netfilter: nft_ct: fix random validation errors for zone set support

2017-02-13 Thread Florian Westphal 
Dan reports:
 net/netfilter/nft_ct.c:549 nft_ct_set_init()
 error: uninitialized symbol 'len'.

Reported-by: Dan Carpenter 
Fixes: edee4f1e924582 ("netfilter: nft_ct: add zone id set support")
Signed-off-by: Florian Westphal 
---
 net/netfilter/nft_ct.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index c6b8022c0e47..bf548a7a71ec 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -528,6 +528,7 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
if (!nft_ct_tmpl_alloc_pcpu())
return -ENOMEM;
nft_ct_pcpu_template_refcnt++;
+   len = sizeof(u16);
break;
 #endif
default:
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AUDIT_NETFILTER_PKT message format

2017-02-13 Thread Richard Guy Briggs 
On 2017-02-13 12:57, Steve Grubb wrote:
> On Friday, February 10, 2017 5:54:45 PM EST Richard Guy Briggs wrote:
> > On 2017-02-10 17:39, Steve Grubb wrote:
> > > > The alternatives that I currently see are to drop packets for which
> > > > there is no local process ownership, or to leave the ownership fields
> > > > unset.> 
> >
> > > What ownership fields are we talking about?
> > 
> > The ones you want, auid, pid, ses.  Perhaps I'm using the wrong
> > terminology.  What technical term is there for the collection of subject
> > identifiers?
> 
> Subject attributes.

Ah ok, I'll try to remember to use that term...

Now that you know what I'm talking about, can you go back and answer the
questions I had about packet "ownership" (which is really packet subject
attributes)?  If we have that information, how to we include it in the
message format?  And if we don't have it, do we ignore the packet, or do
we swing fields out, or do we set those fields to "unset" or do we use
an auxiliary record?

> > > > > I don't think audit should worry about spoofing. Yes it can be done,
> > > > > but we should accurately record what was presented to the system.
> > > > > Other tools can be employed to watch for arp spoofing and source 
> > > > > routed
> > > > > packets. Its a bigger problem than just the audit logs.
> > > > 
> > > > I find this statement a bit surprising given we're trying to find out
> > > > who's doing what where.
> > > 
> > > We're just recording what's presented to the system that meets the rules
> > > programmed in.
> > 
> > I don't quite understand.  Are you saying only display the fields that
> > were specifically used in the netfilter rule to trigger the target that
> > records a packet?
> 
> No. I'm saying we shouldn't do any processing to figure out if we have a 
> spoofed or source routed packet. There are other tools that do that kind of 
> thing.

I never suggested that.  I only suggested including that information so
that some other tool actually has the information to work with.

> > I don't think that's what you want and it isn't easy
> > to get without being more invasive in netfilter and swinging fields.
> > I'd record the MAC header since it is part of the packet that tells us
> > where it came from and where it's going.
> 
> Do we really need the MAC header for every event? I really don't think so.

It certainly makes my job simpler to just ignore the MAC header and
avoid complicating things, but if I were a network admin and a packet
came in that I wasn't expecting because of other network rules that had
been set up to prevent it, I'd want more information to figure out why.

> -Steve

- RGB

--
Richard Guy Briggs 
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] netfilter: nft_ct: add zone id set support

2017-02-13 Thread Dan Carpenter 
Hello Florian Westphal,

The patch edee4f1e9245: "netfilter: nft_ct: add zone id set support"
from Feb 3, 2017, leads to the following static checker warning:

net/netfilter/nft_ct.c:549 nft_ct_set_init()
error: uninitialized symbol 'len'.

net/netfilter/nft_ct.c
   498  static int nft_ct_set_init(const struct nft_ctx *ctx,
   499 const struct nft_expr *expr,
   500 const struct nlattr * const tb[])
   501  {
   502  struct nft_ct *priv = nft_expr_priv(expr);
   503  unsigned int len;


   504  int err;
   505  
   506  priv->dir = IP_CT_DIR_MAX;
   507  priv->key = ntohl(nla_get_be32(tb[NFTA_CT_KEY]));
   508  switch (priv->key) {
   509  #ifdef CONFIG_NF_CONNTRACK_MARK
   510  case NFT_CT_MARK:
   511  if (tb[NFTA_CT_DIRECTION])
   512  return -EINVAL;
   513  len = FIELD_SIZEOF(struct nf_conn, mark);
   514  break;
   515  #endif
   516  #ifdef CONFIG_NF_CONNTRACK_LABELS
   517  case NFT_CT_LABELS:
   518  if (tb[NFTA_CT_DIRECTION])
   519  return -EINVAL;
   520  len = NF_CT_LABELS_MAX_SIZE;
   521  err = nf_connlabels_get(ctx->net, (len * BITS_PER_BYTE) 
- 1);
   522  if (err)
   523  return err;
   524  break;
   525  #endif
   526  #ifdef CONFIG_NF_CONNTRACK_ZONES
   527  case NFT_CT_ZONE:

"len" not set for this case statement.

   528  if (!nft_ct_tmpl_alloc_pcpu())
   529  return -ENOMEM;
   530  nft_ct_pcpu_template_refcnt++;
   531  break;
   532  #endif
   533  default:
   534  return -EOPNOTSUPP;
   535  }
   536  
   537  if (tb[NFTA_CT_DIRECTION]) {
   538  priv->dir = nla_get_u8(tb[NFTA_CT_DIRECTION]);
   539  switch (priv->dir) {
   540  case IP_CT_DIR_ORIGINAL:
   541  case IP_CT_DIR_REPLY:
   542  break;
   543  default:
   544  return -EINVAL;
   545  }
   546  }
   547  
   548  priv->sreg = nft_parse_register(tb[NFTA_CT_SREG]);
   549  err = nft_validate_register_load(priv->sreg, len);
 ^^^
Which seems probably bad.

   550  if (err < 0)
   551  goto err1;
   552  
   553  err = nft_ct_netns_get(ctx->net, ctx->afi->family);
   554  if (err < 0)
   555  goto err1;

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AUDIT_NETFILTER_PKT message format

2017-02-13 Thread Steve Grubb 
On Friday, February 10, 2017 5:54:45 PM EST Richard Guy Briggs wrote:
> On 2017-02-10 17:39, Steve Grubb wrote:
> > > The alternatives that I currently see are to drop packets for which
> > > there is no local process ownership, or to leave the ownership fields
> > > unset.> 
>
> > What ownership fields are we talking about?
> 
> The ones you want, auid, pid, ses.  Perhaps I'm using the wrong
> terminology.  What technical term is there for the collection of subject
> identifiers?

Subject attributes.

 
> > > > I don't think audit should worry about spoofing. Yes it can be done,
> > > > but we should accurately record what was presented to the system.
> > > > Other tools can be employed to watch for arp spoofing and source routed
> > > > packets. Its a bigger problem than just the audit logs.
> > > 
> > > I find this statement a bit surprising given we're trying to find out
> > > who's doing what where.
> > 
> > We're just recording what's presented to the system that meets the rules
> > programmed in.
> 
> I don't quite understand.  Are you saying only display the fields that
> were specifically used in the netfilter rule to trigger the target that
> records a packet?

No. I'm saying we shouldn't do any processing to figure out if we have a 
spoofed or source routed packet. There are other tools that do that kind of 
thing.


> I don't think that's what you want and it isn't easy
> to get without being more invasive in netfilter and swinging fields.
> I'd record the MAC header since it is part of the packet that tells us
> where it came from and where it's going.

Do we really need the MAC header for every event? I really don't think so.

-Steve
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nft] doc: Document maps

2017-02-13 Thread Elise Lennion 
This patch adds the missing documentation for maps. Also, updates sets
policy to match maps.

Signed-off-by: Elise Lennion 
---
 doc/nft.xml | 105 +++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/doc/nft.xml b/doc/nft.xml
index 2825810..ed97859 100644
--- a/doc/nft.xml
+++ b/doc/nft.xml
@@ -784,7 +784,110 @@ filter input iif $int_ifs accept

policy
set policy
-   string: performance, 
memory
+   string: performance 
[default], memory
+   
+   
+   
+   
+   
+
+   
+   Maps
+   
+   
+   add
+map
+   family
+   table
+   map
+   {
+   type
+   flags
+   elements
+   size
+   policy
+   }
+   
+   
+   
+   delete
+   list
+   
+map
+   family
+   table
+   map
+   
+   
+   
+   Maps store data based on some specific key used as 
input, they are uniquely identified by an user-defined name and attached to 
tables.
+   
+
+   
+   
+   add
+   
+   
+   Add a new map in the specified 
table.
+   
+   
+   
+   
+   delete
+   
+   
+   Delete the specified map.
+   
+   
+   
+   
+   list
+   
+   
+   Display the elements in the 
specified map.
+   
+   
+   
+   
+
+   
+   Map specifications
+   
+   
+   
+   
+   
+   
+   Keyword
+   Description
+   Type
+   
+   
+   
+   
+   type
+   data type of map 
elements
+   string ':' string:  
ipv4_addr, ipv6_addr, ether_addr, inet_proto, inet_service, mark, counter, 
quota. Counter and quota can't be used as keys
+   
+   
+   flags
+   map flags
+   string: constant, 
interval
+   
+   
+   elements
+   elements contained by 
the map
+   map data type
+   
+   
+   size
+   maximun number of 
elements in the map
+   unsigned integer (64 
bit)
+   
+   
+   policy
+   map policy
+   string:

[PATCH ulogd2 2/2] rotate all default output files

2017-02-13 Thread Kaarle Ritvanen 
Signed-off-by: Kaarle Ritvanen 
---
 ulogd.logrotate | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ulogd.logrotate b/ulogd.logrotate
index b3fb6d1..3b4c487 100644
--- a/ulogd.logrotate
+++ b/ulogd.logrotate
@@ -1,4 +1,4 @@
-/var/log/ulogd.log /var/log/ulogd.syslogemu /var/log/ulogd.pktlog 
/var/log/ulogd.pcap {
+/var/log/ulogd.log /var/log/ulogd.syslogemu /var/log/ulogd.pktlog 
/var/log/ulogd.gprint /var/log/ulogd.json /var/log/ulogd.pcap 
/var/log/ulogd.sqlite3db /var/log/nacctdata.log {
 missingok
 sharedscripts
 postrotate
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html