From 7ad19a30a4fd6c53effa892a17ad978aacb8f3a6 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 15:12:38 +0100 Subject: [PATCH 01/15] First start of windows test support --- crypto_sign/dilithium-iii/clean/api.h | 1 + test/Makefile.Microsoft_nmake | 20 +++++++++++------- test/crypto_sign/functest.c | 10 ++++----- test/crypto_sign/testvectors.c | 1 + test/helpers.py | 30 +++++++++++++++++++++++---- test/test_dynamic_memory.py | 3 +-- test/test_format.py | 4 +++- test/test_functest.py | 18 +++++++++++----- 8 files changed, 63 insertions(+), 24 deletions(-) diff --git a/crypto_sign/dilithium-iii/clean/api.h b/crypto_sign/dilithium-iii/clean/api.h index 74effc3c..c6e80d95 100644 --- a/crypto_sign/dilithium-iii/clean/api.h +++ b/crypto_sign/dilithium-iii/clean/api.h @@ -1,4 +1,5 @@ #ifndef API_H +#include #define API_H #include diff --git a/test/Makefile.Microsoft_nmake b/test/Makefile.Microsoft_nmake index c37a727e..52616574 100644 --- a/test/Makefile.Microsoft_nmake +++ b/test/Makefile.Microsoft_nmake @@ -1,7 +1,7 @@ # This Makefile can be used with Microsoft Visual Studio's nmake using the command: # nmake /f Makefile.Microsoft_nmake -# override as desired +# override as desired, use /E # vim: set ts=4 sw=4 et: TYPE=kem SCHEME=kyber768 @@ -31,17 +31,23 @@ clean-scheme: nmake /f Makefile.Microsoft_nmake clean cd ..\..\..\test -$(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).EXE: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\randombytes.obj - -MKDIR $(DEST_DIR) - $(CC) /c crypto_$(TYPE)\functest.c $(CFLAGS) /I $(SCHEME_DIR) /DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) - LINK.EXE /OUT:$@ functest.obj $(COMMON_OBJECTS_NOPATH) randombytes.obj $(SCHEME_DIR)\lib$(SCHEME)_$(IMPLEMENTATION).lib Advapi32.lib +functest: $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).exe -$(DEST_DIR)\testvectors_$(SCHEME)_$(IMPLEMENTATION).EXE: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\notrandombytes.obj +testvectors: $(DEST_DIR)\testvectors_$(SCHEME)_$(IMPLEMENTATION).exe + +$(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).exe: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\randombytes.obj -MKDIR $(DEST_DIR) + -DEL functest.obj + $(CC) /c crypto_$(TYPE)\functest.c $(CFLAGS) /I $(SCHEME_DIR) /DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) + LINK.EXE /OUT:$@ functest.obj $(COMMON_OBJECTS_NOPATH) notrandombytes.obj $(SCHEME_DIR)\lib$(SCHEME)_$(IMPLEMENTATION).lib Advapi32.lib + +$(DEST_DIR)\testvectors_$(SCHEME)_$(IMPLEMENTATION).exe: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\notrandombytes.obj + -MKDIR $(DEST_DIR) + -DEL testvectors.obj $(CC) /c crypto_$(TYPE)\testvectors.c $(CFLAGS) /I $(SCHEME_DIR) /DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) LINK.EXE /OUT:$@ testvectors.obj $(COMMON_OBJECTS_NOPATH) notrandombytes.obj $(SCHEME_DIR)\lib$(SCHEME)_$(IMPLEMENTATION).lib clean: -DEL functest.obj testvectors.obj -DEL $(COMMON_OBJECTS_NOPATH) randombytes.obj notrandombytes.obj - -DEL $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).EXE + -DEL $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).exe diff --git a/test/crypto_sign/functest.c b/test/crypto_sign/functest.c index 60ea57c2..42a73cf6 100644 --- a/test/crypto_sign/functest.c +++ b/test/crypto_sign/functest.c @@ -98,9 +98,9 @@ static int test_sign(void) { // twice if ((returncode = crypto_sign_open(sm + 8, &mlen, sm + 8, smlen, pk + 8)) != 0) { - printf("ERROR Signature did not verify correctly!\n"); + fprintf(stderr, "ERROR Signature did not verify correctly!\n"); if (returncode > 0) { - puts("ERROR return code should be < 0 on failure"); + fprintf(stderr, "ERROR return code should be < 0 on failure"); } return 1; } @@ -109,7 +109,7 @@ static int test_sign(void) { check_canary(sk) || check_canary(sk + CRYPTO_SECRETKEYBYTES + 8) || check_canary(sm) || check_canary(sm + MLEN + CRYPTO_BYTES + 8) || check_canary(m) || check_canary(m + MLEN + 8)) { - printf("ERROR canary overwritten\n"); + fprintf(stderr, "ERROR canary overwritten\n"); return 1; } } @@ -143,9 +143,9 @@ static int test_wrong_pk(void) { // twice returncode = crypto_sign_open(sm, &mlen, sm, smlen, pk2); if (!returncode) { - printf("ERROR Signature did verify correctly under wrong public key!\n"); + fprintf(stderr, "ERROR Signature did verify correctly under wrong public key!\n"); if (returncode > 0) { - puts("ERROR return code should be < 0"); + fprintf(stderr, "ERROR return code should be < 0"); } return 1; } diff --git a/test/crypto_sign/testvectors.c b/test/crypto_sign/testvectors.c index 8388d8c1..dcbc4e38 100644 --- a/test/crypto_sign/testvectors.c +++ b/test/crypto_sign/testvectors.c @@ -32,6 +32,7 @@ int main(void) { uint8_t mi[MAXMLEN]; uint8_t sm[MAXMLEN + CRYPTO_BYTES]; + size_t smlen; size_t mlen; diff --git a/test/helpers.py b/test/helpers.py index 09a20985..db984484 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -1,5 +1,7 @@ +import functools import os import subprocess +import unittest def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): @@ -15,6 +17,7 @@ def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): # Note we need to capture stdout/stderr from the subprocess, # then print it, which nose/unittest will then capture and # buffer appropriately + print(working_dir + " > " + " ".join(command)) result = subprocess.run( command, stdout=subprocess.PIPE, @@ -22,7 +25,6 @@ def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): cwd=working_dir, env=env, ) - print(working_dir + " > " + " ".join(command)) print(result.stdout.decode('utf-8')) assert(result.returncode == expected_returncode) return result.stdout.decode('utf-8') @@ -35,13 +37,33 @@ def make(*args, working_dir='.', env=None, **kwargs): Usage: make('clean', 'targetb', SCHEME='bla') """ - make_command = 'make' + if os.name == 'nt': + make_command = ['nmake', '/f', 'Makefile.Microsoft_nmake', '/E'] + # we need SCHEME_UPPERCASE and IMPLEMENTATION_UPPERCASE with nmake + for envvar in ['IMPLEMENTATION', 'SCHEME']: + if envvar in kwargs: + kwargs['{}_UPPERCASE'.format(envvar)] = kwargs[envvar].upper().replace('-', '') + else: + make_command = ['make'] + return run_subprocess( [ - make_command, - *args, + *make_command, *['{}={}'.format(k, v) for k, v in kwargs.items()], + *args, ], working_dir=working_dir, env=env, ) + + +def skip_windows(message="This test is not supported on Windows"): + def wrapper(f): + @functools.wraps(f) + def skip_windows(*args, **kwargs): + raise unittest.SkipTest(message) + if os.name == 'nt': + return skip_windows + else: + return f + return wrapper \ No newline at end of file diff --git a/test/test_dynamic_memory.py b/test/test_dynamic_memory.py index f2fbb698..8853eaea 100644 --- a/test/test_dynamic_memory.py +++ b/test/test_dynamic_memory.py @@ -9,8 +9,6 @@ import unittest def test_dynamic_memory(): - if sys.platform not in ['linux', 'darwin']: - raise unittest.SkipTest() for scheme in pqclean.Scheme.all_schemes(): for implementation in scheme.implementations: # Keep this loop outside, to allow multiple assertions @@ -18,6 +16,7 @@ def test_dynamic_memory(): yield (check_dynamic_memory, implementation, function) +@helpers.skip_windows() def check_dynamic_memory(implementation, function): # 'make' will take care of not rebuilding existing library files helpers.make(working_dir=implementation.path()) diff --git a/test/test_format.py b/test/test_format.py index 6dc92084..bcc08f66 100644 --- a/test/test_format.py +++ b/test/test_format.py @@ -2,7 +2,7 @@ import os from glob import glob import pqclean -from helpers import run_subprocess +from helpers import run_subprocess, skip_windows def test_formatting(): @@ -11,6 +11,8 @@ def test_formatting(): yield check_format, implementation +@skip_windows(message="This test needs to be amended to work with Windows " + "installations of astyle") def check_format(implementation: pqclean.Implementation): cfiles = glob(os.path.join(implementation.path(), '*.c')) hfiles = glob(os.path.join(implementation.path(), '*.h')) diff --git a/test/test_functest.py b/test/test_functest.py index 0b0e3d9e..1ec92a8d 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -24,17 +24,22 @@ def test_functest_sanitizers(): def check_functest(implementation): - helpers.make(TYPE=implementation.scheme.type, + helpers.make(#'functest', + TYPE=implementation.scheme.type, SCHEME=implementation.scheme.name, IMPLEMENTATION=implementation.name, working_dir=os.path.join('..', 'test')) helpers.run_subprocess( - ['./functest_{}_{}'.format(implementation.scheme.name, - implementation.name)], + [os.path.join('..', 'bin', 'functest_{}_{}{}'.format( + implementation.scheme.name, + implementation.name, + '.exe' if os.name == 'nt' else '' + ))], os.path.join('..', 'bin'), ) +@helpers.skip_windows def check_functest_sanitizers(implementation): env = None if platform.machine() == 'ppc' and os.environ.get('CC', 'gcc') == 'clang': @@ -51,8 +56,11 @@ def check_functest_sanitizers(implementation): working_dir=os.path.join('..', 'test'), env=env) helpers.run_subprocess( - ['./functest_{}_{}'.format(implementation.scheme.name, - implementation.name)], + [os.path.join('..', 'bin', 'functest_{}_{}{}'.format( + implementation.scheme.name, + implementation.name, + '.exe' if os.name == 'nt' else '' + ))], os.path.join('..', 'bin'), env=env, ) From d503a712ba76ae64e9a0c2013b8b28404c85e3e0 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 16:53:38 +0100 Subject: [PATCH 02/15] Fix nmakefiles LIB is already an environment variable on Windows --- crypto_kem/kyber768/clean/Makefile.Microsoft_nmake | 10 +++++----- .../dilithium-iii/clean/Makefile.Microsoft_nmake | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake b/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake index bbf54ce0..13f435ee 100644 --- a/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake +++ b/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake @@ -1,16 +1,16 @@ # This Makefile can be used with Microsoft Visual Studio's nmake using the command: # nmake /f Makefile.Microsoft_nmake -LIB=libkyber768_clean.lib +LIBRARY=libkyber768_clean.lib OBJECTS=cbd.obj indcpa.obj kem.obj ntt.obj poly.obj polyvec.obj precomp.obj reduce.obj verify.obj CFLAGS=/I ..\..\..\common /W4 /WX -all: $(LIB) +all: $(LIBRARY) -$(LIB): $(OBJECTS) +$(LIBRARY): $(OBJECTS) LIB.EXE /OUT:$@ $** clean: - DEL $(OBJECTS) - DEL $(LIB) + -DEL $(OBJECTS) + -DEL $(LIBRARY) diff --git a/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake b/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake index 18cc6db9..12a49210 100644 --- a/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake +++ b/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake @@ -1,16 +1,16 @@ # This Makefile can be used with Microsoft Visual Studio's nmake using the command: # nmake /f Makefile.Microsoft_nmake -LIB=libdilithium-iii_clean.lib +LIBRARY=libdilithium-iii_clean.lib OBJECTS=ntt.obj packing.obj poly.obj polyvec.obj reduce.obj rounding.obj sign.obj CFLAGS=/I ..\..\..\common /W1 /WX # FIXME: ideally would use /W4 instead of /W1, but too many failures in Dilithium right now -all: $(LIB) +all: $(LIBRARY) -$(LIB): $(OBJECTS) +$(LIBRARY): $(OBJECTS) LIB.EXE /OUT:$@ $** clean: - DEL $(OBJECTS) - DEL $(LIB) + -DEL $(OBJECTS) + -DEL $(LIBRARY) From dc1f7e204a4dbcd649e21ff8d38a31bce84c1cc4 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 16:54:37 +0100 Subject: [PATCH 03/15] Add returncode support to helper.make --- test/helpers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/helpers.py b/test/helpers.py index db984484..476cc260 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -26,11 +26,12 @@ def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): env=env, ) print(result.stdout.decode('utf-8')) - assert(result.returncode == expected_returncode) + assert result.returncode == expected_returncode, \ + "Got unexpected return code {}".format(result.returncode) return result.stdout.decode('utf-8') -def make(*args, working_dir='.', env=None, **kwargs): +def make(*args, working_dir='.', env=None, expected_returncode=0, **kwargs): """ Runs a make target in the specified working directory @@ -54,6 +55,7 @@ def make(*args, working_dir='.', env=None, **kwargs): ], working_dir=working_dir, env=env, + expected_returncode=expected_returncode, ) From 13867ab7b47fb3ad8af3faa9f268987d6d9c99c5 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 16:56:05 +0100 Subject: [PATCH 04/15] Fix tests on Windows --- test/pqclean.py | 13 +++++++ test/test_makefile_dependencies.py | 61 ++++++++++++++++++++---------- test/test_testvectors.py | 12 ++++-- 3 files changed, 62 insertions(+), 24 deletions(-) diff --git a/test/pqclean.py b/test/pqclean.py index df909fd7..f3384dbe 100644 --- a/test/pqclean.py +++ b/test/pqclean.py @@ -1,4 +1,5 @@ import os +import glob import yaml @@ -73,8 +74,20 @@ class Implementation: return os.path.join(self.scheme.path(), self.name) def libname(self) -> str: + if os.name == 'nt': + return "lib{}_{}.lib".format(self.scheme.name, self.name) return "lib{}_{}.a".format(self.scheme.name, self.name) + def cfiles(self) -> [str]: + return glob.glob(os.path.join(self.path(), '*.c')) + + def hfiles(self) -> [str]: + return glob.glob(os.path.join(self.path(), '*.h')) + + def ofiles(self) -> [str]: + return glob.glob(os.path.join(self.path(), + '*.o' if os.name != 'nt' else '*.obj')) + @staticmethod def by_name(scheme_name, implementation_name): scheme = Scheme.by_name(scheme_name) diff --git a/test/test_makefile_dependencies.py b/test/test_makefile_dependencies.py index b416f94f..9045cb3f 100644 --- a/test/test_makefile_dependencies.py +++ b/test/test_makefile_dependencies.py @@ -19,41 +19,62 @@ def test_makefile_dependencies(): # test case for each candidate file cfiles = glob.glob(os.path.join(implementation.path(), '*.c')) hfiles = glob.glob(os.path.join(implementation.path(), '*.h')) - for file in cfiles + hfiles: + for file in (cfiles + hfiles): yield (check_makefile_dependencies, implementation, file) +def touch(time, *files): + if not files: + raise Exception("Please specify the files to update") + if os.name == 'nt': + formatstring = "Get-Date -year %Y -month %m -day %d -hour %H -minute %M -second %S" + time = time.strftime(formatstring) + commands = [] + for file in files: + commands.append('(ls {}).LastWriteTime = {}'.format(file, time)) + + helpers.run_subprocess(['powershell', '; '.join(commands)]) + else: + formatstring = "%Y%m%d%H%M.%S" + time = time.strftime(formatstring) + helpers.run_subprocess(['touch', '-t', time, *files]) + + +def make_check(path, expect_error=False): + makeflag = '-q' if os.name != 'nt' else '/Q' + expected_returncode = 0 + if expect_error: + expected_returncode = 1 if os.name != 'nt' else 255 + helpers.make(makeflag, working_dir=path, + expected_returncode=expected_returncode) + + def check_makefile_dependencies(implementation, file): - cfiles = glob.glob(os.path.join(implementation.path(), '*.c')) - hfiles = glob.glob(os.path.join(implementation.path(), '*.h')) - ofiles = glob.glob(os.path.join(implementation.path(), '*.o')) + cfiles = implementation.cfiles() + hfiles = implementation.hfiles() + ofiles = implementation.ofiles() libfile = os.path.join(implementation.path(), implementation.libname()) # modification time-based calculations is tricky on a sub-second basis # so we reset all the modification times to a known and "sensible" order - now = datetime.datetime.now() - ago15 = now - datetime.timedelta(seconds=15) - ago10 = now - datetime.timedelta(seconds=10) - ago5 = now - datetime.timedelta(seconds=5) - formatstring = "%Y%m%d%H%M.%S" - helpers.run_subprocess( - ['touch', '-t', ago15.strftime(formatstring)] + cfiles + hfiles) - helpers.run_subprocess( - ['touch', '-t', ago10.strftime(formatstring)] + ofiles) - helpers.run_subprocess( - ['touch', '-t', ago5.strftime(formatstring), libfile]) + now = datetime.datetime.now() - datetime.timedelta(seconds=10) + ago15 = now - datetime.timedelta(minutes=15) + ago10 = now - datetime.timedelta(minutes=10) + ago5 = now - datetime.timedelta(minutes=5) + + touch(ago15, *cfiles, *hfiles) + touch(ago10, *ofiles) + touch(ago5, libfile) # Sanity check: the scheme is up to date - helpers.run_subprocess(['make', '-q'], implementation.path(), - expected_returncode=0) + make_check(implementation.path()) # touch the candidate .c / .h file - helpers.run_subprocess(['touch', '-t', now.strftime(formatstring), file]) + touch(now, file) # check if it needs to be rebuilt using make -q - helpers.run_subprocess(['make', '-q'], implementation.path(), - expected_returncode=1) + make_check(implementation.path(), expect_error=True) if __name__ == '__main__': diff --git a/test/test_testvectors.py b/test/test_testvectors.py index 4f17c2aa..14afb92f 100644 --- a/test/test_testvectors.py +++ b/test/test_testvectors.py @@ -16,15 +16,19 @@ def test_testvectors(): def check_vectors(implementation): - helpers.make(TYPE=implementation.scheme.type, + helpers.make('testvectors', + TYPE=implementation.scheme.type, SCHEME=implementation.scheme.name, IMPLEMENTATION=implementation.name, working_dir=os.path.join('..', 'test')) out = helpers.run_subprocess( - ['./testvectors_{}_{}'.format(implementation.scheme.name, - implementation.name)], + [os.path.join('..', 'bin', 'testvectors_{}_{}{}'.format( + implementation.scheme.name, + implementation.name, + '.exe' if os.name == 'nt' else '' + ))], os.path.join('..', 'bin'), - ) + ).replace('\r', '') assert(implementation.scheme.metadata()['testvectors-sha256'].lower() == hashlib.sha256(out.encode('utf-8')).hexdigest().lower()) From f25824246f65729620e6ea5b78894134b1a78c13 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 16:56:40 +0100 Subject: [PATCH 05/15] small improvements of reporting --- test/test_functest.py | 2 +- test/test_metadata.py | 2 +- test/test_symbol_namespace.py | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/test/test_functest.py b/test/test_functest.py index 1ec92a8d..465b639a 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -43,7 +43,7 @@ def check_functest(implementation): def check_functest_sanitizers(implementation): env = None if platform.machine() == 'ppc' and os.environ.get('CC', 'gcc') == 'clang': - raise unittest.SkipTest() + raise unittest.SkipTest("Clang does not support ASAN on ppc") elif platform.machine() in ['armv7l', 'aarch64']: env = {'ASAN_OPTIONS': 'detect_leaks=0'} else: diff --git a/test/test_metadata.py b/test/test_metadata.py index b5112576..71ca7798 100644 --- a/test/test_metadata.py +++ b/test/test_metadata.py @@ -23,7 +23,7 @@ def check_metadata(scheme): specification = itertools.chain(specification, SIGNATURE_FIELDS.items()) else: - assert(False) + assert False, "Wrong type of metadata" check_spec(copy.deepcopy(metadata), specification) diff --git a/test/test_symbol_namespace.py b/test/test_symbol_namespace.py index 654d2dda..5d6ea9a0 100644 --- a/test/test_symbol_namespace.py +++ b/test/test_symbol_namespace.py @@ -10,18 +10,17 @@ import unittest def test_symbol_namespace(): - if sys.platform not in ['linux', 'darwin']: - raise unittest.SkipTest() for scheme in pqclean.Scheme.all_schemes(): for implementation in scheme.implementations: yield check_symbol_namespace, implementation def check_symbol_namespace(implementation): + if sys.platform not in ['linux', 'darwin']: + raise unittest.SkipTest("Unsupported platform") helpers.make(working_dir=implementation.path()) out = helpers.run_subprocess( - ['nm', '-g', 'lib{}_{}.a'.format(implementation.scheme.name, - implementation.name)], + ['nm', '-g', implementation.libname()], implementation.path() ) From 89e7383ecc6d301fbcb3d7dfcbb0d434d50e213b Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Mon, 4 Mar 2019 17:01:27 +0100 Subject: [PATCH 06/15] Add Python tests to Appveyor --- appveyor.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index bbfc9e6a..fe0b7ccf 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -7,6 +7,9 @@ build: init: - call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat" + - set PATH="C:\\Python37";%PATH% build_script: - - scripts_windows\build_all.bat + - python -m pip install -r requirements.txt + - cd test + - python -m nose -v --rednose From 45e645c3a7f633fc395cf126e9b05a0e5bc9c447 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 11:45:00 +0100 Subject: [PATCH 07/15] Fix Makefile.Microsoft_nmake header dependencies --- crypto_kem/kyber768/clean/Makefile.Microsoft_nmake | 7 +++++-- crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake | 7 +++++-- scripts_windows/build_all.bat | 4 ++-- test/Makefile.Microsoft_nmake | 2 +- test/helpers.py | 3 ++- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake b/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake index 13f435ee..eb7bb622 100644 --- a/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake +++ b/crypto_kem/kyber768/clean/Makefile.Microsoft_nmake @@ -4,12 +4,15 @@ LIBRARY=libkyber768_clean.lib OBJECTS=cbd.obj indcpa.obj kem.obj ntt.obj poly.obj polyvec.obj precomp.obj reduce.obj verify.obj -CFLAGS=/I ..\..\..\common /W4 /WX +CFLAGS=/nologo /I ..\..\..\common /W4 /WX all: $(LIBRARY) +# Make sure objects are recompiled if headers change. +$(OBJECTS): *.h + $(LIBRARY): $(OBJECTS) - LIB.EXE /OUT:$@ $** + LIB.EXE /NOLOGO /WX /OUT:$@ $** clean: -DEL $(OBJECTS) diff --git a/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake b/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake index 12a49210..bff95f50 100644 --- a/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake +++ b/crypto_sign/dilithium-iii/clean/Makefile.Microsoft_nmake @@ -4,12 +4,15 @@ LIBRARY=libdilithium-iii_clean.lib OBJECTS=ntt.obj packing.obj poly.obj polyvec.obj reduce.obj rounding.obj sign.obj -CFLAGS=/I ..\..\..\common /W1 /WX # FIXME: ideally would use /W4 instead of /W1, but too many failures in Dilithium right now +CFLAGS=/nologo /I ..\..\..\common /W1 /WX # FIXME: ideally would use /W4 instead of /W1, but too many failures in Dilithium right now all: $(LIBRARY) +# Make sure objects are recompiled if headers change. +$(OBJECTS): *.h + $(LIBRARY): $(OBJECTS) - LIB.EXE /OUT:$@ $** + LIB.EXE /NOLOGO /WX /OUT:$@ $** clean: -DEL $(OBJECTS) diff --git a/scripts_windows/build_all.bat b/scripts_windows/build_all.bat index 39393ce1..297fdcac 100644 --- a/scripts_windows/build_all.bat +++ b/scripts_windows/build_all.bat @@ -27,9 +27,9 @@ FOR %%T IN (kem sign) DO ( CD %%K FOR /D %%L IN (*) DO ( CD ..\..\test - nmake /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L clean + nmake /NOLOGO /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L clean IF ERRORLEVEL 1 SET EL=4 - nmake /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L + nmake /NOLOGO /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L IF ERRORLEVEL 1 SET EL=5 CD .. bin\functest_%%K_%%L diff --git a/test/Makefile.Microsoft_nmake b/test/Makefile.Microsoft_nmake index 52616574..e0e057fb 100644 --- a/test/Makefile.Microsoft_nmake +++ b/test/Makefile.Microsoft_nmake @@ -17,7 +17,7 @@ COMMON_OBJECTS_NOPATH=fips202.obj sha2.obj DEST_DIR=..\bin -CFLAGS=/I $(COMMON_DIR) /W4 /WX +CFLAGS=/nologo /I $(COMMON_DIR) /W4 /WX all: $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).EXE $(DEST_DIR)\testvectors_$(SCHEME)_$(IMPLEMENTATION).EXE diff --git a/test/helpers.py b/test/helpers.py index 476cc260..0f043691 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -39,7 +39,8 @@ def make(*args, working_dir='.', env=None, expected_returncode=0, **kwargs): make('clean', 'targetb', SCHEME='bla') """ if os.name == 'nt': - make_command = ['nmake', '/f', 'Makefile.Microsoft_nmake', '/E'] + make_command = ['nmake', '/f', 'Makefile.Microsoft_nmake', + '/NOLOGO', '/E'] # we need SCHEME_UPPERCASE and IMPLEMENTATION_UPPERCASE with nmake for envvar in ['IMPLEMENTATION', 'SCHEME']: if envvar in kwargs: From a230c51cf5e97e1e02c1514f500d847168b0f680 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 12:48:59 +0100 Subject: [PATCH 08/15] Disable clang-tidy lint on windows.h include --- common/randombytes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/randombytes.c b/common/randombytes.c index 82f2107c..f3bbbc2f 100644 --- a/common/randombytes.c +++ b/common/randombytes.c @@ -33,6 +33,7 @@ THE SOFTWARE. #if defined(_WIN32) /* Windows */ +// NOLINTNEXTLINE(llvm-include-order): Include order required by Windows #include #include /* CryptAcquireContext, CryptGenRandom */ #endif /* defined(_WIN32) */ From 5fd11050ff1a54aeafca885ae287b92ccac08499 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 12:50:47 +0100 Subject: [PATCH 09/15] Remove google-readability-todo lint --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 9ec04c26..c871d305 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '*,-llvm-header-guard,-hicpp-*,-readability-function-size' +Checks: '*,-llvm-header-guard,-hicpp-*,-readability-function-size,-google-readability-todo' WarningsAsErrors: '*' HeaderFilterRegex: '.*' AnalyzeTemporaryDtors: false From f47a690837c21cc69b7265d014863369226f987c Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 13:01:01 +0100 Subject: [PATCH 10/15] Remove Windows build_all bat file --- scripts_windows/build_all.bat | 44 ----------------------------------- 1 file changed, 44 deletions(-) delete mode 100644 scripts_windows/build_all.bat diff --git a/scripts_windows/build_all.bat b/scripts_windows/build_all.bat deleted file mode 100644 index 297fdcac..00000000 --- a/scripts_windows/build_all.bat +++ /dev/null @@ -1,44 +0,0 @@ -@ECHO OFF -SETLOCAL EnableDelayedExpansion -SET EL=0 - -REM CALL "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat" -REM IF ERRORLEVEL 1 SET EL=1 - -:: Build the library files for individual implementations -FOR /D %%K IN (crypto_kem\* crypto_sign\*) DO ( - FOR /D %%L IN (%%K\*) DO ( - CD %%L - nmake /f Makefile.Microsoft_nmake clean - IF ERRORLEVEL 1 SET EL=2 - nmake /f Makefile.Microsoft_nmake - IF ERRORLEVEL 1 SET EL=3 - CD ..\..\.. - ) -) - -:: Build and run the functional tests and test vector programs for each implementation -FOR %%T IN (kem sign) DO ( - CD crypto_%%T - FOR /D %%K IN (*) DO ( - SET schemeuppercase=%%K - FOR %%B IN (A B C D E F G H I J K L M N O P Q R S T U V W X Y Z) DO SET "schemeuppercase=!schemeuppercase:%%B=%%B!" - SET "schemeuppercase=!schemeuppercase:-=!" - CD %%K - FOR /D %%L IN (*) DO ( - CD ..\..\test - nmake /NOLOGO /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L clean - IF ERRORLEVEL 1 SET EL=4 - nmake /NOLOGO /f Makefile.Microsoft_nmake /E TYPE=%%T SCHEME=%%K SCHEME_UPPERCASE=!schemeuppercase! IMPLEMENTATION=%%L - IF ERRORLEVEL 1 SET EL=5 - CD .. - bin\functest_%%K_%%L - IF ERRORLEVEL 1 SET EL=6 - CD crypto_%%T\%%K - ) - CD .. - ) - CD .. -) - -EXIT /b %EL% From 7e9cabdee24b1666c633170d1dfb00f7c99afc35 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 13:51:15 +0100 Subject: [PATCH 11/15] Try to build Windows on x32 and x64 --- appveyor.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index fe0b7ccf..3db0a57c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,8 +5,13 @@ image: Visual Studio 2017 build: verbosity: minimal +environment: + matrix: + - BITS: 64 + - BITS: 32 + init: - - call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat" + - call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars%BITS%.bat" - set PATH="C:\\Python37";%PATH% build_script: From ff186b3dd60fef16e34fa34fa3667ecfa184ea82 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 14:35:19 +0100 Subject: [PATCH 12/15] Run AStyle if it is installed Also try to install it on AppVeyor --- appveyor.yml | 7 ++++++- test/test_format.py | 14 +++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 3db0a57c..41bc098f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,7 +12,12 @@ environment: init: - call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars%BITS%.bat" - - set PATH="C:\\Python37";%PATH% + - set PATH="C:\\Python37";"C:\\Python37\Scripts";"C:\\tmp\\tools";%PATH% + - mkdir "C:\tmp\tools" + # Download AStyle 3.1: first enable strong crypto in Invoke-WebRequest + - ps: Set-ItemProperty -Path "HKLM:\SOFTWARE\Wow6432Node\Microsoft\.NetFramework\v4.0.30319" -Name 'SchUseStrongCrypto' -Value '1' -Type DWord + - ps: Set-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\.NetFramework\v4.0.30319" -Name "SchUseStrongCrypto" -Value '1' -Type DWord + - ps: Invoke-WebRequest -OutFile "C:\tmp\tools\AStyle.exe" "https://rded.nl/pqclean/AStyle.exe" build_script: - python -m pip install -r requirements.txt diff --git a/test/test_format.py b/test/test_format.py index bcc08f66..e94e3deb 100644 --- a/test/test_format.py +++ b/test/test_format.py @@ -1,8 +1,8 @@ -import os -from glob import glob +import shutil +import unittest import pqclean -from helpers import run_subprocess, skip_windows +from helpers import run_subprocess def test_formatting(): @@ -11,11 +11,11 @@ def test_formatting(): yield check_format, implementation -@skip_windows(message="This test needs to be amended to work with Windows " - "installations of astyle") def check_format(implementation: pqclean.Implementation): - cfiles = glob(os.path.join(implementation.path(), '*.c')) - hfiles = glob(os.path.join(implementation.path(), '*.h')) + if shutil.which('astyle') is None: + raise unittest.SkipTest("AStyle is not installed") + cfiles = implementation.cfiles() + hfiles = implementation.hfiles() run_subprocess(['astyle', '--dry-run', '--options=../.astylerc', From fe1ba0e6151243be772a45900ad346b4bd2414bd Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 14:36:28 +0100 Subject: [PATCH 13/15] Disable AStyle line endings formatter Git takes care of this, and it breaks the checkouts on Windows --- .astylerc | 1 - 1 file changed, 1 deletion(-) diff --git a/.astylerc b/.astylerc index cba802e4..21047d05 100644 --- a/.astylerc +++ b/.astylerc @@ -10,6 +10,5 @@ --add-braces --convert-tabs --mode=c ---lineend=linux # disable backup files --suffix=none From e450cd60424061ab0b10e94b8e3973d0b5793c6a Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Tue, 5 Mar 2019 15:45:09 +0100 Subject: [PATCH 14/15] Set modification time via os.utime Hopefully quicker on Windows --- test/test_makefile_dependencies.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/test/test_makefile_dependencies.py b/test/test_makefile_dependencies.py index 9045cb3f..86730890 100644 --- a/test/test_makefile_dependencies.py +++ b/test/test_makefile_dependencies.py @@ -24,20 +24,9 @@ def test_makefile_dependencies(): def touch(time, *files): - if not files: - raise Exception("Please specify the files to update") - if os.name == 'nt': - formatstring = "Get-Date -year %Y -month %m -day %d -hour %H -minute %M -second %S" - time = time.strftime(formatstring) - commands = [] - for file in files: - commands.append('(ls {}).LastWriteTime = {}'.format(file, time)) - - helpers.run_subprocess(['powershell', '; '.join(commands)]) - else: - formatstring = "%Y%m%d%H%M.%S" - time = time.strftime(formatstring) - helpers.run_subprocess(['touch', '-t', time, *files]) + for path in files: + times = (time.timestamp(), time.timestamp()) + os.utime(path, times) def make_check(path, expect_error=False): From 1399c7fd1f69c425491e6449cfdf8bf4e072edde Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 6 Mar 2019 17:24:02 +0100 Subject: [PATCH 15/15] Only ignore missing helper programs if not on CI use randombytes in functest Got dropped by the rebase Work around missing clang-tidy on Travis Also check if valgrind is available --- .circleci/config.yml | 2 +- appveyor.yml | 8 +++++--- test/Makefile.Microsoft_nmake | 2 +- test/helpers.py | 32 ++++++++++++++++++++++++++++++-- test/test_format.py | 8 ++------ test/test_functest.py | 4 +++- test/test_linter.py | 7 ++----- 7 files changed, 44 insertions(+), 19 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 810608fa..17f13e49 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -10,7 +10,7 @@ version: 2 - run: name: Run the tests in a container command: | - docker run --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c " + docker run -e CI=true --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c " uname -a && export CC=${CC} && cd test && python3 -m nose --rednose --verbose" diff --git a/appveyor.yml b/appveyor.yml index 41bc098f..956ad13d 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,14 +12,16 @@ environment: init: - call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars%BITS%.bat" - - set PATH="C:\\Python37";"C:\\Python37\Scripts";"C:\\tmp\\tools";%PATH% - - mkdir "C:\tmp\tools" # Download AStyle 3.1: first enable strong crypto in Invoke-WebRequest - ps: Set-ItemProperty -Path "HKLM:\SOFTWARE\Wow6432Node\Microsoft\.NetFramework\v4.0.30319" -Name 'SchUseStrongCrypto' -Value '1' -Type DWord - ps: Set-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\.NetFramework\v4.0.30319" -Name "SchUseStrongCrypto" -Value '1' -Type DWord - - ps: Invoke-WebRequest -OutFile "C:\tmp\tools\AStyle.exe" "https://rded.nl/pqclean/AStyle.exe" + # Add Python to PATH + - set PATH="C:\\Python37";"C:\\Python37\Scripts";%PATH% build_script: - python -m pip install -r requirements.txt - cd test + # Download Astyle to local folder because putting it in PATH doesn't work + - ps: Invoke-WebRequest -OutFile "astyle.exe" "https://rded.nl/pqclean/AStyle.exe" + # Run tests - python -m nose -v --rednose diff --git a/test/Makefile.Microsoft_nmake b/test/Makefile.Microsoft_nmake index e0e057fb..21afb364 100644 --- a/test/Makefile.Microsoft_nmake +++ b/test/Makefile.Microsoft_nmake @@ -39,7 +39,7 @@ $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).exe: build-scheme $(COMMON_OBJE -MKDIR $(DEST_DIR) -DEL functest.obj $(CC) /c crypto_$(TYPE)\functest.c $(CFLAGS) /I $(SCHEME_DIR) /DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) - LINK.EXE /OUT:$@ functest.obj $(COMMON_OBJECTS_NOPATH) notrandombytes.obj $(SCHEME_DIR)\lib$(SCHEME)_$(IMPLEMENTATION).lib Advapi32.lib + LINK.EXE /OUT:$@ functest.obj $(COMMON_OBJECTS_NOPATH) randombytes.obj $(SCHEME_DIR)\lib$(SCHEME)_$(IMPLEMENTATION).lib Advapi32.lib $(DEST_DIR)\testvectors_$(SCHEME)_$(IMPLEMENTATION).exe: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\notrandombytes.obj -MKDIR $(DEST_DIR) diff --git a/test/helpers.py b/test/helpers.py index 0f043691..238e234c 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -2,6 +2,8 @@ import functools import os import subprocess import unittest +import shutil +import sys def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): @@ -44,7 +46,8 @@ def make(*args, working_dir='.', env=None, expected_returncode=0, **kwargs): # we need SCHEME_UPPERCASE and IMPLEMENTATION_UPPERCASE with nmake for envvar in ['IMPLEMENTATION', 'SCHEME']: if envvar in kwargs: - kwargs['{}_UPPERCASE'.format(envvar)] = kwargs[envvar].upper().replace('-', '') + kwargs['{}_UPPERCASE'.format(envvar)] = ( + kwargs[envvar].upper().replace('-', '')) else: make_command = ['make'] @@ -69,4 +72,29 @@ def skip_windows(message="This test is not supported on Windows"): return skip_windows else: return f - return wrapper \ No newline at end of file + return wrapper + + +def ensure_available(executable): + """ + Checks if a command is available. + + If a command MUST be available, because we are in a CI environment, + raises an AssertionError. + + In the docker containers, on Travis and on Windows, CI=true is set. + """ + path = shutil.which(executable) + if path: + print("Found", path) + return path + + # Installing clang-tidy on LLVM will be too much of a mess. + if ((executable == 'clang-tidy' and sys.platform == 'darwin') + or 'CI' not in os.environ): + raise unittest.SkipTest( + "{} is not available on PATH. Install it to run this test.{}" + .format(executable, "" if not os.name == 'nt' + else "On Windows, make sure to add it to PATH") + ) + raise AssertionError("{} not available on CI".format(executable)) diff --git a/test/test_format.py b/test/test_format.py index e94e3deb..09cc61fd 100644 --- a/test/test_format.py +++ b/test/test_format.py @@ -1,8 +1,5 @@ -import shutil -import unittest - import pqclean -from helpers import run_subprocess +from helpers import run_subprocess, ensure_available def test_formatting(): @@ -12,8 +9,7 @@ def test_formatting(): def check_format(implementation: pqclean.Implementation): - if shutil.which('astyle') is None: - raise unittest.SkipTest("AStyle is not installed") + ensure_available('astyle') cfiles = implementation.cfiles() hfiles = implementation.hfiles() run_subprocess(['astyle', diff --git a/test/test_functest.py b/test/test_functest.py index 465b639a..01807e75 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -24,7 +24,7 @@ def test_functest_sanitizers(): def check_functest(implementation): - helpers.make(#'functest', + helpers.make('functest', TYPE=implementation.scheme.type, SCHEME=implementation.scheme.name, IMPLEMENTATION=implementation.name, @@ -48,6 +48,8 @@ def check_functest_sanitizers(implementation): env = {'ASAN_OPTIONS': 'detect_leaks=0'} else: print("Supported platform: {}".format(platform.machine())) + + helpers.ensure_available('valgrind') helpers.make('clean-scheme', 'functest', TYPE=implementation.scheme.type, SCHEME=implementation.scheme.name, diff --git a/test/test_linter.py b/test/test_linter.py index 8d6b6839..33563f8b 100644 --- a/test/test_linter.py +++ b/test/test_linter.py @@ -1,10 +1,8 @@ import os from glob import glob -import shutil -import unittest import pqclean -from helpers import run_subprocess +from helpers import run_subprocess, ensure_available def test_clang_tidy(): @@ -14,8 +12,7 @@ def test_clang_tidy(): def check_tidy(implementation: pqclean.Implementation): - if shutil.which('clang-tidy') is None: - raise unittest.SkipTest("clang-tidy unavailable in PATH") + ensure_available('clang-tidy') cfiles = glob(os.path.join(implementation.path(), '*.c')) common_files = glob(os.path.join('..', 'common', '*.c')) run_subprocess(['clang-tidy',