[PATCH v6 0/4] Add ACVP Tool

[PATCH v6 0/4] Add ACVP Tool

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v1: https://mails.dpdk.org/archives/ci/2022-January/001599.html
v2: https://mails.dpdk.org/archives/ci/2022-January/001611.html
v3: https://mails.dpdk.org/archives/ci/2022-February/001636.html
v4: https://mails.dpdk.org/archives/ci/2022-April/001702.html
v5: https://mails.dpdk.org/archives/ci/2023-March/002112.html
v6:
  * fix commit messages

Jeremy Spewock (4):
  tools: add acvp_tool
  tools: add default config file for acvp_tool
  tools: add requirements file for acvp_tool
  doc: add readme file for acvp_tool

 tools/acvp/README           | 129 +++++++++++++++
 tools/acvp/__init__.py      |   0
 tools/acvp/acvp_config.json |  52 ++++++
 tools/acvp/acvp_tool.py     | 319 ++++++++++++++++++++++++++++++++++++
 tools/acvp/requirements.txt |   7 +
 5 files changed, 507 insertions(+)
 create mode 100644 tools/acvp/README
 create mode 100644 tools/acvp/__init__.py
 create mode 100644 tools/acvp/acvp_config.json
 create mode 100755 tools/acvp/acvp_tool.py
 create mode 100644 tools/acvp/requirements.txt

-- 
2.40.0

[PATCH v6 1/4] tools: add acvp_tool

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

This tool is used to interact with the ACVP API.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 tools/acvp/__init__.py  |   0
 tools/acvp/acvp_tool.py | 319 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 319 insertions(+)
 create mode 100644 tools/acvp/__init__.py
 create mode 100755 tools/acvp/acvp_tool.py

