diff mbox series

[v2] net/sctp: Make sha1 as default algorithm if fips is enabled

Message ID 1679493880-26421-1-git-send-email-kashwindayan@vmware.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v2] net/sctp: Make sha1 as default algorithm if fips is enabled | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers warning 1 maintainers not CCed: lucien.xin@gmail.com
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes fail Problems with Fixes tag: 1
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 22 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ashwin Dayanand Kamat March 22, 2023, 2:04 p.m. UTC
MD5 is not FIPS compliant. But still md5 was used as the default
algorithm for sctp if fips was enabled.
Due to this, listen() system call in ltp tests was failing for sctp
in fips environment, with below error message.

[ 6397.892677] sctp: failed to load transform for md5: -2

Fix is to not assign md5 as default algorithm for sctp
if fips_enabled is true. Instead make sha1 as default algorithm.

Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
---
v2:
the listener can still fail if fips mode is enabled after
that the netns is initialized. So taking action in sctp_listen_start()
and buming a ratelimited notice the selected hmac is changed due to fips.
---
 net/sctp/socket.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Simon Horman March 22, 2023, 8:46 p.m. UTC | #1
On Wed, Mar 22, 2023 at 07:34:40PM +0530, Ashwin Dayanand Kamat wrote:
> MD5 is not FIPS compliant. But still md5 was used as the default
> algorithm for sctp if fips was enabled.
> Due to this, listen() system call in ltp tests was failing for sctp
> in fips environment, with below error message.
> 
> [ 6397.892677] sctp: failed to load transform for md5: -2
> 
> Fix is to not assign md5 as default algorithm for sctp
> if fips_enabled is true. Instead make sha1 as default algorithm.
> 
> Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
> Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
> ---
> v2:
> the listener can still fail if fips mode is enabled after
> that the netns is initialized. So taking action in sctp_listen_start()
> and buming a ratelimited notice the selected hmac is changed due to fips.
> ---
>  net/sctp/socket.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index b91616f819de..a1107f42869e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -49,6 +49,7 @@
>  #include <linux/poll.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/fips.h>
>  #include <linux/file.h>
>  #include <linux/compat.h>
>  #include <linux/rhashtable.h>
> @@ -8496,6 +8497,15 @@ static int sctp_listen_start(struct sock *sk, int backlog)
>  	struct crypto_shash *tfm = NULL;
>  	char alg[32];
>  
> +	if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
> +#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))

I'm probably misunderstanding things, but would
IS_ENABLED(CONFIG_SCTP_COOKIE_HMAC_SHA1)
be more appropriate here?

> +		sp->sctp_hmac_alg = "sha1";
> +#else
> +		sp->sctp_hmac_alg = NULL;
> +#endif
> +		net_info_ratelimited("changing the hmac algorithm, as md5 is not supported when fips is enabled");
> +	}
> +
>  	/* Allocate HMAC for generating cookie. */
>  	if (!sp->hmac && sp->sctp_hmac_alg) {
>  		sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);
> -- 
> 2.39.0
>
Ashwin Dayanand Kamat March 25, 2023, 6:33 a.m. UTC | #2
> On 23-Mar-2023, at 2:16 AM, Simon Horman <simon.horman@corigine.com> wrote:
> 
> !! External Email
> 
> On Wed, Mar 22, 2023 at 07:34:40PM +0530, Ashwin Dayanand Kamat wrote:
>> MD5 is not FIPS compliant. But still md5 was used as the default
>> algorithm for sctp if fips was enabled.
>> Due to this, listen() system call in ltp tests was failing for sctp
>> in fips environment, with below error message.
>> 
>> [ 6397.892677] sctp: failed to load transform for md5: -2
>> 
>> Fix is to not assign md5 as default algorithm for sctp
>> if fips_enabled is true. Instead make sha1 as default algorithm.
>> 
>> Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
>> Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
>> ---
>> v2:
>> the listener can still fail if fips mode is enabled after
>> that the netns is initialized. So taking action in sctp_listen_start()
>> and buming a ratelimited notice the selected hmac is changed due to fips.
>> ---
>> net/sctp/socket.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>> index b91616f819de..a1107f42869e 100644
>> --- a/net/sctp/socket.c
>> +++ b/net/sctp/socket.c
>> @@ -49,6 +49,7 @@
>> #include <linux/poll.h>
>> #include <linux/init.h>
>> #include <linux/slab.h>
>> +#include <linux/fips.h>
>> #include <linux/file.h>
>> #include <linux/compat.h>
>> #include <linux/rhashtable.h>
>> @@ -8496,6 +8497,15 @@ static int sctp_listen_start(struct sock *sk, int backlog)
>> struct crypto_shash *tfm = NULL;
>> char alg[32];
>> 
>> + if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
>> +#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))
> 
> I'm probably misunderstanding things, but would
> IS_ENABLED(CONFIG_SCTP_COOKIE_HMAC_SHA1)
> be more appropriate here?
> 

