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: Fri, 9 Oct 2020 09:33:19 +0000
Message-ID: <BL0PR11MB304379052021D8C2CD6CDF92B8080@BL0PR11MB3043.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20201008141947.GY21395@platinum>

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

  reply	other threads:[~2020-10-09  9:33 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
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 [this message]
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=BL0PR11MB304379052021D8C2CD6CDF92B8080@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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git