diff --git a/tools/acvp/__init__.py b/tools/acvp/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tools/acvp/acvp_tool.py b/tools/acvp/acvp_tool.py
new file mode 100755
index 0000000..40d2f2f
--- /dev/null
+++ b/tools/acvp/acvp_tool.py
@@ -0,0 +1,319 @@
+#!/usr/bin/env python3
+
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022 The University of New Hampshire
+
+import hashlib
+import sys
+import time
+import base64
+import argparse
+import os
+import json
+import logging
+from typing import Tuple, Optional, Any, Dict, List
+
+import pyotp
+import requests
+
+
+class ACVPProxy:
+    def __init__(self, cert_path: str, key_path: str, totp_path: str,
+                 config_path: str):
+        """ACVP Proxy used to abstract API calls.
+
+        @param cert_path: Path to the client certificate.
+        @param key_path: Path to the client key.
+        @param totp_path: Path to the one-time password seed.
+        @param config_path: Path to the configuration for the session.
+        """
+        self.cert: Tuple[str, str] = (cert_path, key_path)
+
+        if None in self.cert:
+            logging.error('Missing certificate/key file.')
+            sys.exit(1)
+
+        self.totp_path: str = totp_path
+        self.login_data: Optional[Dict[str, Any]] = None
+        self.session_data: Optional[Dict[str, Any]] = None
+
+        with open(config_path, 'r') as f:
+            self.config: Any = json.load(f)
+
+    def __get_totp(self) -> str:
+        """Get the current one-time password.
+
+        Uses the totp_path argument when instantiating to
+        read the TOTP seed.
+
+        @return: String containing the password.
+        """
+        with open(self.totp_path, 'r') as f:
+            seed = f.read().strip()
+        base64_seed = base64.b32encode(base64.b64decode(seed)).decode('utf-8')
+        totp = pyotp.TOTP(s=base64_seed, digits=8, digest=hashlib.sha256,
+                          interval=30)
+        return totp.now()
+
+    def __fetch_vector_set(self, url: str) -> Optional[Dict]:
+        """Fetch the vector set object from a URL.
+
+        @param url: URL of the vector set given by NIST.
+        @return: Dictionary of the response from the NIST API.
+        The returned dictionary comes without the session information.
+        """
+        logging.info(f'Fetching vector set {url}')
+        token = self.session_data['jwt']
+        while True:
+            response = requests.get(
+                f'{self.config["url"]}{url}',
+                cert=self.cert,
+                headers={'Authorization': f'Bearer {token}'}
+            )
+            if not response.ok:
+                logging.error(f'Failed to fetch vector set {url}')
+                logging.error(json.dumps(response.json(), indent=4))
+                return None
+
+            vector_set_json = response.json()[1]
+            if 'retry' in vector_set_json:
+                duration = vector_set_json['retry']
+                logging.info(f'Server says retry in {duration} seconds...')
+                time.sleep(duration)
+                continue
+
+            logging.info(f'Downloaded vector set {url}')
+            return vector_set_json
+
+    def login(self) -> bool:
+        """Log into the API server.
+
+        Uses the instance's current TOTP seed and certificate file paths
+        to authenticate with the API.
+
+        If successful, the access token of the account will be stored in
+        the instance.
+
+        @return: True if authentication succeeded, false otherwise.
+        """
+        response = requests.post(
+            url=f'{self.config["url"]}/acvp/v1/login',
+            json=[
+                {'acvVersion': '1.0'},
+                {'password': self.__get_totp()},
+            ],
+            cert=self.cert,
+        )
+
+        if not response.ok:
+            logging.error('Failed to log in.')
+            logging.error(json.dumps(response.json(), indent=4))
+            return False
+
+        self.login_data = response.json()[1]
+        # Renamed 'accessToken' to 'jwt' in the json object
+        # to stay consistent with libacvp
+        self.login_data['jwt'] = self.login_data.pop('accessToken')
+        return True
+
+    def register(self) -> Optional[List[Any]]:
+        """Register a new test session.
+
+        This requires the ACVPProxy instance to be authenticated (use .login).
+
+        @return: If registration succeeded, it will return the list
+        containing session information and vector sets.
+        """
+        if self.login_data is None:
+            logging.error('ACVP proxy cannot register a test session without '
+                          'logging in first.')
+            return None
+
+        response = requests.post(
+            url=f'{self.config["url"]}/acvp/v1/testSessions',
+            json=[
+                {'acvVersion': '1.0'},
+                {
+                    'isSample': False,
+                    'algorithms': self.config['algorithms']
+                }
+            ],
+            cert=self.cert,
+            headers={'Authorization': f'Bearer {self.login_data["jwt"]}'}
+        )
+
+        if not response.ok:
+            logging.error('Unable to register.')
+            logging.error(json.dumps(response.json(), indent=4))
+            return None
+
+        self.session_data = response.json()[1]
+        # Renamed 'accessToken' to 'jwt' in the json object
+        # to stay consistent with libacvp
+        self.session_data['jwt'] = self.session_data.pop('accessToken')
+        write_data = [self.session_data]
+
+        for url in self.session_data['vectorSetUrls']:
+            write_data.append(self.__fetch_vector_set(url))
+
+        return write_data
+
+    def fetch_verdict(self, vector_results: List[Any]) -> List[Any]:
+        """Fetch verdict for a list of vector sets.
+
+        @param vector_results: List of vector set dictionaries with answers.
+        @return: A list containing the session information and verdicts.
+        """
+        session_url = self.session_data['url']
+        write_data = [self.session_data]
+        for _, vector_set in vector_results:
+            vector_set_id = vector_set['vsId']
+            logging.info(f'Downloading verdict for vector set {vector_set_id}')
+            while True:
+                result = requests.get(
+                    f'{self.config["url"]}{session_url}'
+                    f'/vectorSets/{vector_set_id}/results',
+                    cert=self.cert,
+                    headers={
+                        'Authorization': f'Bearer {self.session_data["jwt"]}'
+                    }
+                )
+                version, result_json = result.json()
+                if 'retry' in result_json:
+                    duration = result_json['retry']
+                    logging.info(f'Vector set verdict not ready, waiting '
+                                 f'{duration} seconds...')
+                    time.sleep(duration)
+                    continue
+
+                write_data.append([version, result_json])
+                break
+        return write_data
+
+    def upload(self, vector_sets: List[Any]) -> bool:
+        """Upload the given vector sets.
+
+        @param vector_sets: List of vector set dictionaries with answers.
+        @return: True if uploading succeeded.
+        """
+        has_error = False
+        session_url = self.session_data['url']
+
+        for version, vector_set in vector_sets:
+            response = requests.post(
+                f'{self.config["url"]}{session_url}/vectorSets/'
+                f'{vector_set["vsId"]}/results',
+                json=[version, vector_set],
+                cert=self.cert,
+                headers={'Authorization': f'Bearer {self.session_data["jwt"]}'}
+            )
+
+            if not response.ok:
+                has_error = True
+                logging.error(f'Could not upload vector set response for '
+                              f'vector set ID {vector_set["vsId"]}.')
+                logging.error(json.dumps(response.json(), indent=4))
+                continue
+
+        return not has_error
+
+
+def main(request_path: Optional[str],
+         response_path: Optional[str],
+         verdict_path: Optional[str],
+         do_upload: bool,
+         config_path: str):
+
+    if request_path and response_path:
+        logging.error('You cannot use both a request and a response file.')
+        sys.exit(1)
+
+    if not any([request_path, response_path]):
+        logging.error('You must specify either a request or a response file.')
+        sys.exit(1)
+
+    proxy = ACVPProxy(
+        cert_path=os.getenv('ACVP_CERT_FILE'),
+        key_path=os.getenv('ACVP_KEY_FILE'),
+        totp_path=os.getenv('ACVP_SEED_FILE'),
+        config_path=config_path,
+    )
+
+    logging.info('Attempting to log in...')
+    if not proxy.login():
+        logging.error('Could not log in.')
+        sys.exit(1)
+    logging.info('Successfully logged in.')
+
+    if request_path:
+        logging.info('Creating a new test session and downloading vectors...')
+        test_session = proxy.register()
+        if not test_session:
+            logging.error('Could not create a new test session.')
+            sys.exit(1)
+
+        with open(request_path, 'w') as f:
+            json.dump(test_session, f, indent=4)
+    elif response_path:
+        logging.info('Using response file...')
+        with open(response_path, 'r') as upload_file:
+            upload_json: List[Any] = json.load(upload_file)
+        proxy.session_data = upload_json[0]
+
+        if do_upload:
+            logging.info('Uploading response file...')
+            if not proxy.upload(upload_json[1:]):
+                logging.error('Could not successfully upload results file.')
+                sys.exit(1)
+
+        if verdict_path:
+            logging.info('Fetching verdict...')
+            verdict = proxy.fetch_verdict(upload_json[1:])
+            if not verdict:
+                logging.error('Could not successfully fetch verdict file.')
+                sys.exit(1)
+            with open(verdict_path, 'w') as f:
+                json.dump(verdict, f, indent=4)
+
+    logging.info('Done')
+
+
+if __name__ == '__main__':
+    logging.basicConfig(level=logging.INFO)
+    parser = argparse.ArgumentParser(description='ACVP tool to fetch tests '
+                                                 'and upload results.')
+    parser.add_argument('--request', '-r',
+                        help='Path to download a request file. '
+                             'If specified, the tool start a new test session '
+                             'and download the test information into the '
+                             'given path.')
+    parser.add_argument('--response', '-s',
+                        help='Path of the response file. '
+                             'If specified, the tool will use this file '
+                             'to determine session information. '
+                             'This argument must be used when uploading '
+                             'results or downloading verdicts.')
+    parser.add_argument('--verdict', '-v',
+                        help='Download the verdict to the specified path. '
+                             'If this flag is set, the tool will download the '
+                             'verdict for the given response file '
+                             '(--response).')
+    parser.add_argument('--upload', '-u',
+                        help='Upload the given response file to the API. '
+                             'If this flag is set, the tool will upload the '
+                             'given response file (--response).',
+                        action='store_true')
+    parser.add_argument('--config', '-c',
+                        help='Path of the configuration file. '
+                             '(Default: acvp_config.json)',
+                        default='acvp_config.json')
+
+    args = parser.parse_args()
+
+    main(
+        request_path=args.request,
+        response_path=args.response,
+        verdict_path=args.verdict,
+        do_upload=args.upload,
+        config_path=args.config,
+    )
-- 
2.40.0