Hi Simon,
I have moved the same check from sctp_init() to here based on the review for v1 patch.
Please let me know if there is any alternative which can be used?

Thanks,
Ashwin Kamat

>> + sp->sctp_hmac_alg = "sha1";
>> +#else
>> + sp->sctp_hmac_alg = NULL;
>> +#endif
>> + net_info_ratelimited("changing the hmac algorithm, as md5 is not supported when fips is enabled");
>> + }
>> +
>> /* Allocate HMAC for generating cookie. */
>> if (!sp->hmac && sp->sctp_hmac_alg) {
>> sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);
>> --
>> 2.39.0
>> 
> 
> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
Ashwin Dayanand Kamat May 27, 2023, 7:49 a.m. UTC | #3
> On 25-Mar-2023, at 12:03 PM, Ashwin Dayanand Kamat <kashwindayan@vmware.com> wrote:
> 
> 
>> On 23-Mar-2023, at 2:16 AM, Simon Horman <simon.horman@corigine.com> wrote:
>> 
>> !! External Email
>> 
>> On Wed, Mar 22, 2023 at 07:34:40PM +0530, Ashwin Dayanand Kamat wrote:
>>> MD5 is not FIPS compliant. But still md5 was used as the default
>>> algorithm for sctp if fips was enabled.
>>> Due to this, listen() system call in ltp tests was failing for sctp
>>> in fips environment, with below error message.
>>> 
>>> [ 6397.892677] sctp: failed to load transform for md5: -2
>>> 
>>> Fix is to not assign md5 as default algorithm for sctp
>>> if fips_enabled is true. Instead make sha1 as default algorithm.
>>> 
>>> Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
>>> Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
>>> ---
>>> v2:
>>> the listener can still fail if fips mode is enabled after
>>> that the netns is initialized. So taking action in sctp_listen_start()
>>> and buming a ratelimited notice the selected hmac is changed due to fips.
>>> ---
>>> net/sctp/socket.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>> 
>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>> index b91616f819de..a1107f42869e 100644
>>> --- a/net/sctp/socket.c
>>> +++ b/net/sctp/socket.c
>>> @@ -49,6 +49,7 @@
>>> #include <linux/poll.h>
>>> #include <linux/init.h>
>>> #include <linux/slab.h>
>>> +#include <linux/fips.h>
>>> #include <linux/file.h>
>>> #include <linux/compat.h>
>>> #include <linux/rhashtable.h>
>>> @@ -8496,6 +8497,15 @@ static int sctp_listen_start(struct sock *sk, int backlog)
>>> struct crypto_shash *tfm = NULL;
>>> char alg[32];
>>> 
>>> + if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
>>> +#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))
>> 
>> I'm probably misunderstanding things, but would
>> IS_ENABLED(CONFIG_SCTP_COOKIE_HMAC_SHA1)
>> be more appropriate here?
>> 
> 
> Hi Simon,
> I have moved the same check from sctp_init() to here based on the review for v1 patch.
> Please let me know if there is any alternative which can be used?
> 
> Thanks,
> Ashwin Kamat
> 
Hi Team,
Any update on this?

