[PATCH net-next] ipv6: Reinject IPv6 packets if IPsec policy matches after SNAT

2017-12-21 Thread Tobias Brunner
If SNAT modifies the source address the resulting packet might match
an IPsec policy, reinject the packet if that's the case.

The exact same thing is already done for IPv4.

Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 net/ipv6/ip6_output.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 176d74fb3b4d..c90f02632782 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -138,6 +138,14 @@ static int ip6_finish_output(struct net *net, struct sock 
*sk, struct sk_buff *s
return ret;
}
 
+#if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
+   /* Policy lookup after SNAT yielded a new policy */
+   if (skb_dst(skb)->xfrm) {
+   IPCB(skb)->flags |= IPSKB_REROUTED;
+   return dst_output(net, sk, skb);
+   }
+#endif
+
if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
dst_allfrag(skb_dst(skb)) ||
(IP6CB(skb)->frag_max_size && skb->len > IP6CB(skb)->frag_max_size))
-- 
2.7.4


[PATCH net 2/2] esp6: Fix integrity verification when ESN are used

2016-11-29 Thread Tobias Brunner
When handling inbound packets, the two halves of the sequence number
stored on the skb are already in network order.

Fixes: 000ae7b2690e ("esp6: Switch to new AEAD interface")
Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 net/ipv6/esp6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 060a60b2f8a6..111ba55fd512 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -418,7 +418,7 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff 
*skb)
esph = (void *)skb_push(skb, 4);
*seqhi = esph->spi;
esph->spi = esph->seq_no;
-   esph->seq_no = htonl(XFRM_SKB_CB(skb)->seq.input.hi);
+   esph->seq_no = XFRM_SKB_CB(skb)->seq.input.hi;
aead_request_set_callback(req, 0, esp_input_done_esn, skb);
}
 
-- 
1.9.1


[PATCH net 1/2] esp4: Fix integrity verification when ESN are used

2016-11-29 Thread Tobias Brunner
When handling inbound packets, the two halves of the sequence number
stored on the skb are already in network order.

Fixes: 7021b2e1cddd ("esp4: Switch to new AEAD interface")
Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 net/ipv4/esp4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index d95631d09248..20fb25e3027b 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -476,7 +476,7 @@ static int esp_input(struct xfrm_state *x, struct sk_buff 
*skb)
esph = (void *)skb_push(skb, 4);
*seqhi = esph->spi;
esph->spi = esph->seq_no;
-   esph->seq_no = htonl(XFRM_SKB_CB(skb)->seq.input.hi);
+   esph->seq_no = XFRM_SKB_CB(skb)->seq.input.hi;
aead_request_set_callback(req, 0, esp_input_done_esn, skb);
}
 
-- 
1.9.1


[PATCH v2 net] macsec: Fix header length if SCI is added if explicitly disabled

2016-10-24 Thread Tobias Brunner
Even if sending SCIs is explicitly disabled, the code that creates the
Security Tag might still decide to add it (e.g. if multiple RX SCs are
defined on the MACsec interface).
But because the header length so far only depended on the configuration
option the SCI overwrote the original frame's contents (EtherType and
e.g. the beginning of the IP header) and if encrypted did not visibly
end up in the packet, while the SC flag in the TCI field of the Security
Tag was still set, resulting in invalid MACsec frames.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 drivers/net/macsec.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 3ea47f28e143..d2e61e002926 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -397,6 +397,14 @@ static struct macsec_cb *macsec_skb_cb(struct sk_buff *skb)
 #define DEFAULT_ENCRYPT false
 #define DEFAULT_ENCODING_SA 0
 