[PATCH v6 2/4] tools: add default config file for acvp_tool

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

expanded coverage of acvp_tool default config file

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 tools/acvp/acvp_config.json | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 tools/acvp/acvp_config.json

diff --git a/tools/acvp/acvp_config.json b/tools/acvp/acvp_config.json
new file mode 100644
index 0000000..55c20bf
--- /dev/null
+++ b/tools/acvp/acvp_config.json
@@ -0,0 +1,52 @@
+{
+    "url": "https://demo.acvts.nist.gov",
+    "algorithms": [
+        {
+            "algorithm": "ACVP-TDES-CBC",
+            "revision": "1.0",
+	    "keyingOption": [
+                1
+            ],
+	    "messageLength": [{"min": 0, "max": 65535, "increment": 1}],
+	    "capabilities": [
+		{
+		  "direction": ["gen", "ver"],
+		  "keyLen": [128],
+		  "msgLen": [
+			{
+			  "max": 65536,
+			  "min": 0,
+			  "increment": 256
+			}
+		  ],
+		  "macLen": [
+			{
+			  "min": 64,
+			  "max": 128,
+			  "increment": 8
+			}
+		  ]
+		}
+	    ],
+            "direction": ["encrypt"],
+            "keyLen": [128, 192, 256],
+	    "macLen": [
+    		{
+      		    "min": 80,
+      		    "max": 160,
+      		   "increment": 8
+    		}
+  	    ],
+            "tagLen": [128],
+            "aadLen": [0],
+            "ivGen": "internal",
+	    "ivGenMode": "8.2.2",
+            "ivLen": [96],
+            "payloadLen": [
+                128
+            ],
+	    "overflowCounter": true,
+	    "incrementalCounter": true
+        }
+    ]
+}
-- 
2.40.0

