diff --git a/.github/workflows/baremetal.yml b/.github/workflows/baremetal.yml index 7269f44dc9..c3016cfea6 100644 --- a/.github/workflows/baremetal.yml +++ b/.github/workflows/baremetal.yml @@ -17,7 +17,7 @@ jobs: matrix: target: - runner: ubuntu-latest - name: 'AVR ATmega128RFR2 (modified for 32K RAM)' + name: 'AVR ATmega128RFR2 (modified for 63.5K RAM)' makefile: test/baremetal/platform/avr/platform.mk nix-shell: cross-avr func: true diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index 92cf9b60a4..93c13820ce 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -67,7 +67,7 @@ jobs: name: 'aarch64' - runner: ubuntu-latest name: 'x86_64' - acvp-version: [v1.1.0.40, v1.1.0.41, v1.1.0.42] + acvp-version: [v1.1.0.41, v1.1.0.42, v1.1.0.43] exclude: - {external: true, target: { diff --git a/nix/avr/default.nix b/nix/avr/default.nix index 714de72149..66333e12a5 100644 --- a/nix/avr/default.nix +++ b/nix/avr/default.nix @@ -7,9 +7,9 @@ let # Patched simavr with increased RAM and fixed UART output simavr-patched = pkgs.simavr.overrideAttrs (oldAttrs: { patches = (oldAttrs.patches or [ ]) ++ [ - ./simavr-32kb-ram.patch + ./simavr-64kb-ram.patch ./simavr-uart-output-fix.patch - ./simavr-16k-eeprom.patch + ./simavr-32k-eeprom.patch # Exit-code commands (SIMAVR_CMD_EXIT_CODE_*), not yet in a release (pkgs.fetchpatch { url = "https://github.com/buserror/simavr/commit/c9354b32e057e409c2fbc9454e26db3b3103c26a.patch"; diff --git a/nix/avr/simavr-16k-eeprom.patch b/nix/avr/simavr-32k-eeprom.patch similarity index 87% rename from nix/avr/simavr-16k-eeprom.patch rename to nix/avr/simavr-32k-eeprom.patch index a0cbfc1a8c..d0590d79c1 100644 --- a/nix/avr/simavr-16k-eeprom.patch +++ b/nix/avr/simavr-32k-eeprom.patch @@ -6,4 +6,4 @@ index 1234567..abcdefg 100644 +++ b/simavr/cores/avr/iom128rfr2.h @@ -1,1 +1,1 @@ -#define E2END (0xFFF) -+#define E2END (0x3FFF) ++#define E2END (0x7FFF) diff --git a/nix/avr/simavr-32kb-ram.patch b/nix/avr/simavr-64kb-ram.patch similarity index 84% rename from nix/avr/simavr-32kb-ram.patch rename to nix/avr/simavr-64kb-ram.patch index 580dc8a4d2..47c26a32fb 100644 --- a/nix/avr/simavr-32kb-ram.patch +++ b/nix/avr/simavr-64kb-ram.patch @@ -8,8 +8,8 @@ #define RAMSTART (0x200) -#define RAMSIZE (0x4000) -#define RAMEND (0x41FF) -+#define RAMSIZE (0x8000) -+#define RAMEND (0x81FF) ++#define RAMSIZE (0xFE00) ++#define RAMEND (0xFFFF) #define XRAMSTART (0x0000) #define XRAMSIZE (0x0000) #define XRAMEND RAMEND diff --git a/scripts/tests b/scripts/tests index b5301667b4..db35abee74 100755 --- a/scripts/tests +++ b/scripts/tests @@ -1353,8 +1353,8 @@ def cli(): ) acvp_parser.add_argument( "--version", - default="v1.1.0.41", - help="ACVP test vector version (default: v1.1.0.41)", + default="v1.1.0.43", + help="ACVP test vector version (default: v1.1.0.43)", ) # wycheproof arguments diff --git a/test/acvp/acvp_client.py b/test/acvp/acvp_client.py index 0c31b3a10a..a585c47ca8 100755 --- a/test/acvp/acvp_client.py +++ b/test/acvp/acvp_client.py @@ -20,6 +20,16 @@ exec_prefix = exec_prefix.split(" ") if exec_prefix != "" else [] +def acvp_version_has_tr1(version): + """Whether `version` ships the FIPS203-tr1 encapDecap vectors (v1.1.0.43+).""" + try: + parts = tuple(int(x) for x in version.lstrip("v").split(".")) + except ValueError: + # Non-numeric ref (branch or commit); assume the vectors are present. + return True + return parts >= (1, 1, 0, 43) + + def download_acvp_files(version): """Download ACVP test files for the specified version if not present.""" base_url = f"https://raw.githubusercontent.com/usnistgov/ACVP-Server/{version}/gen-val/json-files" @@ -32,6 +42,13 @@ def download_acvp_files(version): "ML-KEM-encapDecap-FIPS203/expectedResults.json", ] + # The FIPS203-tr1 encapDecap vectors add seed/expanded key-format groups. + if acvp_version_has_tr1(version): + files_to_download += [ + "ML-KEM-encapDecap-FIPS203-tr1/prompt.json", + "ML-KEM-encapDecap-FIPS203-tr1/expectedResults.json", + ] + # Create directory structure data_dir = Path(f"test/acvp/.acvp-data/{version}/files") data_dir.mkdir(parents=True, exist_ok=True) @@ -63,6 +80,38 @@ def download_acvp_files(version): return True +def unwrap_acvts(data): + # ACVTS files wrap the payload as [{"acvVersion": ...}, {...}]. + return data[1] if isinstance(data, list) else data + + +def drop_keyless_decap_key_checks(data): + """Drop decapsulationKeyCheck cases that provide no key. + + Some ML-KEM sample vectors omit dk for these cases; the check cannot run + without a key. Removed from prompt and expected so the comparison stays + consistent, and runs normally once a key is present. + """ + dropped = 0 + for _, promptData, _, expectedData in data: + drop = set() + for tg in unwrap_acvts(promptData).get("testGroups", []): + if tg.get("function") != "decapsulationKeyCheck": + continue + for tc in tg["tests"]: + if "dk" not in tc: + drop.add((tg["tgId"], tc["tcId"])) + for d in (promptData, expectedData): + if d is None: + continue + for tg in unwrap_acvts(d).get("testGroups", []): + tg["tests"] = [ + tc for tc in tg["tests"] if (tg["tgId"], tc["tcId"]) not in drop + ] + dropped += len(drop) + return dropped + + def loadAcvpData(prompt, expectedResults): with open(prompt, "r") as f: promptData = json.load(f) @@ -86,6 +135,17 @@ def loadDefaultAcvpData(version): f"{data_dir}/ML-KEM-encapDecap-FIPS203/expectedResults.json", ), ] + + # FIPS203-tr1 encapDecap vectors (seed/expanded key formats) exist from + # v1.1.0.43. + if acvp_version_has_tr1(version): + acvp_jsons_for_version.append( + ( + f"{data_dir}/ML-KEM-encapDecap-FIPS203-tr1/prompt.json", + f"{data_dir}/ML-KEM-encapDecap-FIPS203-tr1/expectedResults.json", + ) + ) + acvp_data = [] for prompt, expectedResults in acvp_jsons_for_version: acvp_data.append(loadAcvpData(prompt, expectedResults)) @@ -139,12 +199,18 @@ def run_encapDecap_test(tg, tc): results[k] = v elif tg["function"] == "decapsulation": acvp_bin = get_acvp_binary(tg) + # keyFormat 'seed' provides d and z to expand into the key; 'expanded' + # (or absent) provides the expanded dk directly. + if tg.get("keyFormat") == "seed": + key_arg = f"seed={tc['d'] + tc['z']}" + else: + key_arg = f"dk={tc['dk']}" acvp_call = exec_prefix + [ acvp_bin, "encapDecap", "VAL", "decapsulation", - f"dk={tc['dk']}", + key_arg, f"c={tc['c']}", ] result = subprocess.run(acvp_call, encoding="utf-8", capture_output=True) @@ -319,6 +385,10 @@ def test(prompt, expected, output, version): # load data from downloaded files data = loadDefaultAcvpData(version) + dropped = drop_keyless_decap_key_checks(data) + if dropped: + info(f"Skipping {dropped} decapsulationKeyCheck case(s) with no key") + runTest(data, output) @@ -338,8 +408,8 @@ def test(prompt, expected, output, version): parser.add_argument( "--version", "-v", - default="v1.1.0.41", - help="ACVP test vector version (default: v1.1.0.41)", + default="v1.1.0.43", + help="ACVP test vector version (default: v1.1.0.43)", ) args = parser.parse_args() diff --git a/test/acvp/acvp_mlkem.c b/test/acvp/acvp_mlkem.c index 277b699f86..d3649b3083 100644 --- a/test/acvp/acvp_mlkem.c +++ b/test/acvp/acvp_mlkem.c @@ -119,6 +119,38 @@ static int decode_hex(const char *prefix, unsigned char *out, size_t out_len, return 1; } +/* + * Decode the decapsulation-key argument into dk. It is either the expanded + * key ("dk=HEX", keyFormat 'expanded') or a seed d||z to expand via keyGen + * ("seed=HEX", keyFormat 'seed'). Returns 0 on success, 1 on failure. + * MLK_NOINLINE keeps the keyGen scratch (ek) out of the caller's (main's) + * stack frame; under -fsanitize=undefined it would not share slots and would + * overflow AVR RAM. + */ +static MLK_NOINLINE int decode_dk(const char *arg, + unsigned char dk[CRYPTO_SECRETKEYBYTES]) +{ + size_t seed_len = strlen("seed="); + + /* Prefix check via memcmp; strncmp is unavailable on baremetal builds. */ + if (strlen(arg) >= seed_len && memcmp(arg, "seed=", seed_len) == 0) + { + unsigned char ek[CRYPTO_PUBLICKEYBYTES]; + unsigned char coins[2 * MLKEM_SYMBYTES]; + if (decode_hex("seed", coins, sizeof(coins), arg) != 0) + { + return 1; + } + if (crypto_kem_keypair_derand(ek, dk, coins) != 0) + { + fprintf(stderr, "Failed to expand seed into decapsulation key\n"); + return 1; + } + return 0; + } + return decode_hex("dk", dk, CRYPTO_SECRETKEYBYTES, arg); +} + static void print_hex(const char *name, const unsigned char *raw, size_t len) { if (name != NULL) @@ -321,8 +353,8 @@ int main(int argc, char *argv[]) goto decaps_usage; } - /* Parse dk */ - if (argc == 0 || decode_hex("dk", dk, sizeof(dk), *argv) != 0) + /* Parse dk (expanded key, or a seed to expand) */ + if (argc == 0 || decode_dk(*argv, dk) != 0) { goto decaps_usage; } diff --git a/test/baremetal/platform/avr/README.md b/test/baremetal/platform/avr/README.md index 3ee640160d..3e94e3c44c 100644 --- a/test/baremetal/platform/avr/README.md +++ b/test/baremetal/platform/avr/README.md @@ -6,4 +6,4 @@ This directory provides a baremetal build and test environment for AVR MCUs, usi This is primarily a vehicle to test that mlkem-native builds and is functionally correct in 16-bit C implementations. For actual practical use on 16-bit MCUs, stack usage would need to be reduced. -**Note:** We currently need 32K of RAM, more than any MCU supported by `simavr`; we therefore use a patched version of `simavr` where Atmega128rfr2 is given 32K of RAM. To test this, you must work in the `nix .#avr` shell specified in nix flake. +**Note:** We currently need close to the full 64K data address space of the AVR architecture, more than any MCU supported by `simavr`; we therefore use a patched version of `simavr` where Atmega128rfr2 is given 63.5K of RAM (0x0200-0xFFFF). To test this, you must work in the `nix .#cross-avr` shell specified in nix flake. diff --git a/test/baremetal/platform/avr/avr_wrapper.c b/test/baremetal/platform/avr/avr_wrapper.c index dabb8ab2bc..90ea1b9300 100644 --- a/test/baremetal/platform/avr/avr_wrapper.c +++ b/test/baremetal/platform/avr/avr_wrapper.c @@ -14,7 +14,17 @@ /* Register for sending commands (e.g. exit codes) to simavr */ AVR_MCU_SIMAVR_COMMAND(&GPIOR0); -#define RAM_BASE 0x2000 +/* The argc/argv block is placed at the top of RAM, just below 16 bytes of + * scratch stack used during startup. The exec wrapper chooses the base + * address RAM_TOP - blocksize and stores it in the first two bytes of + * EEPROM, followed by the block itself. The stack grows downwards from + * just below the block, so binaries with little argument data + * automatically get the largest possible stack. */ +#define RAM_TOP 0xFFF0 + +/* Base address of the argc/argv block, read from EEPROM. Used by the + * argc/argv register setup in init7.S. */ +uint16_t mlk_argv_base; static int uart_putchar(char c, FILE *stream) { @@ -27,11 +37,22 @@ static int uart_putchar(char c, FILE *stream) /* Set up stdout stream for avr-libc printf */ static FILE mystdout = FDEV_SETUP_STREAM(uart_putchar, NULL, _FDEV_SETUP_WRITE); -/* Init6 function - copy EEPROM to RAM */ +/* Init6 function - copy argc/argv block from EEPROM to top of RAM and + * point the stack just below it. The scratch stack above RAM_TOP is used + * for the eeprom_read_* calls; the default stack location (RAMEND of the + * unpatched MCU) may lie inside .data/.bss and must not be used. */ void setup_args(void) __attribute__((naked, section(".init6"), used)); void setup_args(void) { - eeprom_read_block((void *)RAM_BASE, (void *)0x0000, 0x4000); + uint16_t base; + SPH = 0xFF; + SPL = 0xFF; + base = eeprom_read_word((const uint16_t *)0); + eeprom_read_block((void *)base, (const void *)2, (size_t)(RAM_TOP - base)); + mlk_argv_base = base; + base--; + SPH = (uint8_t)(base >> 8); + SPL = (uint8_t)(base & 0xFF); } /* This is run as part of the init sequence, after setting up the stack diff --git a/test/baremetal/platform/avr/exec_wrapper.py b/test/baremetal/platform/avr/exec_wrapper.py index 23bde2b32a..897b4c4738 100755 --- a/test/baremetal/platform/avr/exec_wrapper.py +++ b/test/baremetal/platform/avr/exec_wrapper.py @@ -7,6 +7,14 @@ import os import tempfile +# The argc/argv block is placed at the top of RAM, just below 16 bytes of +# scratch stack used during startup (see avr_wrapper.c). The stack grows +# downwards from just below the block. +RAM_TOP = 0xFFF0 + +# Patched EEPROM size (see nix/avr/simavr-32k-eeprom.patch) +EEPROM_SIZE = 0x8000 + def intel_hex_line(addr, data): """Generate Intel HEX format line""" @@ -25,6 +33,10 @@ def intel_hex_line(addr, data): def create_eeprom_hex(args, output_file): """ Create EEPROM hex file from command line arguments. + + EEPROM layout: 2 bytes block base address (little-endian), followed by + the argc/argv block to be copied to that address: + argc (2 bytes) + argv array (len(args) * 2 bytes) + packed strings. """ # First arg should be binary name (strip path) args = [os.path.basename(args[0])] + args[1:] @@ -38,15 +50,23 @@ def create_eeprom_hex(args, output_file): strings_data.extend(arg.encode("utf-8")) strings_data.append(0x00) # Null terminator - # Step 2: Calculate where strings will be in RAM - # Layout: argc (2 bytes) + argv array (len(args) * 2 bytes) + strings + # Step 2: Calculate where the block will be in RAM argc_size = 2 argv_size = len(args) * 2 - strings_ram_base = 0x2000 + argc_size + argv_size + block_size = argc_size + argv_size + len(strings_data) + if block_size + 2 > EEPROM_SIZE: + print( + f"Error: argument block of {block_size} bytes does not fit in EEPROM", + file=sys.stderr, + ) + sys.exit(1) + base = RAM_TOP - block_size + strings_ram_base = base + argc_size + argv_size - # Step 3: Build data starting with argc + # Step 3: Build EEPROM data: block base address, then argc data = bytearray() - data.extend([len(args) & 0xFF, (len(args) >> 8) & 0xFF]) # argc (little-endian) + data.extend([base & 0xFF, (base >> 8) & 0xFF]) + data.extend([len(args) & 0xFF, (len(args) >> 8) & 0xFF]) # Step 4: Build argv array with pointers to RAM addresses for offset in string_offsets: @@ -96,7 +116,7 @@ def main(): # Run with simavr - enable UART output # Note that we use a patched version of simavr where atmega128rfr2 - # has 32K of RAM. This is purely for testing purposes. + # has 63.5K of RAM. This is purely for testing purposes. cmd = [ "simavr", "-m", @@ -126,12 +146,12 @@ def main(): filtered_lines.append(clean_line) output = "\n".join(filtered_lines) - # simavr does not propagate the guest exit code (returncode is always - # 0), so a guest-side failure would otherwise be reported as success. - # As a stopgap until simavr's exit-code mechanism is backported (#1728), - # scan the output for "ERROR" and fail instead. This catches both - # the UBSan-trap abort() in avr_wrapper.c and the CHECK(...) macros in - # the tests, which print "ERROR (file,line)" on failure. + # The guest exit code is propagated via simavr's exit-code commands + # (see avr_wrapper.c). As an additional safety net, scan the output + # for "ERROR" and fail: this catches failures where the firmware + # stops without reaching exit(), and both the UBSan-trap abort() in + # avr_wrapper.c and the CHECK(...) macros in the tests print + # "ERROR (file,line)" on failure. # # On failure, write to stderr, otherwise stdout. if "ERROR" in output: diff --git a/test/baremetal/platform/avr/init7.S b/test/baremetal/platform/avr/init7.S index 56db0aff2f..6ef84c476e 100644 --- a/test/baremetal/platform/avr/init7.S +++ b/test/baremetal/platform/avr/init7.S @@ -7,16 +7,18 @@ /* AVR calling convention for main(int argc, char** argv): * - argc (int, 16-bit) goes in r24:r25 (r24=low, r25=high) * - argv (char**, 16-bit pointer) goes in r22:r23 (r22=low, r23=high) + * + * The argc/argv block was copied to mlk_argv_base by setup_args (init6): + * argc at mlk_argv_base, argv array at mlk_argv_base + 2. */ -/* Load argc from 0x2000 */ -ldi r30, 0x00 -ldi r31, 0x20 +lds r30, mlk_argv_base +lds r31, mlk_argv_base+1 ld r24, Z ldi r25, 0 -/* Set argv = 0x2002 */ -ldi r22, lo8(0x2002) -ldi r23, hi8(0x2002) +movw r22, r30 +subi r22, lo8(-2) +sbci r23, hi8(-2) /* Fall through to init8 - no ret instruction */ diff --git a/test/baremetal/platform/avr/platform.mk b/test/baremetal/platform/avr/platform.mk index 8b702f0740..033584c08b 100644 --- a/test/baremetal/platform/avr/platform.mk +++ b/test/baremetal/platform/avr/platform.mk @@ -7,11 +7,10 @@ CROSS_PREFIX=avr- CC=gcc # AVR target configuration -# We would need ATMega256rfr2 with 32K RAM, but it's not supported by simavr. -# We instead use ATMega128rfr1 with 16K RAM, but modify its specification in -# simavr to bump the RAM to 32K. -# Once simavr supports ATMega256rfr2, or once mlkem-native has [an option for] -# lower stack usage, this should be changed. +# ML-KEM-1024 (together with the RAM-resident test vectors) needs more than +# the 16K RAM of any MCU supported by simavr, so we use ATMega128rfr2 with its +# RAM specification in simavr bumped from 16K to the maximal 63.5K +# (0x0200-0xFFFF). AVR_MCU ?= atmega128rfr2 AVR_FREQ ?= 16000000UL @@ -35,12 +34,16 @@ CFLAGS += \ CFLAGS += $(CFLAGS_EXTRA) -# Non-standard stack end: 0x81FF = 0x200 + 8K instead of default 0x41FF +# Memory layout (data address space 0x0200-0xFFFF): +# - .data/.bss grow upwards from 0x0200 (data region length raised from the +# default 16K to the full RAM size) +# - the argc/argv block sits at the top of RAM, with the stack growing +# downwards from just below it (set up at runtime, see avr_wrapper.c) LDFLAGS += \ -mmcu=$(AVR_MCU) \ -Wl,--gc-sections \ -Wl,--relax \ - -Wl,--defsym=__stack=0x81FF \ + -Wl,--defsym=__DATA_REGION_LENGTH__=0xFC00 \ -Wl,--undefined=_simavr_command_register \ -Wl,--section-start=.mmcu=0x910000 \ -lprintf_min