[dpdk-dev] [PATCH 0/3] examples/fips_validation: misc fixes

[dpdk-dev] [PATCH 0/3] examples/fips_validation: misc fixes

[Olivier Matz]

Seen while trying to test this example: the first two patches
are fixes (buffer overflow and an incorrect parsing). The last
one makes it possible to only run the self test, without passing
a req file.

Olivier Matz (3):
  examples/fips_validation: fix buffer overflow
  examples/fips_validation: ignore \r in input files
  examples/fips_validation: support self-test only

 examples/fips_validation/fips_validation.c | 14 ++++++++++++--
 examples/fips_validation/fips_validation.h |  3 ++-
 examples/fips_validation/main.c            | 22 ++++++++++++++++++++--
 3 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH 1/3] examples/fips_validation: fix buffer overflow

[Olivier Matz]

If the file name is larger than MAX_STRING_SIZE (64), strcpy()
will overwrite the content of memory.

Replace strcpy() by rte_strscpy(), check its return value, and
increase file_name size to 256.

Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
Cc: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/fips_validation/fips_validation.c | 12 ++++++++++--
 examples/fips_validation/fips_validation.h |  3 ++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/examples/fips_validation/fips_validation.c b/examples/fips_validation/fips_validation.c
index 9bdf257b8b..13f763c9aa 100644
--- a/examples/fips_validation/fips_validation.c
+++ b/examples/fips_validation/fips_validation.c
@@ -281,7 +281,11 @@ fips_test_init(const char *req_file_path, const char *rsp_file_path,
 
 	fips_test_clear();
 
-	strcpy(info.file_name, req_file_path);
+	if (rte_strscpy(info.file_name, req_file_path,
+				sizeof(info.file_name)) < 0) {
+		RTE_LOG(ERR, USER1, "Path %s too long\n", req_file_path);
+		return -EINVAL;
+	}
 	info.algo = FIPS_TEST_ALGO_MAX;
 	if (parse_file_type(req_file_path) < 0) {
 		RTE_LOG(ERR, USER1, "File %s type not supported\n",
@@ -307,7 +311,11 @@ fips_test_init(const char *req_file_path, const char *rsp_file_path,
 		return -ENOMEM;
 	}
 
-	strlcpy(info.device_name, device_name, sizeof(info.device_name));
+	if (rte_strscpy(info.device_name, device_name,
+				sizeof(info.device_name)) < 0) {
+		RTE_LOG(ERR, USER1, "Device name %s too long\n", device_name);
+		return -EINVAL;
+	}
 
 	if (fips_test_parse_header() < 0) {
 		RTE_LOG(ERR, USER1, "Failed parsing header\n");
diff --git a/examples/fips_validation/fips_validation.h b/examples/fips_validation/fips_validation.h
index 75fa555fa6..deba83eada 100644
--- a/examples/fips_validation/fips_validation.h
+++ b/examples/fips_validation/fips_validation.h
@@ -14,6 +14,7 @@
 #define MAX_NB_TESTS		10240
 #define MAX_BUF_SIZE		2048
 #define MAX_STRING_SIZE		64
+#define MAX_FILE_NAME_SIZE	256
 #define MAX_DIGEST_SIZE		64
 
 #define POSITIVE_TEST		0
@@ -164,7 +165,7 @@ struct fips_test_interim_info {
 	uint32_t vec_start_off;
 	uint32_t nb_vec_lines;
 	char device_name[MAX_STRING_SIZE];
-	char file_name[MAX_STRING_SIZE];
+	char file_name[MAX_FILE_NAME_SIZE];
 
 	union {
 		struct aesavs_interim_data aes_data;
-- 
2.25.1

[dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Olivier Matz]

Some test vectors contain '\r' before '\n' (see link). Ignore them.

Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-10.tar.gz
Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
Cc: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/fips_validation/fips_validation.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/examples/fips_validation/fips_validation.c b/examples/fips_validation/fips_validation.c
index 13f763c9aa..858f581ba3 100644
--- a/examples/fips_validation/fips_validation.c
+++ b/examples/fips_validation/fips_validation.c
@@ -33,6 +33,8 @@ get_file_line(void)
 
 		if (loc >= MAX_LINE_CHAR - 1)
 			return -ENOMEM;
+		if (c == '\r')
+			continue;
 		if (c == '\n')
 			break;
 		line[loc++] = c;
-- 
2.25.1

[dpdk-dev] [PATCH 3/3] examples/fips_validation: support self-test only

[Olivier Matz]

Make it possible to pass the self-tests when no req path is set.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/fips_validation/main.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
index 0a1c8b568c..ee3a890e9e 100644
--- a/examples/fips_validation/main.c
+++ b/examples/fips_validation/main.c
@@ -315,8 +315,21 @@ cryptodev_fips_validate_parse_args(int argc, char **argv)
 		}
 	}
 
-	if (env.req_path == NULL || env.rsp_path == NULL ||
-			env.dev_id == UINT32_MAX) {
+	if (env.dev_id == UINT32_MAX) {
+		RTE_LOG(ERR, USER1, "No device specified\n");
+		cryptodev_fips_validate_usage(prgname);
+		return -EINVAL;
+	}
+
+	if ((env.req_path == NULL && env.rsp_path != NULL) ||
+			(env.req_path != NULL && env.rsp_path == NULL)) {
+		RTE_LOG(ERR, USER1, "Missing req path or rsp path\n");
+		cryptodev_fips_validate_usage(prgname);
+		return -EINVAL;
+	}
+
+	if (env.req_path == NULL && env.self_test == 0) {
+		RTE_LOG(ERR, USER1, "--self-test must be set if req path is missing\n");
 		cryptodev_fips_validate_usage(prgname);
 		return -EINVAL;
 	}
@@ -348,6 +361,11 @@ main(int argc, char *argv[])
 		return -1;
 	}
 
+	if (env.req_path == NULL || env.rsp_path == NULL) {
+		printf("No request, exit.\n");
+		goto exit;
+	}
+
 	if (!env.is_path_folder) {
 		printf("Processing file %s... ", env.req_path);
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Zhang, Roy Fan]

Hi Olivier,

The patch looks ok but the test file link you provided in the patch is CAVS 5.3. 
As mentioned in https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the supported CAVS supported version is 21.0 (not latest one by newer than 5.3). In CAVS 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows right).

Regards,
Fan

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Tuesday, October 6, 2020 8:42 AM
> To: dev@dpdk.org
> Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> 
> Some test vectors contain '\r' before '\n' (see link). Ignore them.
> 
> Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-10.tar.gz
> Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  examples/fips_validation/fips_validation.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/examples/fips_validation/fips_validation.c
> b/examples/fips_validation/fips_validation.c
> index 13f763c9aa..858f581ba3 100644
> --- a/examples/fips_validation/fips_validation.c
> +++ b/examples/fips_validation/fips_validation.c
> @@ -33,6 +33,8 @@ get_file_line(void)
> 
>  		if (loc >= MAX_LINE_CHAR - 1)
>  			return -ENOMEM;
> +		if (c == '\r')
> +			continue;
>  		if (c == '\n')
>  			break;
>  		line[loc++] = c;
> --
> 2.25.1

Re: [dpdk-dev] [PATCH 1/3] examples/fips_validation: fix buffer overflow

[Zhang, Roy Fan]

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Tuesday, October 6, 2020 8:42 AM
> To: dev@dpdk.org
> Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] examples/fips_validation: fix buffer overflow
> 
> If the file name is larger than MAX_STRING_SIZE (64), strcpy()
> will overwrite the content of memory.
> 
> Replace strcpy() by rte_strscpy(), check its return value, and
> increase file_name size to 256.
> 
> Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

Re: [dpdk-dev] [PATCH 3/3] examples/fips_validation: support self-test only

[Zhang, Roy Fan]

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Tuesday, October 6, 2020 8:42 AM
> To: dev@dpdk.org
> Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH 3/3] examples/fips_validation: support self-test only
> 
> Make it possible to pass the self-tests when no req path is set.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Olivier Matz]