[PATCH v6 3/4] tools: add requirements file for acvp_tool

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Adds the basic requirements for the acvp_tool script.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 tools/acvp/requirements.txt | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 tools/acvp/requirements.txt

diff --git a/tools/acvp/requirements.txt b/tools/acvp/requirements.txt
new file mode 100644
index 0000000..428f06c
--- /dev/null
+++ b/tools/acvp/requirements.txt
@@ -0,0 +1,7 @@
+certifi==2021.10.8
+charset-normalizer==2.0.10
+idna==3.3
+pyotp==2.6.0
+requests==2.27.1
+six==1.16.0
+urllib3==1.26.8
-- 
2.40.0

[PATCH v6 4/4] doc: add readme file for acvp_tool

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

updated out-of-date acvp_tool readme

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 tools/acvp/README | 129 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 tools/acvp/README

diff --git a/tools/acvp/README b/tools/acvp/README
new file mode 100644
index 0000000..23a1aef
--- /dev/null
+++ b/tools/acvp/README
@@ -0,0 +1,129 @@
+The ACVP tool is a general tool for interacting with the NIST ACVP API
+in order to test different cryptographic implementations.
+
+It produces machine-readable output for parsing in a CI environment.
+
+Supported Algorithms
+--------------------
+* AES-CBC
+* AES-CMAC
+* AES-GMAC
+* HMAC-SHA-1
+* TDES-CBC
+* AES-CTR
+
+Requirements
+------------
+
+There are also python packages you need to download from the requirements.txt file:
+* pyotp
+* requests
+
+Along with these, you will also need to install the `nasm` package using your local package manager.
+
+The tool expects that you have all the credential files from NIST:
+* Client certificate (usually a .cer file from NIST)
+* Key file for the certificate
+* Time-based one-time password seed file (usually a .txt file from NIST)
+
+The path to each file must be stored in an environment variable:
+* $ACVP_SEED_FILE  =  Path to the TOTP seed .txt file    (given by NIST).
+* $ACVP_CERT_FILE  =  Path to the client .cer/.crt file  (given by NIST).
+* $ACVP_KEY_FILE   =  Path to the certificate key file   (generated by user).
+
+If you do not have the required files from NIST, you must email them
+to create demo credentials.
+https://pages.nist.gov/ACVP/#access
+
+
+Setup
+-----
+
+After setting the environment variables as described in the
+"Requirements" section, you will need to edit the acvp_config.json file.
+
+The acvp_config.json file is expected to be a json object
+containing two keys: "url" and "algorithms"
+
+"url" must be the base URL string of the API you want to use.
+"algorithms" must be an array of algorithm objects as detailed in the
+ACVP API specification here:
+https://github.com/usnistgov/ACVP/wiki/ACVTS-End-User-Documentation . In the case of the supported algorithms listed above, the only thing that will need to change in the config file is the `"algorithm"` field to match the name of the algorithm you would like to test.
+* In order to test AES-CTR you'll also have to remove the key `"ivGenMode"` 
+
+Now you can use the acvp_tool.py script to register a test session,
+upload the results, and download the verdict.
+
+In order to run the DPDK sample application, there are a few libraries which must be installed:
+* Intel IPSec Multi-buffer (v1.3)
+```
+git clone https://github.com/intel/intel-ipsec-mb.git
+cd intel-ipsec-mb
+git checkout v1.3
+make -j 4
+make install
+```
+* FIPS Object Module
+```
+curl -o openssl-fips-2.0.16.tar.gz https://www.openssl.org/source/openssl-fips-2.0.16.tar.gz
+tar xvfm openssl-fips-2.0.16.tar.gz
+cd openssl-fips-2.0.16
+./config
+make
+make install
+```
+* OpenSSL library
+```
+curl -o openssl-1.0.2o.tar.gz https://www.openssl.org/source/openssl-1.0.2o.tar.gz
+export CFLAGS='-fPIC'
+tar xvfm openssl-1.0.2o.tar.gz
+cd openssl-1.0.2o
+./config shared fips
+make depend
+make
+```
+Usage
+-----
+### Interacting with ACVP API
+To see all options available, use the --help flag.
+
+First, register and download a new test session with the tool:
+
+    acvp_tool.py --request $DOWNLOAD_PATH
+The file written to $DOWNLOAD_PATH will contain both the session information and the test vectors.
+
+You should use the DPDK FIPS validation example application to test
+the vectors in this file. The example application will generate
+the result file which is uploaded back to the ACVP API.
+
+After running tests with the vector file, you can submit the result:
+
+    acvp_tool.py --response $RESULT_PATH --upload
+where $RESULT_PATH is the path of the file containing the answers.
+
+Once you submit your results, you can do
+
+    acvp_tool.py --response $RESULT_PATH --verdict $VERDICT_PATH
+where $VERDICT_PATH is where you want to save the verdict information.
+The verdict file will contain the result of each test case submitted.
+
+You can also combine the options:
+
+    acvp_tool.py --response $RESULT_PATH --upload --verdict $VERDICT_PATH
+
+### Using the DPDK FIPS Validation Example Application
+First, you have to make sure that you configure DPDK to build the FIPS sample application before you compile with ninja
+```
+#inside dpdk/
+meson build --werror
+meson configure -Dexamples=fips_validation build
+sudo ninja -C build
+```
+Once this has finished, you can now run the sample application and validate the test vectors. In order to run this validation step, you have to supply a valid crypto device and either a `*.json` or `*.req` file with vectors for validation. You can use the virtual device `crypto_aesni_mb` provided by the Intel IPSec Multi-buffer library and pass the JSON file containing test vectors from the ACVP API using `--req-file`. 
+
+Example usage:
+    
+    #inside dpdk/
+    build/examples/dpdk-fips_validation --vdev crypto_aesni_mb -- --req-file aes-cbc-vectors.json --rsp-file aes-cbc-answers.rsp --cryptodev crypto_aesni_mb`
+
+The file path passed into `--rsp-file` will contain the validated vectors from the sample applications and can be passed to the ACVP API to receive a verdict on your results.
\ No newline at end of file
-- 
2.40.0

RE: [PATCH v6 2/4] tools: add default config file for acvp_tool

[Ali Alnubani]

> -----Original Message-----
> From: jspewock@iol.unh.edu <jspewock@iol.unh.edu>
> Sent: Monday, March 27, 2023 7:52 PM
> To: ci@dpdk.org
> Cc: Jeremy Spewock <jspewock@iol.unh.edu>
> Subject: [PATCH v6 2/4] tools: add default config file for acvp_tool
> 
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> expanded coverage of acvp_tool default config file

This only describes the diff with Brandon's patch. The original commit message should have been kept instead and expanded if needed.

> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---


I see the following warnings when applying the patch:

.git/rebase-apply/patch:45: space before tab in indent.
                {
.git/rebase-apply/patch:46: space before tab in indent.
                    "min": 80,
.git/rebase-apply/patch:47: space before tab in indent.
                    "max": 160,
.git/rebase-apply/patch:48: space before tab in indent.
                   "increment": 8
.git/rebase-apply/patch:49: space before tab in indent.
                }
warning: squelched 1 whitespace error
warning: 6 lines add whitespace errors.

- Ali

RE: [PATCH v6 4/4] doc: add readme file for acvp_tool

[Ali Alnubani]

> -----Original Message-----
> From: jspewock@iol.unh.edu <jspewock@iol.unh.edu>
> Sent: Monday, March 27, 2023 7:52 PM
> To: ci@dpdk.org
> Cc: Jeremy Spewock <jspewock@iol.unh.edu>
> Subject: [PATCH v6 4/4] doc: add readme file for acvp_tool
> 
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> updated out-of-date acvp_tool readme

This only describes the diff with Brandon's patch. The original commit message should have been kept instead and expanded if needed.

> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
[..]
> +* FIPS Object Module
> +```
> +curl -o openssl-fips-2.0.16.tar.gz https://www.openssl.org/source/openssl-
> fips-2.0.16.tar.gz
> +tar xvfm openssl-fips-2.0.16.tar.gz
> +cd openssl-fips-2.0.16
> +./config
> +make
> +make install
> +```

Is this module required even with recent versions of openssl?

> +* OpenSSL library
> +```
> +curl -o openssl-1.0.2o.tar.gz https://www.openssl.org/source/openssl-
> 1.0.2o.tar.gz
> +export CFLAGS='-fPIC'
> +tar xvfm openssl-1.0.2o.tar.gz
> +cd openssl-1.0.2o
> +./config shared fips
> +make depend
> +make
> +```

OpenSSL 1.0.2 is deprecated and probably vulnerable (https://www.openssl.org/news/vulnerabilities-1.0.2.html).
You're also only building locally here, and you aren't pointing DPDK build to it, so I doubt it's making a difference.

Can you please double check these dependencies?

[..]
> +### Using the DPDK FIPS Validation Example Application
> +First, you have to make sure that you configure DPDK to build the FIPS
> sample application before you compile with ninja
> +```
> +#inside dpdk/
> +meson build --werror
> +meson configure -Dexamples=fips_validation build

