Re: [PATCH v2] tests: add functional tests for ecb/cbc helpers

Hi Glenn,
thanks for taking a look.

On 1/2/26 10:44 AM, Glenn Washburn wrote:
> On Mon,  8 Dec 2025 13:58:16 +0530
> Srish Srinivasan <[email protected]> wrote:
>
>> Test the following helper functions using AES with 128, 192, and
>> 256 bit keys:
>>
>> grub_crypto_ecb_encrypt
>> grub_crypto_ecb_decrypt
>> grub_crypto_cbc_encrypt
>> grub_crypto_cbc_decrypt
>>
>> Signed-off-by: Srish Srinivasan <[email protected]>
>> ---
>>   Link to v1: https://lists.gnu.org/archive/html/grub-devel/2025-11/msg00237.html
>>   docs/grub.texi                        |   5 +
>>   grub-core/Makefile.core.def           |   5 +
>>   grub-core/tests/ecb_cbc_test.c        | 197 ++++++++++++++++++++++++++
> I would prefer it be named more generally so the
> name still makes sense when/if different cipher mode tests are added,
> perhaps crypto_cipher_mode_test.c.


Sure, I will make the change.


>
>>   grub-core/tests/ecb_cbc_vectors.h     | 135 ++++++++++++++++++
>>   grub-core/tests/lib/functional_test.c |   1 +
>>   5 files changed, 343 insertions(+)
>>   create mode 100644 grub-core/tests/ecb_cbc_test.c
>>   create mode 100644 grub-core/tests/ecb_cbc_vectors.h
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 7181009b6..63a82bba2 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -4092,6 +4092,7 @@ Modules can be loaded via the @command{insmod} (@pxref{insmod}) command.
>>   * dm_nv_module::
>>   * drivemap_module::
>>   * dsa_sexp_test_module::
>> +* ecb_cbc_test_module::
>>   * echo_module::
>>   * efi_gop_module::
>>   * efi_uga_module::
>> @@ -4677,6 +4678,10 @@ mappings. @xref{drivemap} for more information.
>>   @section dsa_sexp_test
>>   This module provides a test of the libgcrypt DSA functionality in GRUB.
>>   
>> +@node ecb_cbc_test_module
>> +@section ecb_cbc_test
>> +This module is intended for performing functional tests for ecb and cbc
> This is a bit cryptic (cbc ?= Canadian Broadcasting Corporation). If we
> change the name of the module and file, I think we could put: "This
> module performs various cipher mode encryption/decryption tests".

Sure, I will keep the file naming and the description more generic
as per your suggestion.

>
> It would be nice to have more tests of crypto functions, like
> grub_crypto_hash. But that can be for another time.
>
> Other than the above,
> Reviewed-by: Glenn Washburn <[email protected]>


Thanks for the review and feedback.
I will send out v3 with these changes shortly.