Hi Fan,

On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> Hi Olivier,
>
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Tuesday, October 6, 2020 8:42 AM
> > To: dev@dpdk.org
> > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > 
> > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > 
> > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-10.tar.gz
> > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >  examples/fips_validation/fips_validation.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/examples/fips_validation/fips_validation.c
> > b/examples/fips_validation/fips_validation.c
> > index 13f763c9aa..858f581ba3 100644
> > --- a/examples/fips_validation/fips_validation.c
> > +++ b/examples/fips_validation/fips_validation.c
> > @@ -33,6 +33,8 @@ get_file_line(void)
> > 
> >  		if (loc >= MAX_LINE_CHAR - 1)
> >  			return -ENOMEM;
> > +		if (c == '\r')
> > +			continue;
> >  		if (c == '\n')
> >  			break;
> >  		line[loc++] = c;
> > --
> 
>
> The patch looks ok but the test file link you provided in the patch is CAVS
> 5.3.
>
> As mentioned in
> https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the supported
> CAVS supported version is 21.0 (not latest one by newer than 5.3). In CAVS
> 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows
> right).

Thank you for your feedback.

I'm ok to drop this patch from the patchset if you feel it's useless, or
I can update the commit log with the information you provide, to clarify
that it should not happen with the supported version of CAVS.

Please let me know what you prefer.


Thanks,
Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Zhang, Roy Fan]

Hi Olivier,

Anood and us had the similar discussion.

Can we change the sample application to parse version data instead,
and for the version specific code changes we will wrap them by a
branch to compare the parsed version and the expected version? 
(we probably should have done that long time ago).

I drafted a code change to parse the version data, see if you think it
is ok?

diff --git a/examples/fips_validation/fips_validation.c b/examples/fips_validation/fips_validation.c
index 9bdf257b8..9b6518c92 100644
--- a/examples/fips_validation/fips_validation.c
+++ b/examples/fips_validation/fips_validation.c
@@ -98,7 +98,7 @@ fips_test_parse_header(void)
 	uint32_t i;
 	char *tmp;
 	int ret;
-	int algo_parsed = 0;
+	int algo_parsed = 0, version_parsed = 0;
 	time_t t = time(NULL);
 	struct tm *tm_now = localtime(&t);
 
@@ -107,6 +107,27 @@ fips_test_parse_header(void)
 		return ret;
 
 	for (i = 0; i < info.nb_vec_lines; i++) {
+		/* parse the version info */
+		tmp = strstr(info.vec[i], "CAVS ");
+		if (tmp != NULL) {
+			if (version_parsed != 0) {
+				RTE_LOG(ERR, USER1,
+					"Multiple version data\n");
+				return -1;
+			}
+
+			tmp = tmp + sizeof("CAVS ");
+
+			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
+				RTE_LOG(ERR, USER1, "Version (%s) too long\n",
+						tmp);
+				return -1;
+			}
+
+			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
+			version_parsed = 1;
+		}
+
 		if (!algo_parsed) {
 			if (strstr(info.vec[i], "AESVS")) {
 				algo_parsed = 1;
diff --git a/examples/fips_validation/fips_validation.h b/examples/fips_validation/fips_validation.h
index 75fa555fa..b8c60c55f 100644
--- a/examples/fips_validation/fips_validation.h
+++ b/examples/fips_validation/fips_validation.h
@@ -15,6 +15,9 @@
 #define MAX_BUF_SIZE		2048
 #define MAX_STRING_SIZE		64
 #define MAX_DIGEST_SIZE		64
+#define MAX_VER_STRING_SIZE	8
+
+#define FIPS_DEF_VERSION	"21.0"
 
 #define POSITIVE_TEST		0
 #define NEGATIVE_TEST		-1
@@ -155,6 +158,7 @@ struct sha_interim_data {
 };
 
 struct fips_test_interim_info {
+	char version[MAX_VER_STRING_SIZE];
 	FILE *fp_rd;
 	FILE *fp_wr;
 	enum file_types file_type;


Regards,
Fan

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Tuesday, October 6, 2020 11:09 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> <anoobj@marvell.com>
> Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> 
> Hi Fan,
> 
> On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > To: dev@dpdk.org
> > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> Kusztal,
> > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > >
> > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > >
> > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-
> 10.tar.gz
> > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> application")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > >  examples/fips_validation/fips_validation.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/examples/fips_validation/fips_validation.c
> > > b/examples/fips_validation/fips_validation.c
> > > index 13f763c9aa..858f581ba3 100644
> > > --- a/examples/fips_validation/fips_validation.c
> > > +++ b/examples/fips_validation/fips_validation.c
> > > @@ -33,6 +33,8 @@ get_file_line(void)
> > >
> > >  		if (loc >= MAX_LINE_CHAR - 1)
> > >  			return -ENOMEM;
> > > +		if (c == '\r')
> > > +			continue;
> > >  		if (c == '\n')
> > >  			break;
> > >  		line[loc++] = c;
> > > --
> >
> >
> > The patch looks ok but the test file link you provided in the patch is CAVS
> > 5.3.
> >
> > As mentioned in
> > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the
> supported
> > CAVS supported version is 21.0 (not latest one by newer than 5.3). In CAVS
> > 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows
> > right).
> 
> Thank you for your feedback.
> 
> I'm ok to drop this patch from the patchset if you feel it's useless, or
> I can update the commit log with the information you provide, to clarify
> that it should not happen with the supported version of CAVS.
> 
> Please let me know what you prefer.
> 
> 
> Thanks,
> Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Olivier Matz]

Hi,

On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> Hi Olivier,
> 
> Anood and us had the similar discussion.
> 
> Can we change the sample application to parse version data instead,
> and for the version specific code changes we will wrap them by a
> branch to compare the parsed version and the expected version? 
> (we probably should have done that long time ago).
> 
> I drafted a code change to parse the version data, see if you think it
> is ok?

Thank you for your feedback.

The code that gets the version looks good to me (I just have a
small comment, see below). However I'm not sure what to do with it.

Do you mean we should return an error if the version is incorrect? Or
should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used
in your patch. In that case, I think it is a bit overkill. Do you think
it is a problem to always drop '\r'?

If you think we should not support files containing '\r', I'm fine
with it, I can drop this particular patch.


> 
> diff --git a/examples/fips_validation/fips_validation.c b/examples/fips_validation/fips_validation.c
> index 9bdf257b8..9b6518c92 100644
> --- a/examples/fips_validation/fips_validation.c
> +++ b/examples/fips_validation/fips_validation.c
> @@ -98,7 +98,7 @@ fips_test_parse_header(void)
>  	uint32_t i;
>  	char *tmp;
>  	int ret;
> -	int algo_parsed = 0;
> +	int algo_parsed = 0, version_parsed = 0;
>  	time_t t = time(NULL);
>  	struct tm *tm_now = localtime(&t);
>  
> @@ -107,6 +107,27 @@ fips_test_parse_header(void)
>  		return ret;
>  
>  	for (i = 0; i < info.nb_vec_lines; i++) {
> +		/* parse the version info */
> +		tmp = strstr(info.vec[i], "CAVS ");
> +		if (tmp != NULL) {
> +			if (version_parsed != 0) {
> +				RTE_LOG(ERR, USER1,
> +					"Multiple version data\n");
> +				return -1;
> +			}
> +
> +			tmp = tmp + sizeof("CAVS ");

I think it should be strlen(), because sizeof() will contain
the '\0'. Or it could be sizeof() - 1.

> +
> +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> +				RTE_LOG(ERR, USER1, "Version (%s) too long\n",
> +						tmp);
> +				return -1;
> +			}
> +
> +			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
> +			version_parsed = 1;
> +		}
> +
>  		if (!algo_parsed) {
>  			if (strstr(info.vec[i], "AESVS")) {
>  				algo_parsed = 1;
> diff --git a/examples/fips_validation/fips_validation.h b/examples/fips_validation/fips_validation.h
> index 75fa555fa..b8c60c55f 100644
> --- a/examples/fips_validation/fips_validation.h
> +++ b/examples/fips_validation/fips_validation.h
> @@ -15,6 +15,9 @@
>  #define MAX_BUF_SIZE		2048
>  #define MAX_STRING_SIZE		64
>  #define MAX_DIGEST_SIZE		64
> +#define MAX_VER_STRING_SIZE	8
> +
> +#define FIPS_DEF_VERSION	"21.0"
>  
>  #define POSITIVE_TEST		0
>  #define NEGATIVE_TEST		-1
> @@ -155,6 +158,7 @@ struct sha_interim_data {
>  };
>  
>  struct fips_test_interim_info {
> +	char version[MAX_VER_STRING_SIZE];
>  	FILE *fp_rd;
>  	FILE *fp_wr;
>  	enum file_types file_type;
> 
> 
> Regards,
> Fan
> 
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Tuesday, October 6, 2020 11:09 AM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > <anoobj@marvell.com>
> > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > 
> > Hi Fan,
> > 
> > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > Hi Olivier,
> > >
> > > > -----Original Message-----
> > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > To: dev@dpdk.org
> > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> > Kusztal,
> > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > > >
> > > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > > >
> > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-
> > 10.tar.gz
> > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> > application")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > b/examples/fips_validation/fips_validation.c
> > > > index 13f763c9aa..858f581ba3 100644
> > > > --- a/examples/fips_validation/fips_validation.c
> > > > +++ b/examples/fips_validation/fips_validation.c
> > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > >
> > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > >  			return -ENOMEM;
> > > > +		if (c == '\r')
> > > > +			continue;
> > > >  		if (c == '\n')
> > > >  			break;
> > > >  		line[loc++] = c;
> > > > --
> > >
> > >
> > > The patch looks ok but the test file link you provided in the patch is CAVS
> > > 5.3.
> > >
> > > As mentioned in
> > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the
> > supported
> > > CAVS supported version is 21.0 (not latest one by newer than 5.3). In CAVS
> > > 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows
> > > right).
> > 
> > Thank you for your feedback.
> > 
> > I'm ok to drop this patch from the patchset if you feel it's useless, or
> > I can update the commit log with the information you provide, to clarify
> > that it should not happen with the supported version of CAVS.
> > 
> > Please let me know what you prefer.
> > 
> > 
> > Thanks,
> > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Zhang, Roy Fan]

Hi Olivier,

Sorry I didn't state myself clear in the first place.

My intention is '\r' check, or any future CAVS version specific change to the
application should be wrapped into a branch that is checked with parsed
version number. With this way the original application's behavior should
remain the same.

The reason for that is we are having an issue right now that the validation
team is struggling with the limited test vectors and inconsistency formatting
between different FIPS CAVS versions. For example we still have FIPS TDES test
failing today due to the different test file versions.
https://bugs.dpdk.org/show_bug.cgi?id=512 

The solution is certainly far from pretty but should help to share the
maintenance effort amongst the contributors.

The "FIPS_DEF_VERSION" can be removed of course.

Regards,
Fan

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, October 8, 2020 10:22 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> <anoobj@marvell.com>
> Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> 
> Hi,
> 
> On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> > Hi Olivier,
> >
> > Anood and us had the similar discussion.
> >
> > Can we change the sample application to parse version data instead,
> > and for the version specific code changes we will wrap them by a
> > branch to compare the parsed version and the expected version?
> > (we probably should have done that long time ago).
> >
> > I drafted a code change to parse the version data, see if you think it
> > is ok?
> 
> Thank you for your feedback.
> 
> The code that gets the version looks good to me (I just have a
> small comment, see below). However I'm not sure what to do with it.
> 
> Do you mean we should return an error if the version is incorrect? Or
> should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used
> in your patch. In that case, I think it is a bit overkill. Do you think
> it is a problem to always drop '\r'?
> 
> If you think we should not support files containing '\r', I'm fine
> with it, I can drop this particular patch.
> 
> 
> >
> > diff --git a/examples/fips_validation/fips_validation.c
> b/examples/fips_validation/fips_validation.c
> > index 9bdf257b8..9b6518c92 100644
> > --- a/examples/fips_validation/fips_validation.c
> > +++ b/examples/fips_validation/fips_validation.c
> > @@ -98,7 +98,7 @@ fips_test_parse_header(void)
> >  	uint32_t i;
> >  	char *tmp;
> >  	int ret;
> > -	int algo_parsed = 0;
> > +	int algo_parsed = 0, version_parsed = 0;
> >  	time_t t = time(NULL);
> >  	struct tm *tm_now = localtime(&t);
> >
> > @@ -107,6 +107,27 @@ fips_test_parse_header(void)
> >  		return ret;
> >
> >  	for (i = 0; i < info.nb_vec_lines; i++) {
> > +		/* parse the version info */
> > +		tmp = strstr(info.vec[i], "CAVS ");
> > +		if (tmp != NULL) {
> > +			if (version_parsed != 0) {
> > +				RTE_LOG(ERR, USER1,
> > +					"Multiple version data\n");
> > +				return -1;
> > +			}
> > +
> > +			tmp = tmp + sizeof("CAVS ");
> 
> I think it should be strlen(), because sizeof() will contain
> the '\0'. Or it could be sizeof() - 1.
> 
> > +
> > +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> > +				RTE_LOG(ERR, USER1, "Version (%s) too
> long\n",
> > +						tmp);
> > +				return -1;
> > +			}
> > +
> > +			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
> > +			version_parsed = 1;
> > +		}
> > +
> >  		if (!algo_parsed) {
> >  			if (strstr(info.vec[i], "AESVS")) {
> >  				algo_parsed = 1;
> > diff --git a/examples/fips_validation/fips_validation.h
> b/examples/fips_validation/fips_validation.h
> > index 75fa555fa..b8c60c55f 100644
> > --- a/examples/fips_validation/fips_validation.h
> > +++ b/examples/fips_validation/fips_validation.h
> > @@ -15,6 +15,9 @@
> >  #define MAX_BUF_SIZE		2048
> >  #define MAX_STRING_SIZE		64
> >  #define MAX_DIGEST_SIZE		64
> > +#define MAX_VER_STRING_SIZE	8
> > +
> > +#define FIPS_DEF_VERSION	"21.0"
> >
> >  #define POSITIVE_TEST		0
> >  #define NEGATIVE_TEST		-1
> > @@ -155,6 +158,7 @@ struct sha_interim_data {
> >  };
> >
> >  struct fips_test_interim_info {
> > +	char version[MAX_VER_STRING_SIZE];
> >  	FILE *fp_rd;
> >  	FILE *fp_wr;
> >  	enum file_types file_type;
> >
> >
> > Regards,
> > Fan
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Tuesday, October 6, 2020 11:09 AM
> > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> Akhil
> > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > <anoobj@marvell.com>
> > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > >
> > > Hi Fan,
> > >
> > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > > Hi Olivier,
> > > >
> > > > > -----Original Message-----
> > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> > > Kusztal,
> > > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > > > >
> > > > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > > > >
> > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-
> > > 10.tar.gz
> > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> > > application")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > ---
> > > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > b/examples/fips_validation/fips_validation.c
> > > > > index 13f763c9aa..858f581ba3 100644
> > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > > >
> > > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > > >  			return -ENOMEM;
> > > > > +		if (c == '\r')
> > > > > +			continue;
> > > > >  		if (c == '\n')
> > > > >  			break;
> > > > >  		line[loc++] = c;
> > > > > --
> > > >
> > > >
> > > > The patch looks ok but the test file link you provided in the patch is
> CAVS
> > > > 5.3.
> > > >
> > > > As mentioned in
> > > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the
> > > supported
> > > > CAVS supported version is 21.0 (not latest one by newer than 5.3). In
> CAVS
> > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows
> > > > right).
> > >
> > > Thank you for your feedback.
> > >
> > > I'm ok to drop this patch from the patchset if you feel it's useless, or
> > > I can update the commit log with the information you provide, to clarify
> > > that it should not happen with the supported version of CAVS.
> > >
> > > Please let me know what you prefer.
> > >
> > >
> > > Thanks,
> > > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Olivier Matz]

Hi Fan,

Thank you for the clarification. One more question: do you know where I
can find a description of the different FIPS CAVS versions? I would like
to know from what version the \r has been removed.

Thanks,
Olivier

On Thu, Oct 08, 2020 at 10:24:48AM +0000, Zhang, Roy Fan wrote:
> Hi Olivier,
> 
> Sorry I didn't state myself clear in the first place.
> 
> My intention is '\r' check, or any future CAVS version specific change to the
> application should be wrapped into a branch that is checked with parsed
> version number. With this way the original application's behavior should
> remain the same.
> 
> The reason for that is we are having an issue right now that the validation
> team is struggling with the limited test vectors and inconsistency formatting
> between different FIPS CAVS versions. For example we still have FIPS TDES test
> failing today due to the different test file versions.
> https://bugs.dpdk.org/show_bug.cgi?id=512 
> 
> The solution is certainly far from pretty but should help to share the
> maintenance effort amongst the contributors.
> 
> The "FIPS_DEF_VERSION" can be removed of course.
> 
> Regards,
> Fan
> 
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, October 8, 2020 10:22 AM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > <anoobj@marvell.com>
> > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > 
> > Hi,
> > 
> > On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> > > Hi Olivier,
> > >
> > > Anood and us had the similar discussion.
> > >
> > > Can we change the sample application to parse version data instead,
> > > and for the version specific code changes we will wrap them by a
> > > branch to compare the parsed version and the expected version?
> > > (we probably should have done that long time ago).
> > >
> > > I drafted a code change to parse the version data, see if you think it
> > > is ok?
> > 
> > Thank you for your feedback.
> > 
> > The code that gets the version looks good to me (I just have a
> > small comment, see below). However I'm not sure what to do with it.
> > 
> > Do you mean we should return an error if the version is incorrect? Or
> > should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used
> > in your patch. In that case, I think it is a bit overkill. Do you think
> > it is a problem to always drop '\r'?
> > 
> > If you think we should not support files containing '\r', I'm fine
> > with it, I can drop this particular patch.
> > 
> > 
> > >
> > > diff --git a/examples/fips_validation/fips_validation.c
> > b/examples/fips_validation/fips_validation.c
> > > index 9bdf257b8..9b6518c92 100644
> > > --- a/examples/fips_validation/fips_validation.c
> > > +++ b/examples/fips_validation/fips_validation.c
> > > @@ -98,7 +98,7 @@ fips_test_parse_header(void)
> > >  	uint32_t i;
> > >  	char *tmp;
> > >  	int ret;
> > > -	int algo_parsed = 0;
> > > +	int algo_parsed = 0, version_parsed = 0;
> > >  	time_t t = time(NULL);
> > >  	struct tm *tm_now = localtime(&t);
> > >
> > > @@ -107,6 +107,27 @@ fips_test_parse_header(void)
> > >  		return ret;
> > >
> > >  	for (i = 0; i < info.nb_vec_lines; i++) {
> > > +		/* parse the version info */
> > > +		tmp = strstr(info.vec[i], "CAVS ");
> > > +		if (tmp != NULL) {
> > > +			if (version_parsed != 0) {
> > > +				RTE_LOG(ERR, USER1,
> > > +					"Multiple version data\n");
> > > +				return -1;
> > > +			}
> > > +
> > > +			tmp = tmp + sizeof("CAVS ");
> > 
> > I think it should be strlen(), because sizeof() will contain
> > the '\0'. Or it could be sizeof() - 1.
> > 
> > > +
> > > +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> > > +				RTE_LOG(ERR, USER1, "Version (%s) too
> > long\n",
> > > +						tmp);
> > > +				return -1;
> > > +			}
> > > +
> > > +			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
> > > +			version_parsed = 1;
> > > +		}
> > > +
> > >  		if (!algo_parsed) {
> > >  			if (strstr(info.vec[i], "AESVS")) {
> > >  				algo_parsed = 1;
> > > diff --git a/examples/fips_validation/fips_validation.h
> > b/examples/fips_validation/fips_validation.h
> > > index 75fa555fa..b8c60c55f 100644
> > > --- a/examples/fips_validation/fips_validation.h
> > > +++ b/examples/fips_validation/fips_validation.h
> > > @@ -15,6 +15,9 @@
> > >  #define MAX_BUF_SIZE		2048
> > >  #define MAX_STRING_SIZE		64
> > >  #define MAX_DIGEST_SIZE		64
> > > +#define MAX_VER_STRING_SIZE	8
> > > +
> > > +#define FIPS_DEF_VERSION	"21.0"
> > >
> > >  #define POSITIVE_TEST		0
> > >  #define NEGATIVE_TEST		-1
> > > @@ -155,6 +158,7 @@ struct sha_interim_data {
> > >  };
> > >
> > >  struct fips_test_interim_info {
> > > +	char version[MAX_VER_STRING_SIZE];
> > >  	FILE *fp_rd;
> > >  	FILE *fp_wr;
> > >  	enum file_types file_type;
> > >
> > >
> > > Regards,
> > > Fan
> > >
> > > > -----Original Message-----
> > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > Sent: Tuesday, October 6, 2020 11:09 AM
> > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> > Akhil
> > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > <anoobj@marvell.com>
> > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > > >
> > > > Hi Fan,
> > > >
> > > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > > > Hi Olivier,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > > > To: dev@dpdk.org
> > > > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> > > > Kusztal,
> > > > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > > > > >
> > > > > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > > > > >
> > > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10-
> > > > 10.tar.gz
> > > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> > > > application")
> > > > > > Cc: stable@dpdk.org
> > > > > >
> > > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > > ---
> > > > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > > b/examples/fips_validation/fips_validation.c
> > > > > > index 13f763c9aa..858f581ba3 100644
> > > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > > > >
> > > > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > > > >  			return -ENOMEM;
> > > > > > +		if (c == '\r')
> > > > > > +			continue;
> > > > > >  		if (c == '\n')
> > > > > >  			break;
> > > > > >  		line[loc++] = c;
> > > > > > --
> > > > >
> > > > >
> > > > > The patch looks ok but the test file link you provided in the patch is
> > CAVS
> > > > > 5.3.
> > > > >
> > > > > As mentioned in
> > > > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the
> > > > supported
> > > > > CAVS supported version is 21.0 (not latest one by newer than 5.3). In
> > CAVS
> > > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows
> > > > > right).
> > > >
> > > > Thank you for your feedback.
> > > >
> > > > I'm ok to drop this patch from the patchset if you feel it's useless, or
> > > > I can update the commit log with the information you provide, to clarify
> > > > that it should not happen with the supported version of CAVS.
> > > >
> > > > Please let me know what you prefer.
> > > >
> > > >
> > > > Thanks,
> > > > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Zhang, Roy Fan]

Hi Olivier,

Unfortunately I wanted to find the same document since forever. NIST
did not provide this on their website. What I am sure is for CAVS 21.0
both the test vectors Intel used for testing and the ones provided by
our customer for debugging did not have \r in the files. In 2018 we
could find some sample request and response files from NIST website
but I just checked and they are gone.

Regards,
Fan

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, October 8, 2020 12:32 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> <anoobj@marvell.com>
> Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> 
> Hi Fan,
> 
> Thank you for the clarification. One more question: do you know where I
> can find a description of the different FIPS CAVS versions? I would like
> to know from what version the \r has been removed.
> 
> Thanks,
> Olivier
> 
> On Thu, Oct 08, 2020 at 10:24:48AM +0000, Zhang, Roy Fan wrote:
> > Hi Olivier,
> >
> > Sorry I didn't state myself clear in the first place.
> >
> > My intention is '\r' check, or any future CAVS version specific change to the
> > application should be wrapped into a branch that is checked with parsed
> > version number. With this way the original application's behavior should
> > remain the same.
> >
> > The reason for that is we are having an issue right now that the validation
> > team is struggling with the limited test vectors and inconsistency formatting
> > between different FIPS CAVS versions. For example we still have FIPS TDES
> test
> > failing today due to the different test file versions.
> > https://bugs.dpdk.org/show_bug.cgi?id=512
> >
> > The solution is certainly far from pretty but should help to share the
> > maintenance effort amongst the contributors.
> >
> > The "FIPS_DEF_VERSION" can be removed of course.
> >
> > Regards,
> > Fan
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Thursday, October 8, 2020 10:22 AM
> > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> Akhil
> > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > <anoobj@marvell.com>
> > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > >
> > > Hi,
> > >
> > > On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> > > > Hi Olivier,
> > > >
> > > > Anood and us had the similar discussion.
> > > >
> > > > Can we change the sample application to parse version data instead,
> > > > and for the version specific code changes we will wrap them by a
> > > > branch to compare the parsed version and the expected version?
> > > > (we probably should have done that long time ago).
> > > >
> > > > I drafted a code change to parse the version data, see if you think it
> > > > is ok?
> > >
> > > Thank you for your feedback.
> > >
> > > The code that gets the version looks good to me (I just have a
> > > small comment, see below). However I'm not sure what to do with it.
> > >
> > > Do you mean we should return an error if the version is incorrect? Or
> > > should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used
> > > in your patch. In that case, I think it is a bit overkill. Do you think
> > > it is a problem to always drop '\r'?
> > >
> > > If you think we should not support files containing '\r', I'm fine
> > > with it, I can drop this particular patch.
> > >
> > >
> > > >
> > > > diff --git a/examples/fips_validation/fips_validation.c
> > > b/examples/fips_validation/fips_validation.c
> > > > index 9bdf257b8..9b6518c92 100644
> > > > --- a/examples/fips_validation/fips_validation.c
> > > > +++ b/examples/fips_validation/fips_validation.c
> > > > @@ -98,7 +98,7 @@ fips_test_parse_header(void)
> > > >  	uint32_t i;
> > > >  	char *tmp;
> > > >  	int ret;
> > > > -	int algo_parsed = 0;
> > > > +	int algo_parsed = 0, version_parsed = 0;
> > > >  	time_t t = time(NULL);
> > > >  	struct tm *tm_now = localtime(&t);
> > > >
> > > > @@ -107,6 +107,27 @@ fips_test_parse_header(void)
> > > >  		return ret;
> > > >
> > > >  	for (i = 0; i < info.nb_vec_lines; i++) {
> > > > +		/* parse the version info */
> > > > +		tmp = strstr(info.vec[i], "CAVS ");
> > > > +		if (tmp != NULL) {
> > > > +			if (version_parsed != 0) {
> > > > +				RTE_LOG(ERR, USER1,
> > > > +					"Multiple version data\n");
> > > > +				return -1;
> > > > +			}
> > > > +
> > > > +			tmp = tmp + sizeof("CAVS ");
> > >
> > > I think it should be strlen(), because sizeof() will contain
> > > the '\0'. Or it could be sizeof() - 1.
> > >
> > > > +
> > > > +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> > > > +				RTE_LOG(ERR, USER1, "Version (%s) too
> > > long\n",
> > > > +						tmp);
> > > > +				return -1;
> > > > +			}
> > > > +
> > > > +			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
> > > > +			version_parsed = 1;
> > > > +		}
> > > > +
> > > >  		if (!algo_parsed) {
> > > >  			if (strstr(info.vec[i], "AESVS")) {
> > > >  				algo_parsed = 1;
> > > > diff --git a/examples/fips_validation/fips_validation.h
> > > b/examples/fips_validation/fips_validation.h
> > > > index 75fa555fa..b8c60c55f 100644
> > > > --- a/examples/fips_validation/fips_validation.h
> > > > +++ b/examples/fips_validation/fips_validation.h
> > > > @@ -15,6 +15,9 @@
> > > >  #define MAX_BUF_SIZE		2048
> > > >  #define MAX_STRING_SIZE		64
> > > >  #define MAX_DIGEST_SIZE		64
> > > > +#define MAX_VER_STRING_SIZE	8
> > > > +
> > > > +#define FIPS_DEF_VERSION	"21.0"
> > > >
> > > >  #define POSITIVE_TEST		0
> > > >  #define NEGATIVE_TEST		-1
> > > > @@ -155,6 +158,7 @@ struct sha_interim_data {
> > > >  };
> > > >
> > > >  struct fips_test_interim_info {
> > > > +	char version[MAX_VER_STRING_SIZE];
> > > >  	FILE *fp_rd;
> > > >  	FILE *fp_wr;
> > > >  	enum file_types file_type;
> > > >
> > > >
> > > > Regards,
> > > > Fan
> > > >
> > > > > -----Original Message-----
> > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > Sent: Tuesday, October 6, 2020 11:09 AM
> > > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> > > Akhil
> > > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > > <anoobj@marvell.com>
> > > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input
> files
> > > > >
> > > > > Hi Fan,
> > > > >
> > > > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > > > > Hi Olivier,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > > > > To: dev@dpdk.org
> > > > > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > > > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>;
> > > > > Kusztal,
> > > > > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input
> files
> > > > > > >
> > > > > > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > > > > > >
> > > > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-
> 10-
> > > > > 10.tar.gz
> > > > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> > > > > application")
> > > > > > > Cc: stable@dpdk.org
> > > > > > >
> > > > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > ---
> > > > > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > > > > >  1 file changed, 2 insertions(+)
> > > > > > >
> > > > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > > > b/examples/fips_validation/fips_validation.c
> > > > > > > index 13f763c9aa..858f581ba3 100644
> > > > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > > > > >
> > > > > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > > > > >  			return -ENOMEM;
> > > > > > > +		if (c == '\r')
> > > > > > > +			continue;
> > > > > > >  		if (c == '\n')
> > > > > > >  			break;
> > > > > > >  		line[loc++] = c;
> > > > > > > --
> > > > > >
> > > > > >
> > > > > > The patch looks ok but the test file link you provided in the patch is
> > > CAVS
> > > > > > 5.3.
> > > > > >
> > > > > > As mentioned in
> > > > > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html,
> the
> > > > > supported
> > > > > > CAVS supported version is 21.0 (not latest one by newer than 5.3).
> In
> > > CAVS
> > > > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for
> Windows
> > > > > > right).
> > > > >
> > > > > Thank you for your feedback.
> > > > >
> > > > > I'm ok to drop this patch from the patchset if you feel it's useless, or
> > > > > I can update the commit log with the information you provide, to
> clarify
> > > > > that it should not happen with the supported version of CAVS.
> > > > >
> > > > > Please let me know what you prefer.
> > > > >
> > > > >
> > > > > Thanks,
> > > > > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Olivier Matz]

Hi Fan,

So if we cannot know which version removed the \r, I suggest to just
drop this patch. I thought it was a bug in the parser, but if it does
not happen with files matching the supported CAVS version, there is
nothing to fix.

What do you think?

Thanks,
Olivier


On Thu, Oct 08, 2020 at 12:41:11PM +0000, Zhang, Roy Fan wrote:
> Hi Olivier,
> 
> Unfortunately I wanted to find the same document since forever. NIST
> did not provide this on their website. What I am sure is for CAVS 21.0
> both the test vectors Intel used for testing and the ones provided by
> our customer for debugging did not have \r in the files. In 2018 we
> could find some sample request and response files from NIST website
> but I just checked and they are gone.
> 
> Regards,
> Fan
> 
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, October 8, 2020 12:32 PM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > <anoobj@marvell.com>
> > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > 
> > Hi Fan,
> > 
> > Thank you for the clarification. One more question: do you know where I
> > can find a description of the different FIPS CAVS versions? I would like
> > to know from what version the \r has been removed.
> > 
> > Thanks,
> > Olivier
> > 
> > On Thu, Oct 08, 2020 at 10:24:48AM +0000, Zhang, Roy Fan wrote:
> > > Hi Olivier,
> > >
> > > Sorry I didn't state myself clear in the first place.
> > >
> > > My intention is '\r' check, or any future CAVS version specific change to the
> > > application should be wrapped into a branch that is checked with parsed
> > > version number. With this way the original application's behavior should
> > > remain the same.
> > >
> > > The reason for that is we are having an issue right now that the validation
> > > team is struggling with the limited test vectors and inconsistency formatting
> > > between different FIPS CAVS versions. For example we still have FIPS TDES
> > test
> > > failing today due to the different test file versions.
> > > https://bugs.dpdk.org/show_bug.cgi?id=512
> > >
> > > The solution is certainly far from pretty but should help to share the
> > > maintenance effort amongst the contributors.
> > >
> > > The "FIPS_DEF_VERSION" can be removed of course.
> > >
> > > Regards,
> > > Fan
> > >
> > > > -----Original Message-----
> > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > Sent: Thursday, October 8, 2020 10:22 AM
> > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> > Akhil
> > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > <anoobj@marvell.com>
> > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> > > > > Hi Olivier,
> > > > >
> > > > > Anood and us had the similar discussion.
> > > > >
> > > > > Can we change the sample application to parse version data instead,
> > > > > and for the version specific code changes we will wrap them by a
> > > > > branch to compare the parsed version and the expected version?
> > > > > (we probably should have done that long time ago).
> > > > >
> > > > > I drafted a code change to parse the version data, see if you think it
> > > > > is ok?
> > > >
> > > > Thank you for your feedback.
> > > >
> > > > The code that gets the version looks good to me (I just have a
> > > > small comment, see below). However I'm not sure what to do with it.
> > > >
> > > > Do you mean we should return an error if the version is incorrect? Or
> > > > should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used
> > > > in your patch. In that case, I think it is a bit overkill. Do you think
> > > > it is a problem to always drop '\r'?
> > > >
> > > > If you think we should not support files containing '\r', I'm fine
> > > > with it, I can drop this particular patch.
> > > >
> > > >
> > > > >
> > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > b/examples/fips_validation/fips_validation.c
> > > > > index 9bdf257b8..9b6518c92 100644
> > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > @@ -98,7 +98,7 @@ fips_test_parse_header(void)
> > > > >  	uint32_t i;
> > > > >  	char *tmp;
> > > > >  	int ret;
> > > > > -	int algo_parsed = 0;
> > > > > +	int algo_parsed = 0, version_parsed = 0;
> > > > >  	time_t t = time(NULL);
> > > > >  	struct tm *tm_now = localtime(&t);
> > > > >
> > > > > @@ -107,6 +107,27 @@ fips_test_parse_header(void)
> > > > >  		return ret;
> > > > >
> > > > >  	for (i = 0; i < info.nb_vec_lines; i++) {
> > > > > +		/* parse the version info */
> > > > > +		tmp = strstr(info.vec[i], "CAVS ");
> > > > > +		if (tmp != NULL) {
> > > > > +			if (version_parsed != 0) {
> > > > > +				RTE_LOG(ERR, USER1,
> > > > > +					"Multiple version data\n");
> > > > > +				return -1;
> > > > > +			}
> > > > > +
> > > > > +			tmp = tmp + sizeof("CAVS ");
> > > >
> > > > I think it should be strlen(), because sizeof() will contain
> > > > the '\0'. Or it could be sizeof() - 1.
> > > >
> > > > > +
> > > > > +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> > > > > +				RTE_LOG(ERR, USER1, "Version (%s) too
> > > > long\n",
> > > > > +						tmp);
> > > > > +				return -1;
> > > > > +			}
> > > > > +
> > > > > +			strlcpy(info.version, tmp, MAX_VER_STRING_SIZE);
> > > > > +			version_parsed = 1;
> > > > > +		}
> > > > > +
> > > > >  		if (!algo_parsed) {
> > > > >  			if (strstr(info.vec[i], "AESVS")) {
> > > > >  				algo_parsed = 1;
> > > > > diff --git a/examples/fips_validation/fips_validation.h
> > > > b/examples/fips_validation/fips_validation.h
> > > > > index 75fa555fa..b8c60c55f 100644
> > > > > --- a/examples/fips_validation/fips_validation.h
> > > > > +++ b/examples/fips_validation/fips_validation.h
> > > > > @@ -15,6 +15,9 @@
> > > > >  #define MAX_BUF_SIZE		2048
> > > > >  #define MAX_STRING_SIZE		64
> > > > >  #define MAX_DIGEST_SIZE		64
> > > > > +#define MAX_VER_STRING_SIZE	8
> > > > > +
> > > > > +#define FIPS_DEF_VERSION	"21.0"
> > > > >
> > > > >  #define POSITIVE_TEST		0
> > > > >  #define NEGATIVE_TEST		-1
> > > > > @@ -155,6 +158,7 @@ struct sha_interim_data {
> > > > >  };
> > > > >
> > > > >  struct fips_test_interim_info {
> > > > > +	char version[MAX_VER_STRING_SIZE];
> > > > >  	FILE *fp_rd;
> > > > >  	FILE *fp_wr;
> > > > >  	enum file_types file_type;
> > > > >
> > > > >
> > > > > Regards,
> > > > > Fan
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > Sent: Tuesday, October 6, 2020 11:09 AM
> > > > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> > > > Akhil
> > > > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > > > <anoobj@marvell.com>
> > > > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input
> > files
> > > > > >
> > > > > > Hi Fan,
> > > > > >
> > > > > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > > > > > Hi Olivier,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > > > > > To: dev@dpdk.org
> > > > > > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil Goyal
> > > > > > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>;
> > > > > > Kusztal,
> > > > > > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input
> > files
> > > > > > > >
> > > > > > > > Some test vectors contain '\r' before '\n' (see link). Ignore them.
> > > > > > > >
> > > > > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-
> > 10-
> > > > > > 10.tar.gz
> > > > > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS
> > > > > > application")
> > > > > > > > Cc: stable@dpdk.org
> > > > > > > >
> > > > > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > > ---
> > > > > > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > > > > > >  1 file changed, 2 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > > > > b/examples/fips_validation/fips_validation.c
> > > > > > > > index 13f763c9aa..858f581ba3 100644
> > > > > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > > > > > >
> > > > > > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > > > > > >  			return -ENOMEM;
> > > > > > > > +		if (c == '\r')
> > > > > > > > +			continue;
> > > > > > > >  		if (c == '\n')
> > > > > > > >  			break;
> > > > > > > >  		line[loc++] = c;
> > > > > > > > --
> > > > > > >
> > > > > > >
> > > > > > > The patch looks ok but the test file link you provided in the patch is
> > > > CAVS
> > > > > > > 5.3.
> > > > > > >
> > > > > > > As mentioned in
> > > > > > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html,
> > the
> > > > > > supported
> > > > > > > CAVS supported version is 21.0 (not latest one by newer than 5.3).
> > In
> > > > CAVS
> > > > > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for
> > Windows
> > > > > > > right).
> > > > > >
> > > > > > Thank you for your feedback.
> > > > > >
> > > > > > I'm ok to drop this patch from the patchset if you feel it's useless, or
> > > > > > I can update the commit log with the information you provide, to
> > clarify
> > > > > > that it should not happen with the supported version of CAVS.
> > > > > >
> > > > > > Please let me know what you prefer.
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Zhang, Roy Fan]

Hi, 

I agree. Thanks a lot Olivier.
Also thanks for the other 2 FIPS patches :-).

Regards,
Fan


> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, October 8, 2020 3:20 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> <anoobj@marvell.com>
> Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> 
> Hi Fan,
> 
> So if we cannot know which version removed the \r, I suggest to just
> drop this patch. I thought it was a bug in the parser, but if it does
> not happen with files matching the supported CAVS version, there is
> nothing to fix.
> 
> What do you think?
> 
> Thanks,
> Olivier
> 
> 
> On Thu, Oct 08, 2020 at 12:41:11PM +0000, Zhang, Roy Fan wrote:
> > Hi Olivier,
> >
> > Unfortunately I wanted to find the same document since forever. NIST
> > did not provide this on their website. What I am sure is for CAVS 21.0
> > both the test vectors Intel used for testing and the ones provided by
> > our customer for debugging did not have \r in the files. In 2018 we
> > could find some sample request and response files from NIST website
> > but I just checked and they are gone.
> >
> > Regards,
> > Fan
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Thursday, October 8, 2020 12:32 PM
> > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> Akhil
> > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > <anoobj@marvell.com>
> > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files
> > >
> > > Hi Fan,
> > >
> > > Thank you for the clarification. One more question: do you know where I
> > > can find a description of the different FIPS CAVS versions? I would like
> > > to know from what version the \r has been removed.
> > >
> > > Thanks,
> > > Olivier
> > >
> > > On Thu, Oct 08, 2020 at 10:24:48AM +0000, Zhang, Roy Fan wrote:
> > > > Hi Olivier,
> > > >
> > > > Sorry I didn't state myself clear in the first place.
> > > >
> > > > My intention is '\r' check, or any future CAVS version specific change to
> the
> > > > application should be wrapped into a branch that is checked with
> parsed
> > > > version number. With this way the original application's behavior should
> > > > remain the same.
> > > >
> > > > The reason for that is we are having an issue right now that the
> validation
> > > > team is struggling with the limited test vectors and inconsistency
> formatting
> > > > between different FIPS CAVS versions. For example we still have FIPS
> TDES
> > > test
> > > > failing today due to the different test file versions.
> > > > https://bugs.dpdk.org/show_bug.cgi?id=512
> > > >
> > > > The solution is certainly far from pretty but should help to share the
> > > > maintenance effort amongst the contributors.
> > > >
> > > > The "FIPS_DEF_VERSION" can be removed of course.
> > > >
> > > > Regards,
> > > > Fan
> > > >
> > > > > -----Original Message-----
> > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > Sent: Thursday, October 8, 2020 10:22 AM
> > > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > > Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>;
> > > Akhil
> > > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > > <anoobj@marvell.com>
> > > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input
> files
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote:
> > > > > > Hi Olivier,
> > > > > >
> > > > > > Anood and us had the similar discussion.
> > > > > >
> > > > > > Can we change the sample application to parse version data instead,
> > > > > > and for the version specific code changes we will wrap them by a
> > > > > > branch to compare the parsed version and the expected version?
> > > > > > (we probably should have done that long time ago).
> > > > > >
> > > > > > I drafted a code change to parse the version data, see if you think it
> > > > > > is ok?
> > > > >
> > > > > Thank you for your feedback.
> > > > >
> > > > > The code that gets the version looks good to me (I just have a
> > > > > small comment, see below). However I'm not sure what to do with it.
> > > > >
> > > > > Do you mean we should return an error if the version is incorrect? Or
> > > > > should we only skip '\r' for old versions? FIPS_DEF_VERSION is not
> used
> > > > > in your patch. In that case, I think it is a bit overkill. Do you think
> > > > > it is a problem to always drop '\r'?
> > > > >
> > > > > If you think we should not support files containing '\r', I'm fine
> > > > > with it, I can drop this particular patch.
> > > > >
> > > > >
> > > > > >
> > > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > b/examples/fips_validation/fips_validation.c
> > > > > > index 9bdf257b8..9b6518c92 100644
> > > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > > @@ -98,7 +98,7 @@ fips_test_parse_header(void)
> > > > > >  	uint32_t i;
> > > > > >  	char *tmp;
> > > > > >  	int ret;
> > > > > > -	int algo_parsed = 0;
> > > > > > +	int algo_parsed = 0, version_parsed = 0;
> > > > > >  	time_t t = time(NULL);
> > > > > >  	struct tm *tm_now = localtime(&t);
> > > > > >
> > > > > > @@ -107,6 +107,27 @@ fips_test_parse_header(void)
> > > > > >  		return ret;
> > > > > >
> > > > > >  	for (i = 0; i < info.nb_vec_lines; i++) {
> > > > > > +		/* parse the version info */
> > > > > > +		tmp = strstr(info.vec[i], "CAVS ");
> > > > > > +		if (tmp != NULL) {
> > > > > > +			if (version_parsed != 0) {
> > > > > > +				RTE_LOG(ERR, USER1,
> > > > > > +					"Multiple version data\n");
> > > > > > +				return -1;
> > > > > > +			}
> > > > > > +
> > > > > > +			tmp = tmp + sizeof("CAVS ");
> > > > >
> > > > > I think it should be strlen(), because sizeof() will contain
> > > > > the '\0'. Or it could be sizeof() - 1.
> > > > >
> > > > > > +
> > > > > > +			if (strlen(tmp) >= MAX_VER_STRING_SIZE) {
> > > > > > +				RTE_LOG(ERR, USER1, "Version (%s)
> too
> > > > > long\n",
> > > > > > +						tmp);
> > > > > > +				return -1;
> > > > > > +			}
> > > > > > +
> > > > > > +			strlcpy(info.version, tmp,
> MAX_VER_STRING_SIZE);
> > > > > > +			version_parsed = 1;
> > > > > > +		}
> > > > > > +
> > > > > >  		if (!algo_parsed) {
> > > > > >  			if (strstr(info.vec[i], "AESVS")) {
> > > > > >  				algo_parsed = 1;
> > > > > > diff --git a/examples/fips_validation/fips_validation.h
> > > > > b/examples/fips_validation/fips_validation.h
> > > > > > index 75fa555fa..b8c60c55f 100644
> > > > > > --- a/examples/fips_validation/fips_validation.h
> > > > > > +++ b/examples/fips_validation/fips_validation.h
> > > > > > @@ -15,6 +15,9 @@
> > > > > >  #define MAX_BUF_SIZE		2048
> > > > > >  #define MAX_STRING_SIZE		64
> > > > > >  #define MAX_DIGEST_SIZE		64
> > > > > > +#define MAX_VER_STRING_SIZE	8
> > > > > > +
> > > > > > +#define FIPS_DEF_VERSION	"21.0"
> > > > > >
> > > > > >  #define POSITIVE_TEST		0
> > > > > >  #define NEGATIVE_TEST		-1
> > > > > > @@ -155,6 +158,7 @@ struct sha_interim_data {
> > > > > >  };
> > > > > >
> > > > > >  struct fips_test_interim_info {
> > > > > > +	char version[MAX_VER_STRING_SIZE];
> > > > > >  	FILE *fp_rd;
> > > > > >  	FILE *fp_wr;
> > > > > >  	enum file_types file_type;
> > > > > >
> > > > > >
> > > > > > Regards,
> > > > > > Fan
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > Sent: Tuesday, October 6, 2020 11:09 AM
> > > > > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > > > > > > Cc: dev@dpdk.org; Kovacevic, Marko
> <marko.kovacevic@intel.com>;
> > > > > Akhil
> > > > > > > Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > > > > > > <arkadiuszx.kusztal@intel.com>; stable@dpdk.org; Anoob Joseph
> > > > > > > <anoobj@marvell.com>
> > > > > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in
> input
> > > files
> > > > > > >
> > > > > > > Hi Fan,
> > > > > > >
> > > > > > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote:
> > > > > > > > Hi Olivier,
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > > > Sent: Tuesday, October 6, 2020 8:42 AM
> > > > > > > > > To: dev@dpdk.org
> > > > > > > > > Cc: Kovacevic, Marko <marko.kovacevic@intel.com>; Akhil
> Goyal
> > > > > > > > > <akhil.goyal@nxp.com>; Zhang, Roy Fan
> > > <roy.fan.zhang@intel.com>;
> > > > > > > Kusztal,
> > > > > > > > > ArkadiuszX <arkadiuszx.kusztal@intel.com>; stable@dpdk.org
> > > > > > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in
> input
> > > files
> > > > > > > > >
> > > > > > > > > Some test vectors contain '\r' before '\n' (see link). Ignore
> them.
> > > > > > > > >
> > > > > > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-
> 2007-
> > > 10-
> > > > > > > 10.tar.gz
> > > > > > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto
> FIPS
> > > > > > > application")
> > > > > > > > > Cc: stable@dpdk.org
> > > > > > > > >
> > > > > > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > > > > > ---
> > > > > > > > >  examples/fips_validation/fips_validation.c | 2 ++
> > > > > > > > >  1 file changed, 2 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/examples/fips_validation/fips_validation.c
> > > > > > > > > b/examples/fips_validation/fips_validation.c
> > > > > > > > > index 13f763c9aa..858f581ba3 100644
> > > > > > > > > --- a/examples/fips_validation/fips_validation.c
> > > > > > > > > +++ b/examples/fips_validation/fips_validation.c
> > > > > > > > > @@ -33,6 +33,8 @@ get_file_line(void)
> > > > > > > > >
> > > > > > > > >  		if (loc >= MAX_LINE_CHAR - 1)
> > > > > > > > >  			return -ENOMEM;
> > > > > > > > > +		if (c == '\r')
> > > > > > > > > +			continue;
> > > > > > > > >  		if (c == '\n')
> > > > > > > > >  			break;
> > > > > > > > >  		line[loc++] = c;
> > > > > > > > > --
> > > > > > > >
> > > > > > > >
> > > > > > > > The patch looks ok but the test file link you provided in the patch
> is
> > > > > CAVS
> > > > > > > > 5.3.
> > > > > > > >
> > > > > > > > As mentioned in
> > > > > > > >
> https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html,
> > > the
> > > > > > > supported
> > > > > > > > CAVS supported version is 21.0 (not latest one by newer than
> 5.3).
> > > In
> > > > > CAVS
> > > > > > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for
> > > Windows
> > > > > > > > right).
> > > > > > >
> > > > > > > Thank you for your feedback.
> > > > > > >
> > > > > > > I'm ok to drop this patch from the patchset if you feel it's useless,
> or
> > > > > > > I can update the commit log with the information you provide, to
> > > clarify
> > > > > > > that it should not happen with the supported version of CAVS.
> > > > > > >
> > > > > > > Please let me know what you prefer.
> > > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Olivier

Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files

[Akhil Goyal]

> >
> > So if we cannot know which version removed the \r, I suggest to just
> > drop this patch. I thought it was a bug in the parser, but if it does
> > not happen with files matching the supported CAVS version, there is
> > nothing to fix.
> >

Applied the series to dpdk-next-crypto
As suggested, 2/3 is dropped.

Thanks.