You can combine them into a single command:
meson --werror  -Dexamples=fips_validation build

> +sudo ninja -C build

You're only making local changes here. If sudo wasn't required for the meson commands, it won't be required for this one.

Additionally, I see the following warnings when applying the patch:

Applying: doc: add readme file for acvp_tool
.git/rebase-apply/patch:63: trailing whitespace.
* In order to test AES-CTR you'll also have to remove the key `"ivGenMode"`
.git/rebase-apply/patch:133: trailing whitespace.

- Ali

RE: [PATCH v6 0/4] Add ACVP Tool

[Ali Alnubani]

> -----Original Message-----
> From: jspewock@iol.unh.edu <jspewock@iol.unh.edu>
> Sent: Monday, March 27, 2023 7:52 PM
> To: ci@dpdk.org
> Cc: Jeremy Spewock <jspewock@iol.unh.edu>
> Subject: [PATCH v6 0/4] Add ACVP Tool
> 
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> v1: https://mails.dpdk.org/archives/ci/2022-January/001599.html
> v2: https://mails.dpdk.org/archives/ci/2022-January/001611.html
> v3: https://mails.dpdk.org/archives/ci/2022-February/001636.html
> v4: https://mails.dpdk.org/archives/ci/2022-April/001702.html
> v5: https://mails.dpdk.org/archives/ci/2023-March/002112.html
> v6:
>   * fix commit messages
> 
> Jeremy Spewock (4):
>   tools: add acvp_tool
>   tools: add default config file for acvp_tool
>   tools: add requirements file for acvp_tool
>   doc: add readme file for acvp_tool
> 