>
> Glenn
>
>> +
>>   @node echo_module
>>   @section echo
>>   This module provides support for the @command{echo} to display a line of text.
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index fa4bc54aa..a28de3a93 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -2272,6 +2272,11 @@ module = {
>>     common = tests/argon2_test.c;
>>   };
>>   
>> +module = {
>> +  name = ecb_cbc_test;
>> +  common = tests/ecb_cbc_test.c;
>> +};
>> +
>>   module = {
>>     name = legacy_password_test;
>>     common = tests/legacy_password_test.c;
>> diff --git a/grub-core/tests/ecb_cbc_test.c b/grub-core/tests/ecb_cbc_test.c
>> new file mode 100644
>> index 000000000..e988ab224
>> --- /dev/null
>> +++ b/grub-core/tests/ecb_cbc_test.c
>> @@ -0,0 +1,197 @@
>> +/*
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2025 Free Software Foundation, Inc.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <grub/test.h>
>> +#include <grub/dl.h>
>> +#include <grub/misc.h>
>> +#include <grub/crypto.h>
>> +
>> +#include "ecb_cbc_vectors.h"
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +/* Perform cipher lookup, handle init, and key setting. */
>> +static grub_crypto_cipher_handle_t
>> +handle_init (struct vector vec, grub_crypto_cipher_handle_t handle)
>> +{
>> +  gcry_err_code_t err;
>> +
>> +  const gcry_cipher_spec_t *cipher = grub_crypto_lookup_cipher_by_name (vec.cipher);
>> +  grub_test_assert (cipher != NULL, "\n%s: cipher lookup failed for %s", vec.mode, vec.cipher);
>> +  if (cipher == NULL)
>> +    return NULL;
>> +
>> +  handle = grub_crypto_cipher_open (cipher);
>> +  grub_test_assert (handle != NULL, "\n%s: handle init failed for %s", vec.mode, vec.cipher);
>> +  if (handle == NULL)
>> +    return NULL;
>> +
>> +  err = grub_crypto_cipher_set_key (handle, (grub_uint8_t *) vec.key, vec.keylen);
>> +  grub_test_assert (err == GPG_ERR_NO_ERROR, "\n%s: key set of size %d failed for %s with err = %d",
>> +                    vec.mode, vec.keylen, vec.cipher, err);
>> +  if (err != GPG_ERR_NO_ERROR)
>> +    {
>> +      grub_crypto_cipher_close (handle);
>> +      return NULL;
>> +    }
>> +
>> +  return handle;
>> +}
>> +
>> +static void
>> +ecb_test (struct vector vec)
>> +{
>> +  gcry_err_code_t gcry_err;
>> +  grub_crypto_cipher_handle_t handle = NULL;
>> +  grub_uint8_t *plaintext = NULL, *ciphertext = NULL;
>> +  grub_int32_t rc;
>> +
>> +  handle = handle_init (vec, handle);
>> +  if (handle == NULL)
>> +    return;
>> +
>> +  /* Test encryption. */
>> +  ciphertext = grub_zalloc (vec.plen);
>> +  grub_test_assert (ciphertext != NULL, "\necb: ciphertext buffer allocation failed");
>> +  if (ciphertext == NULL)
>> +    goto out_handle;
>> +
>> +  gcry_err = grub_crypto_ecb_encrypt (handle, ciphertext, vec.ptext, vec.plen);
>> +  grub_test_assert (gcry_err == GPG_ERR_NO_ERROR, "\necb: encryption failed with err = %d",
>> +                    gcry_err);
>> +  if (gcry_err != GPG_ERR_NO_ERROR)
>> +    goto out_ct;
>> +
>> +  rc = grub_memcmp (ciphertext, vec.ctext, vec.plen);
>> +  grub_test_assert (rc == 0, "\necb: ciphertext mismatch after encryption");
>> +  if (rc != 0)
>> +    goto out_ct;
>> +
>> +  /* Test decryption. */
>> +  plaintext = grub_zalloc (vec.plen);
>> +  grub_test_assert (plaintext != NULL, "\necb: plaintext buffer allocation failed");
>> +  if (plaintext == NULL)
>> +    goto out_ct;
>> +
>> +  gcry_err = grub_crypto_ecb_decrypt (handle, plaintext, ciphertext, vec.plen);
>> +  grub_test_assert (gcry_err == GPG_ERR_NO_ERROR, "\necb: decryption failed failed with err = %d",
>> +                    gcry_err);
>> +  if (gcry_err != GPG_ERR_NO_ERROR)
>> +    goto out_pt;
>> +
>> +  rc = grub_memcmp (plaintext, vec.ptext, vec.plen);
>> +  grub_test_assert (rc == 0, "\necb: plaintext mismatch after decryption");
>> +
>> +out_pt:
>> +  grub_free(plaintext);
>> +out_ct:
>> +  grub_free(ciphertext);
>> +out_handle:
>> +  grub_crypto_cipher_close(handle);
>> +}
>> +
>> +static void
>> +cbc_test (struct vector vec)
>> +{
>> +  gcry_err_code_t gcry_err;
>> +  grub_crypto_cipher_handle_t handle = NULL;
>> +  grub_uint8_t *plaintext = NULL, *ciphertext = NULL;
>> +  grub_uint32_t *iv = NULL;
>> +  grub_int32_t rc;
>> +
>> +  handle = handle_init (vec, handle);
>> +  if (handle == NULL)
>> +    return;
>> +
>> +  /* Test Encryption */
>> +  iv = grub_malloc(vec.ivlen);
>> +  grub_test_assert (iv != NULL, "\ncbc: IV buffer allocation failed");
>> +  if (iv == NULL)
>> +    goto out_handle;
>> +
>> +  grub_memcpy (iv, vec.iv_in, vec.ivlen);
>> +
>> +  ciphertext = grub_zalloc (vec.plen);
>> +  grub_test_assert (ciphertext != NULL, "\ncbc: ciphertext buffer allocation failed");
>> +  if (ciphertext == NULL)
>> +    goto out_iv;
>> +
>> +  gcry_err = grub_crypto_cbc_encrypt (handle, ciphertext, vec.ptext, vec.plen, iv);
>> +  grub_test_assert (gcry_err == GPG_ERR_NO_ERROR, "\ncbc: encryption failed with err = %d",
>> +                    gcry_err);
>> +  if (gcry_err != GPG_ERR_NO_ERROR)
>> +    goto out_ct;
>> +
>> +  rc = grub_memcmp (ciphertext, vec.ctext, vec.plen);
>> +  grub_test_assert (rc == 0, "\ncbc: ciphertext mismatch after encryption");
>> +  if (rc != 0)
>> +    goto out_ct;
>> +
>> +  rc = grub_memcmp (iv, vec.iv_out, vec.ivlen);
>> +  grub_test_assert (rc == 0, "\ncbc: IV out mismatch after encryption");
>> +  if (rc != 0)
>> +    goto out_ct;
>> +
>> +  /* Test Decryption  */
>> +  grub_memcpy (iv, vec.iv_in, vec.ivlen);
>> +
>> +  plaintext = grub_zalloc (vec.plen);
>> +  grub_test_assert (plaintext != NULL, "\ncbc: plaintext buffer allocation failed");
>> +  if (plaintext == NULL)
>> +    goto out_ct;
>> +
>> +  gcry_err = grub_crypto_cbc_decrypt (handle, plaintext, ciphertext, vec.plen, iv);
>> +  grub_test_assert (gcry_err == GPG_ERR_NO_ERROR, "\ncbc: decryption failed with err = %d",
>> +                    gcry_err);
>> +  if (gcry_err != GPG_ERR_NO_ERROR)
>> +    goto out_pt;
>> +
>> +  rc = grub_memcmp (plaintext, vec.ptext, vec.plen);
>> +  grub_test_assert (rc == 0, "\ncbc: plaintext mismatch after decryption");
>> +
>> +out_pt:
>> +  grub_free(plaintext);
>> +out_ct:
>> +  grub_free(ciphertext);
>> +out_iv:
>> +  grub_free(iv);
>> +out_handle:
>> +  grub_crypto_cipher_close(handle);
>> +}
>> +
>> +static void
>> +ecb_cbc_test (void)
>> +{
>> +  grub_size_t i;
>> +
>> +  for (i = 0; i < ARRAY_SIZE (vecs); i++)
>> +    {
>> +      if (grub_strcmp (vecs[i].mode, "ecb") == 0)
>> +        ecb_test(vecs[i]);
>> +      else if (grub_strcmp (vecs[i].mode, "cbc") == 0)
>> +        cbc_test(vecs[i]);
>> +      else
>> +        {
>> +          grub_test_assert(0, "\n%s mode unsupported for testing", vecs[i].mode);
>> +          return;
>> +        }
>> +    }
>> +}
>> +
>> +/* Register example_test method as a functional test.  */
>> +GRUB_FUNCTIONAL_TEST (ecb_cbc_test, ecb_cbc_test);
>> diff --git a/grub-core/tests/ecb_cbc_vectors.h b/grub-core/tests/ecb_cbc_vectors.h
>> new file mode 100644
>> index 000000000..8ef948b46
>> --- /dev/null
>> +++ b/grub-core/tests/ecb_cbc_vectors.h
>> @@ -0,0 +1,135 @@
>> +struct vector
>> +{
>> +  const char *cipher;
>> +  const char *mode;
>> +  const char *key;
>> +  grub_uint32_t keylen;
>> +  const char *ptext;
>> +  grub_uint32_t plen;
>> +  const char *ctext;
>> +  const char *iv_in;
>> +  const char *iv_out;
>> +  grub_uint32_t ivlen;
>> +} vecs[] = {
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "ecb",
>> +    .key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
>> +    .keylen = 16,
>> +    .ptext  = "\x00\x11\x22\x33\x44\x55\x66\x77"
>> +              "\x88\x99\xaa\xbb\xcc\xdd\xee\xff",
>> +    .plen   = 16,
>> +    .ctext  = "\x69\xc4\xe0\xd8\x6a\x7b\x04\x30"
>> +              "\xd8\xcd\xb7\x80\x70\xb4\xc5\x5a",
>> +  },
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "ecb",
>> +    .key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
>> +              "\x10\x11\x12\x13\x14\x15\x16\x17",
>> +    .keylen = 24,
>> +    .ptext  = "\x00\x11\x22\x33\x44\x55\x66\x77"
>> +              "\x88\x99\xaa\xbb\xcc\xdd\xee\xff",
>> +    .plen   = 16,
>> +    .ctext  = "\xdd\xa9\x7c\xa4\x86\x4c\xdf\xe0"
>> +              "\x6e\xaf\x70\xa0\xec\x0d\x71\x91",
>> +  },
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "ecb",
>> +    .key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
>> +              "\x10\x11\x12\x13\x14\x15\x16\x17"
>> +              "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
>> +    .keylen = 32,
>> +    .ptext  = "\x00\x11\x22\x33\x44\x55\x66\x77"
>> +              "\x88\x99\xaa\xbb\xcc\xdd\xee\xff",
>> +    .plen   = 16,
>> +    .ctext  = "\x8e\xa2\xb7\xca\x51\x67\x45\xbf"
>> +              "\xea\xfc\x49\x90\x4b\x49\x60\x89",
>> +  },
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "cbc",
>> +    .key    = "\xc2\x86\x69\x6d\x88\x7c\x9a\xa0"
>> +              "\x61\x1b\xbb\x3e\x20\x25\xa4\x5a",
>> +    .keylen = 16,
>> +    .ptext  = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
>> +              "\x10\x11\x12\x13\x14\x15\x16\x17"
>> +              "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
>> +    .plen   = 32,
>> +    .ctext  = "\xd2\x96\xcd\x94\xc2\xcc\xcf\x8a"
>> +              "\x3a\x86\x30\x28\xb5\xe1\xdc\x0a"
>> +              "\x75\x86\x60\x2d\x25\x3c\xff\xf9"
>> +              "\x1b\x82\x66\xbe\xa6\xd6\x1a\xb1",
>> +    .iv_in  = "\x56\x2e\x17\x99\x6d\x09\x3d\x28"
>> +              "\xdd\xb3\xba\x69\x5a\x2e\x6f\x58",
>> +    .iv_out = "\x75\x86\x60\x2d\x25\x3c\xff\xf9"
>> +              "\x1b\x82\x66\xbe\xa6\xd6\x1a\xb1",
>> +    .ivlen  = 16,
>> +  },
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "cbc",
>> +    .key    = "\x8e\x73\xb0\xf7\xda\x0e\x64\x52"
>> +              "\xc8\x10\xf3\x2b\x80\x90\x79\xe5"
>> +              "\x62\xf8\xea\xd2\x52\x2c\x6b\x7b",
>> +    .keylen = 24,
>> +    .ptext  = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
>> +              "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
>> +              "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
>> +              "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
>> +              "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
>> +              "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
>> +              "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
>> +              "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
>> +    .plen   = 64,
>> +    .ctext  = "\x4f\x02\x1d\xb2\x43\xbc\x63\x3d"
>> +              "\x71\x78\x18\x3a\x9f\xa0\x71\xe8"
>> +              "\xb4\xd9\xad\xa9\xad\x7d\xed\xf4"
>> +              "\xe5\xe7\x38\x76\x3f\x69\x14\x5a"
>> +              "\x57\x1b\x24\x20\x12\xfb\x7a\xe0"
>> +              "\x7f\xa9\xba\xac\x3d\xf1\x02\xe0"
>> +              "\x08\xb0\xe2\x79\x88\x59\x88\x81"
>> +              "\xd9\x20\xa9\xe6\x4f\x56\x15\xcd",
>> +    .iv_in  = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
>> +    .iv_out = "\x08\xb0\xe2\x79\x88\x59\x88\x81"
>> +              "\xd9\x20\xa9\xe6\x4f\x56\x15\xcd",
>> +    .ivlen  = 16,
>> +  },
>> +  {
>> +    .cipher = "aes",
>> +    .mode   = "cbc",
>> +    .key    = "\x60\x3d\xeb\x10\x15\xca\x71\xbe"
>> +              "\x2b\x73\xae\xf0\x85\x7d\x77\x81"
>> +              "\x1f\x35\x2c\x07\x3b\x61\x08\xd7"
>> +              "\x2d\x98\x10\xa3\x09\x14\xdf\xf4",
>> +    .keylen = 32,
>> +    .ptext  = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
>> +              "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
>> +              "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
>> +              "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
>> +              "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
>> +              "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
>> +              "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
>> +              "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
>> +    .plen   = 64,
>> +    .ctext  = "\xf5\x8c\x4c\x04\xd6\xe5\xf1\xba"
>> +              "\x77\x9e\xab\xfb\x5f\x7b\xfb\xd6"
>> +              "\x9c\xfc\x4e\x96\x7e\xdb\x80\x8d"
>> +              "\x67\x9f\x77\x7b\xc6\x70\x2c\x7d"
>> +              "\x39\xf2\x33\x69\xa9\xd9\xba\xcf"
>> +              "\xa5\x30\xe2\x63\x04\x23\x14\x61"
>> +              "\xb2\xeb\x05\xe2\xc3\x9b\xe9\xfc"
>> +              "\xda\x6c\x19\x07\x8c\x6a\x9d\x1b",
>> +    .iv_in  = "\x00\x01\x02\x03\x04\x05\x06\x07"
>> +              "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
>> +    .iv_out = "\xb2\xeb\x05\xe2\xc3\x9b\xe9\xfc"
>> +              "\xda\x6c\x19\x07\x8c\x6a\x9d\x1b",
>> +    .ivlen  = 16,
>> +  },
>> +};
>> diff --git a/grub-core/tests/lib/functional_test.c b/grub-core/tests/lib/functional_test.c
>> index 776934e1f..84ef95bbe 100644
>> --- a/grub-core/tests/lib/functional_test.c
>> +++ b/grub-core/tests/lib/functional_test.c
>> @@ -82,6 +82,7 @@ grub_functional_all_tests (grub_extcmd_context_t ctxt __attribute__ ((unused)),
>>     grub_dl_load ("shift_test");
>>     grub_dl_load ("asn1_test");
>>     grub_dl_load ("argon2_test");
>> +  grub_dl_load ("ecb_cbc_test");
>>   
>>     FOR_LIST_ELEMENTS (test, grub_test_list)
>>       ok = !grub_test_run (test) && ok;
> _______________________________________________
> Grub-devel mailing list
> [email protected]
> https://lists.gnu.org/mailman/listinfo/grub-devel

thanks,
Srish.

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel