DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Kovacevic, Marko" <marko.kovacevic@intel.com>,
	Akhil Goyal <akhil.goyal@nxp.com>,
	"Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>,
	Anoob Joseph <anoobj@marvell.com>
Subject: Re: [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files
Date: Thu, 8 Oct 2020 10:24:48 +0000	[thread overview]
Message-ID: <BL0PR11MB30438E89AFD6DEF83A678C87B80B0@BL0PR11MB3043.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20201008092131.GU21395@platinum>

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

  reply	other threads:[~2020-10-08 10:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06  7:41 [dpdk-dev] [PATCH 0/3] examples/fips_validation: misc fixes Olivier Matz
2020-10-06  7:41 ` [dpdk-dev] [PATCH 1/3] examples/fips_validation: fix buffer overflow Olivier Matz
2020-10-06  8:48   ` Zhang, Roy Fan
2020-10-06  7:41 ` [dpdk-dev] [PATCH 2/3] examples/fips_validation: ignore \r in input files Olivier Matz
2020-10-06  8:47   ` Zhang, Roy Fan
2020-10-06 10:09     ` Olivier Matz
2020-10-08  8:50       ` Zhang, Roy Fan
2020-10-08  9:21         ` Olivier Matz
2020-10-08 10:24           ` Zhang, Roy Fan [this message]
2020-10-08 11:32             ` Olivier Matz
2020-10-08 12:41               ` Zhang, Roy Fan
2020-10-08 14:19                 ` Olivier Matz
2020-10-09  9:33                   ` Zhang, Roy Fan
2020-10-09 18:19                     ` Akhil Goyal
2020-10-06  7:41 ` [dpdk-dev] [PATCH 3/3] examples/fips_validation: support self-test only Olivier Matz
2020-10-06  8:55   ` Zhang, Roy Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR11MB30438E89AFD6DEF83A678C87B80B0@BL0PR11MB3043.namprd11.prod.outlook.com \
    --to=roy.fan.zhang@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=arkadiuszx.kusztal@intel.com \
    --cc=dev@dpdk.org \
    --cc=marko.kovacevic@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).