Thanks,
Ashwin Kamat
>>> + sp->sctp_hmac_alg = "sha1";
>>> +#else
>>> + sp->sctp_hmac_alg = NULL;
>>> +#endif
>>> + net_info_ratelimited("changing the hmac algorithm, as md5 is not supported when fips is enabled");
>>> + }
>>> +
>>> /* Allocate HMAC for generating cookie. */
>>> if (!sp->hmac && sp->sctp_hmac_alg) {
>>> sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);
>>> --
>>> 2.39.0
>>> 
>> 
>> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
> 
> 
> 
> 
>
Simon Horman May 27, 2023, 2:13 p.m. UTC | #4
On Sat, May 27, 2023 at 07:49:26AM +0000, Ashwin Dayanand Kamat wrote:
> 
> 
> > On 25-Mar-2023, at 12:03 PM, Ashwin Dayanand Kamat <kashwindayan@vmware.com> wrote:
> > 
> > 
> >> On 23-Mar-2023, at 2:16 AM, Simon Horman <simon.horman@corigine.com> wrote:
> >> 
> >> !! External Email
> >> 
> >> On Wed, Mar 22, 2023 at 07:34:40PM +0530, Ashwin Dayanand Kamat wrote:
> >>> MD5 is not FIPS compliant. But still md5 was used as the default
> >>> algorithm for sctp if fips was enabled.
> >>> Due to this, listen() system call in ltp tests was failing for sctp
> >>> in fips environment, with below error message.
> >>> 
> >>> [ 6397.892677] sctp: failed to load transform for md5: -2
> >>> 
> >>> Fix is to not assign md5 as default algorithm for sctp
> >>> if fips_enabled is true. Instead make sha1 as default algorithm.
> >>> 
> >>> Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
> >>> Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
> >>> ---
> >>> v2:
> >>> the listener can still fail if fips mode is enabled after
> >>> that the netns is initialized. So taking action in sctp_listen_start()
> >>> and buming a ratelimited notice the selected hmac is changed due to fips.
> >>> ---
> >>> net/sctp/socket.c | 10 ++++++++++
> >>> 1 file changed, 10 insertions(+)
> >>> 
> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >>> index b91616f819de..a1107f42869e 100644
> >>> --- a/net/sctp/socket.c
> >>> +++ b/net/sctp/socket.c
> >>> @@ -49,6 +49,7 @@
> >>> #include <linux/poll.h>
> >>> #include <linux/init.h>
> >>> #include <linux/slab.h>
> >>> +#include <linux/fips.h>
> >>> #include <linux/file.h>
> >>> #include <linux/compat.h>
> >>> #include <linux/rhashtable.h>
> >>> @@ -8496,6 +8497,15 @@ static int sctp_listen_start(struct sock *sk, int backlog)
> >>> struct crypto_shash *tfm = NULL;
> >>> char alg[32];
> >>> 
> >>> + if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
> >>> +#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))
> >> 
> >> I'm probably misunderstanding things, but would
> >> IS_ENABLED(CONFIG_SCTP_COOKIE_HMAC_SHA1)
> >> be more appropriate here?
> >> 
> > 
> > Hi Simon,
> > I have moved the same check from sctp_init() to here based on the review for v1 patch.
> > Please let me know if there is any alternative which can be used?
> > 
> > Thanks,
> > Ashwin Kamat
> > 
> Hi Team,
> Any update on this?

Hi Ashwin,

I don't recall exactly what I was thinking 2 months ago.
But looking at this a second time it seems that I may have misread your
patch: I now have no objections to it in its original form.
Ashwin Dayanand Kamat June 1, 2023, 6:21 p.m. UTC | #5
> On 27-May-2023, at 7:43 PM, Simon Horman <simon.horman@corigine.com> wrote:
> 
> !! External Email
> 
> On Sat, May 27, 2023 at 07:49:26AM +0000, Ashwin Dayanand Kamat wrote:
>> 
>> 
>>> On 25-Mar-2023, at 12:03 PM, Ashwin Dayanand Kamat <kashwindayan@vmware.com> wrote:
>>> 
>>> 
>>>> On 23-Mar-2023, at 2:16 AM, Simon Horman <simon.horman@corigine.com> wrote:
>>>> 
>>>> !! External Email
>>>> 
>>>> On Wed, Mar 22, 2023 at 07:34:40PM +0530, Ashwin Dayanand Kamat wrote:
>>>>> MD5 is not FIPS compliant. But still md5 was used as the default
>>>>> algorithm for sctp if fips was enabled.
>>>>> Due to this, listen() system call in ltp tests was failing for sctp
>>>>> in fips environment, with below error message.
>>>>> 
>>>>> [ 6397.892677] sctp: failed to load transform for md5: -2
>>>>> 
>>>>> Fix is to not assign md5 as default algorithm for sctp
>>>>> if fips_enabled is true. Instead make sha1 as default algorithm.
>>>>> 
>>>>> Fixes: ltp testcase failure "cve-2018-5803 sctp_big_chunk"
>>>>> Signed-off-by: Ashwin Dayanand Kamat <kashwindayan@vmware.com>
>>>>> ---
>>>>> v2:
>>>>> the listener can still fail if fips mode is enabled after
>>>>> that the netns is initialized. So taking action in sctp_listen_start()
>>>>> and buming a ratelimited notice the selected hmac is changed due to fips.
>>>>> ---
>>>>> net/sctp/socket.c | 10 ++++++++++
>>>>> 1 file changed, 10 insertions(+)
>>>>> 
>>>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>>>> index b91616f819de..a1107f42869e 100644
>>>>> --- a/net/sctp/socket.c
>>>>> +++ b/net/sctp/socket.c
>>>>> @@ -49,6 +49,7 @@
>>>>> #include <linux/poll.h>
>>>>> #include <linux/init.h>
>>>>> #include <linux/slab.h>
>>>>> +#include <linux/fips.h>
>>>>> #include <linux/file.h>
>>>>> #include <linux/compat.h>
>>>>> #include <linux/rhashtable.h>
>>>>> @@ -8496,6 +8497,15 @@ static int sctp_listen_start(struct sock *sk, int backlog)
>>>>> struct crypto_shash *tfm = NULL;
>>>>> char alg[32];
>>>>> 
>>>>> + if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
>>>>> +#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))
>>>> 
>>>> I'm probably misunderstanding things, but would
>>>> IS_ENABLED(CONFIG_SCTP_COOKIE_HMAC_SHA1)
>>>> be more appropriate here?
>>>> 
>>> 
>>> Hi Simon,
>>> I have moved the same check from sctp_init() to here based on the review for v1 patch.
>>> Please let me know if there is any alternative which can be used?
>>> 
>>> Thanks,
>>> Ashwin Kamat
>>> 
>> Hi Team,
>> Any update on this?
> 
> Hi Ashwin,
> 
> I don't recall exactly what I was thinking 2 months ago.
> But looking at this a second time it seems that I may have misread your
> patch: I now have no objections to it in its original form.
Thanks Simon.
I have Updated the v3 patch with some minor changes.  Please review the same.

> 
> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
diff mbox series

Patch

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b91616f819de..a1107f42869e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -49,6 +49,7 @@ 
 #include <linux/poll.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/fips.h>
 #include <linux/file.h>
 #include <linux/compat.h>
 #include <linux/rhashtable.h>
@@ -8496,6 +8497,15 @@  static int sctp_listen_start(struct sock *sk, int backlog)
 	struct crypto_shash *tfm = NULL;
 	char alg[32];
 
+	if (fips_enabled && !strcmp(sp->sctp_hmac_alg, "md5")) {
+#if (IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1))
+		sp->sctp_hmac_alg = "sha1";
+#else
+		sp->sctp_hmac_alg = NULL;
+#endif
+		net_info_ratelimited("changing the hmac algorithm, as md5 is not supported when fips is enabled");
+	}
+
 	/* Allocate HMAC for generating cookie. */
 	if (!sp->hmac && sp->sctp_hmac_alg) {
 		sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);