Hello Jeremy,

In addition to the other comments on 2/4 and 4/4, I would like to note that Brandon's sign-off-by tag shouldn't be removed from the patches as he was the original author.

Regards,
Ali

Re: [PATCH v6 4/4] doc: add readme file for acvp_tool

[Jeremy Spewock]

[-- Attachment #1: Type: text/plain, Size: 3352 bytes --]

On Tue, Mar 28, 2023 at 4:27 AM Ali Alnubani <alialnu@nvidia.com> wrote:

> > -----Original Message-----
> > From: jspewock@iol.unh.edu <jspewock@iol.unh.edu>
> > Sent: Monday, March 27, 2023 7:52 PM
> > To: ci@dpdk.org
> > Cc: Jeremy Spewock <jspewock@iol.unh.edu>
> > Subject: [PATCH v6 4/4] doc: add readme file for acvp_tool
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > updated out-of-date acvp_tool readme
>
> This only describes the diff with Brandon's patch. The original commit
> message should have been kept instead and expanded if needed.
>
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> [..]
> > +* FIPS Object Module
> > +```
> > +curl -o openssl-fips-2.0.16.tar.gz
> https://www.openssl.org/source/openssl-
> > fips-2.0.16.tar.gz
> > +tar xvfm openssl-fips-2.0.16.tar.gz
> > +cd openssl-fips-2.0.16
> > +./config
> > +make
> > +make install
> > +```
>
> Is this module required even with recent versions of openssl?
>

In recent testing of the dependencies, when this module wasn't present
there is inconsistent behavior that leads to errors.


>
> > +* OpenSSL library
> > +```
> > +curl -o openssl-1.0.2o.tar.gz https://www.openssl.org/source/openssl-
> > 1.0.2o.tar.gz
> > +export CFLAGS='-fPIC'
> > +tar xvfm openssl-1.0.2o.tar.gz
> > +cd openssl-1.0.2o
> > +./config shared fips
> > +make depend
> > +make
> > +```
>
> OpenSSL 1.0.2 is deprecated and probably vulnerable (
> https://www.openssl.org/news/vulnerabilities-1.0.2.html).
> You're also only building locally here, and you aren't pointing DPDK build
> to it, so I doubt it's making a difference.
>

After doing some testing, it seems that this library is not needed in order
to run the application and get passing vectors.


>
> Can you please double check these dependencies?
>
>
These dependencies were actually taken from the test plan documentation
written for running the FIPS sample application. This was the only
documentation I could find that mentions the required libraries in order to
run the sample application so I followed it as closely as I could. The only
thing I had to change was the version of the IPsec library because it
wouldn't build with this older outdated version. This documentation should
likely be updated:

https://git.dpdk.org/tools/dts/tree/test_plans/fips_cryptodev_test_plan.rst


> [..]
> > +### Using the DPDK FIPS Validation Example Application
> > +First, you have to make sure that you configure DPDK to build the FIPS
> > sample application before you compile with ninja
> > +```
> > +#inside dpdk/
> > +meson build --werror
> > +meson configure -Dexamples=fips_validation build
>
> You can combine them into a single command:
> meson --werror  -Dexamples=fips_validation build
>
> > +sudo ninja -C build
>
> You're only making local changes here. If sudo wasn't required for the
> meson commands, it won't be required for this one.
>
> Additionally, I see the following warnings when applying the patch:
>
> Applying: doc: add readme file for acvp_tool
> .git/rebase-apply/patch:63: trailing whitespace.
> * In order to test AES-CTR you'll also have to remove the key `"ivGenMode"`
> .git/rebase-apply/patch:133: trailing whitespace.
>

Fixed in v7.


>
> - Ali
>

Thanks,
Jeremy

[-- Attachment #2: Type: text/html, Size: 5404 bytes --]