+static bool send_sci(const struct macsec_secy *secy)
+{
+   const struct macsec_tx_sc *tx_sc = >tx_sc;
+
+   return tx_sc->send_sci ||
+   (secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb);
+}
+
 static sci_t make_sci(u8 *addr, __be16 port)
 {
sci_t sci;
@@ -437,15 +445,15 @@ static unsigned int macsec_extra_len(bool sci_present)
 
 /* Fill SecTAG according to IEEE 802.1AE-2006 10.5.3 */
 static void macsec_fill_sectag(struct macsec_eth_header *h,
-  const struct macsec_secy *secy, u32 pn)
+  const struct macsec_secy *secy, u32 pn,
+  bool sci_present)
 {
const struct macsec_tx_sc *tx_sc = >tx_sc;
 
-   memset(>tci_an, 0, macsec_sectag_len(tx_sc->send_sci));
+   memset(>tci_an, 0, macsec_sectag_len(sci_present));
h->eth.h_proto = htons(ETH_P_MACSEC);
 
-   if (tx_sc->send_sci ||
-   (secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb)) {
+   if (sci_present) {
h->tci_an |= MACSEC_TCI_SC;
memcpy(>secure_channel_id, >sci,
   sizeof(h->secure_channel_id));
@@ -650,6 +658,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
struct macsec_tx_sc *tx_sc;
struct macsec_tx_sa *tx_sa;
struct macsec_dev *macsec = macsec_priv(dev);
+   bool sci_present;
u32 pn;
 
secy = >secy;
@@ -687,7 +696,8 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 
unprotected_len = skb->len;
eth = eth_hdr(skb);
-   hh = (struct macsec_eth_header *)skb_push(skb, 
macsec_extra_len(tx_sc->send_sci));
+   sci_present = send_sci(secy);
+   hh = (struct macsec_eth_header *)skb_push(skb, 
macsec_extra_len(sci_present));
memmove(hh, eth, 2 * ETH_ALEN);
 
pn = tx_sa_update_pn(tx_sa, secy);
@@ -696,7 +706,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
kfree_skb(skb);
return ERR_PTR(-ENOLINK);
}
-   macsec_fill_sectag(hh, secy, pn);
+   macsec_fill_sectag(hh, secy, pn, sci_present);
macsec_set_shortlen(hh, unprotected_len - 2 * ETH_ALEN);
 
skb_put(skb, secy->icv_len);
@@ -726,10 +736,10 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
skb_to_sgvec(skb, sg, 0, skb->len);
 
if (tx_sc->encrypt) {
-   int len = skb->len - macsec_hdr_len(tx_sc->send_sci) -
+   int len = skb->len - macsec_hdr_len(sci_present) -
  secy->icv_len;
aead_request_set_crypt(req, sg, sg, len, iv);
-   aead_request_set_ad(req, macsec_hdr_len(tx_sc->send_sci));
+   aead_request_set_ad(req, macsec_hdr_len(sci_present));
} else {
aead_request_set_crypt(req, sg, sg, 0, iv);
aead_request_set_ad(req, skb->len - secy->icv_len);
-- 
1.9.1


Re: [PATCH net] macsec: Fix header length if SCI is added if explicitily disabled

2016-10-24 Thread Tobias Brunner
> [snip]
>> @@ -440,12 +448,12 @@ static void macsec_fill_sectag(struct 
>> macsec_eth_header *h,
>> const struct macsec_secy *secy, u32 pn)
>>  {
>>  const struct macsec_tx_sc *tx_sc = >tx_sc;
>> +bool sci_present = send_sci(secy);
> 
> You're already computing this in macsec_encrypt() just before calling
> macsec_fill_sectag(), so you could pass it as argument instead of
> recomputing it.

Right, I'll send a v2.  Would you like me to inline the send_sci()
function, as it will only be called once afterwards.

Regards,
Tobias



[PATCH net] macsec: Fix header length if SCI is added if explicitily disabled

2016-10-21 Thread Tobias Brunner
Even if sending SCIs is explicitly disabled, the code that creates the
Security Tag might still decide to add it (e.g. if multiple RX SCs are
defined on the MACsec interface).
But because the header length so far only depended on the configuration
option the SCI might not actually have ended up in the packet, while the
SC flag in the TCI field of the Security Tag was still set, resulting
in invalid MACsec frames.

Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 drivers/net/macsec.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 3ea47f28e143..246679f24a44 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -397,6 +397,14 @@ static struct macsec_cb *macsec_skb_cb(struct sk_buff *skb)
 #define DEFAULT_ENCRYPT false
 #define DEFAULT_ENCODING_SA 0
 
+static bool send_sci(const struct macsec_secy *secy)
+{
+   const struct macsec_tx_sc *tx_sc = >tx_sc;
+
+   return tx_sc->send_sci ||
+   (secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb);
+}
+
 static sci_t make_sci(u8 *addr, __be16 port)
 {
sci_t sci;
@@ -440,12 +448,12 @@ static void macsec_fill_sectag(struct macsec_eth_header 
*h,
   const struct macsec_secy *secy, u32 pn)
 {
const struct macsec_tx_sc *tx_sc = >tx_sc;
+   bool sci_present = send_sci(secy);
 
-   memset(>tci_an, 0, macsec_sectag_len(tx_sc->send_sci));
+   memset(>tci_an, 0, macsec_sectag_len(sci_present));
h->eth.h_proto = htons(ETH_P_MACSEC);
 
-   if (tx_sc->send_sci ||
-   (secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb)) {
+   if (sci_present) {
h->tci_an |= MACSEC_TCI_SC;
memcpy(>secure_channel_id, >sci,
   sizeof(h->secure_channel_id));
@@ -650,6 +658,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
struct macsec_tx_sc *tx_sc;
struct macsec_tx_sa *tx_sa;
struct macsec_dev *macsec = macsec_priv(dev);
+   bool sci_present;
u32 pn;
 
secy = >secy;
@@ -687,7 +696,8 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 
unprotected_len = skb->len;
eth = eth_hdr(skb);
-   hh = (struct macsec_eth_header *)skb_push(skb, 
macsec_extra_len(tx_sc->send_sci));
+   sci_present = send_sci(secy);
+   hh = (struct macsec_eth_header *)skb_push(skb, 
macsec_extra_len(sci_present));
memmove(hh, eth, 2 * ETH_ALEN);
 
pn = tx_sa_update_pn(tx_sa, secy);
@@ -726,10 +736,10 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
skb_to_sgvec(skb, sg, 0, skb->len);
 
if (tx_sc->encrypt) {
-   int len = skb->len - macsec_hdr_len(tx_sc->send_sci) -
+   int len = skb->len - macsec_hdr_len(sci_present) -
  secy->icv_len;
aead_request_set_crypt(req, sg, sg, len, iv);
-   aead_request_set_ad(req, macsec_hdr_len(tx_sc->send_sci));
+   aead_request_set_ad(req, macsec_hdr_len(sci_present));
} else {
aead_request_set_crypt(req, sg, sg, 0, iv);
aead_request_set_ad(req, skb->len - secy->icv_len);
-- 
1.9.1


[PATCH ipsec] xfrm: Ignore socket policies when rebuilding hash tables

2016-07-29 Thread Tobias Brunner
Whenever thresholds are changed the hash tables are rebuilt.  This is
done by enumerating all policies and hashing and inserting them into
the right table according to the thresholds and direction.

Because socket policies are also contained in net->xfrm.policy_all but
no hash tables are defined for their direction (dir + XFRM_POLICY_MAX)
this causes a NULL or invalid pointer dereference after returning from
policy_hash_bysel() if the rebuild is done while any socket policies
are installed.

Since the rebuild after changing thresholds is scheduled this crash
could even occur if the userland sets thresholds seemingly before
installing any socket policies.

Fixes: 53c2e285f970 ("xfrm: Do not hash socket policies")
Signed-off-by: Tobias Brunner <tob...@strongswan.org>
---
 net/xfrm/xfrm_policy.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b5e665b3cfb0..45f9cf97ea25 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -626,6 +626,10 @@ static void xfrm_hash_rebuild(struct work_struct *work)
 
/* re-insert all policies by order of creation */
list_for_each_entry_reverse(policy, >xfrm.policy_all, walk.all) {
+   if (xfrm_policy_id2dir(policy->index) >= XFRM_POLICY_MAX) {
+   /* skip socket policies */
+   continue;
+   }
newpos = NULL;
chain = policy_hash_bysel(net, >selector,
  policy->family,
-- 
1.9.1