* [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input
@ 2017-01-16 11:17 Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 1/2] IPsec: do not ignore crypto err in ah4 input Gilad Ben-Yossef
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Gilad Ben-Yossef @ 2017-01-16 11:17 UTC (permalink / raw)
To: steffen.klassert, herbert, davem, netdev
Cc: ofir.drang, gilad.benyossef, Alexander Alemayhu
ah input processing uses the asynchronous hash crypto API which
supplies an error code as part of the operation completion but
the error code was being ignored.
Treat a crypto API error indication as a verification failure.
While a crypto API reported error would almost certainly result
in a memcpy of the digest failing anyway and thus the security
risk seems minor, performing a memory compare on what might be
uninitialized memory is wrong.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Alexander Alemayhu <alexander@alemayhu.com>
---
The change was boot tested on Arm64 but I did not exercise
the specific error code path in question.
Changes from v2:
- Added fix for same problem in IPv6 pointed out by Steffen Klassert
Changes from v1:
- Fixed typo in patch description pointed out by Alexander
Gilad Ben-Yossef (2):
IPsec: do not ignore crypto err in ah4 input
IPsec: do not ignore crypto err in ah6 input
net/ipv4/ah4.c | 3 +++
net/ipv6/ah6.c | 3 +++
2 files changed, 6 insertions(+)
--
2.1.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH ipsec-next v3 1/2] IPsec: do not ignore crypto err in ah4 input
2017-01-16 11:17 [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Gilad Ben-Yossef
@ 2017-01-16 11:17 ` Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 2/2] IPsec: do not ignore crypto err in ah6 input Gilad Ben-Yossef
2017-01-16 13:27 ` [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Steffen Klassert
2 siblings, 0 replies; 4+ messages in thread
From: Gilad Ben-Yossef @ 2017-01-16 11:17 UTC (permalink / raw)
To: steffen.klassert, herbert, davem, netdev; +Cc: ofir.drang, gilad.benyossef
ah4 input processing uses the asynchronous hash crypto API which
supplies an error code as part of the operation completion but
the error code was being ignored.
Treat a crypto API error indication as a verification failure.
While a crypto API reported error would almost certainly result
in a memcpy of the digest failing anyway and thus the security
risk seems minor, performing a memory compare on what might be
uninitialized memory is wrong.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
net/ipv4/ah4.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index f2a7102..22377c8 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -270,6 +270,9 @@ static void ah_input_done(struct crypto_async_request *base, int err)
int ihl = ip_hdrlen(skb);
int ah_hlen = (ah->hdrlen + 2) << 2;
+ if (err)
+ goto out;
+
work_iph = AH_SKB_CB(skb)->tmp;
auth_data = ah_tmp_auth(work_iph, ihl);
icv = ah_tmp_icv(ahp->ahash, auth_data, ahp->icv_trunc_len);
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH ipsec-next v3 2/2] IPsec: do not ignore crypto err in ah6 input
2017-01-16 11:17 [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 1/2] IPsec: do not ignore crypto err in ah4 input Gilad Ben-Yossef
@ 2017-01-16 11:17 ` Gilad Ben-Yossef
2017-01-16 13:27 ` [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Steffen Klassert
2 siblings, 0 replies; 4+ messages in thread
From: Gilad Ben-Yossef @ 2017-01-16 11:17 UTC (permalink / raw)
To: steffen.klassert, herbert, davem, netdev; +Cc: ofir.drang, gilad.benyossef
ah6 input processing uses the asynchronous hash crypto API which
supplies an error code as part of the operation completion but
the error code was being ignored.
Treat a crypto API error indication as a verification failure.
While a crypto API reported error would almost certainly result
in a memcpy of the digest failing anyway and thus the security
risk seems minor, performing a memory compare on what might be
uninitialized memory is wrong.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
net/ipv6/ah6.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 189eb10..dda6035 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -474,6 +474,9 @@ static void ah6_input_done(struct crypto_async_request *base, int err)
int hdr_len = skb_network_header_len(skb);
int ah_hlen = (ah->hdrlen + 2) << 2;
+ if (err)
+ goto out;
+
work_iph = AH_SKB_CB(skb)->tmp;
auth_data = ah_tmp_auth(work_iph, hdr_len);
icv = ah_tmp_icv(ahp->ahash, auth_data, ahp->icv_trunc_len);
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input
2017-01-16 11:17 [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 1/2] IPsec: do not ignore crypto err in ah4 input Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 2/2] IPsec: do not ignore crypto err in ah6 input Gilad Ben-Yossef
@ 2017-01-16 13:27 ` Steffen Klassert
2 siblings, 0 replies; 4+ messages in thread
From: Steffen Klassert @ 2017-01-16 13:27 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: herbert, davem, netdev, ofir.drang, gilad.benyossef, Alexander Alemayhu
On Mon, Jan 16, 2017 at 01:17:54PM +0200, Gilad Ben-Yossef wrote:
> ah input processing uses the asynchronous hash crypto API which
> supplies an error code as part of the operation completion but
> the error code was being ignored.
>
> Treat a crypto API error indication as a verification failure.
>
> While a crypto API reported error would almost certainly result
> in a memcpy of the digest failing anyway and thus the security
> risk seems minor, performing a memory compare on what might be
> uninitialized memory is wrong.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> CC: Alexander Alemayhu <alexander@alemayhu.com>
Both applied to ipsec-next, thanks a lot!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-16 13:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 11:17 [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 1/2] IPsec: do not ignore crypto err in ah4 input Gilad Ben-Yossef
2017-01-16 11:17 ` [PATCH ipsec-next v3 2/2] IPsec: do not ignore crypto err in ah6 input Gilad Ben-Yossef
2017-01-16 13:27 ` [PATCH ipsec-next v3 0/2] IPsec: do not ignore crypto err in ah input Steffen Klassert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).