| Message ID | 20220204073202.4141198-2-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/2] efi_loader: fix dual signed image certification | expand |
On Fri, Feb 04, 2022 at 09:32:02AM +0200, Ilias Apalodimas wrote: > The previous patch is changing U-Boot's behavior wrt certificate based > binary authentication. Specifically an image who's digest of a > certificate is found in dbx is now rejected. Fix the test accordingly Please not only fix the given test case, but also add more cases if needed or appropriate for wider coverage of corner cases. You mentioned in the previous commit that the order of certificates should not affect the verification result. If so, we need, at least, one more test case where the order of certificates in db is different. I think that trying to maintain the test scenario that way will help improve the robustness of verification logic. -Takahiro Akashi > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > test/py/tests/test_efi_secboot/test_signed.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > index 0aee34479f55..7f5ec78261da 100644 > --- a/test/py/tests/test_efi_secboot/test_signed.py > +++ b/test/py/tests/test_efi_secboot/test_signed.py > @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): > assert 'Hello, world!' in ''.join(output) > > with u_boot_console.log.section('Test Case 5c'): > - # Test Case 5c, not rejected if one of signatures (digest of > + # Test Case 5c, rejected if one of signatures (digest of > # certificate) is revoked > output = u_boot_console.run_command_list([ > 'fatload host 0:1 4000000 dbx_hash.auth', > @@ -195,7 +195,8 @@ class TestEfiSignedImage(object): > output = u_boot_console.run_command_list([ > 'efidebug boot next 1', > 'efidebug test bootmgr']) > - assert 'Hello, world!' in ''.join(output) > + assert '\'HELLO\' failed' in ''.join(output) > + assert 'efi_start_image() returned: 26' in ''.join(output) > > with u_boot_console.log.section('Test Case 5d'): > # Test Case 5d, rejected if both of signatures are revoked > -- > 2.32.0 >
Akashi-san On Thu, 10 Feb 2022 at 07:22, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Fri, Feb 04, 2022 at 09:32:02AM +0200, Ilias Apalodimas wrote: > > The previous patch is changing U-Boot's behavior wrt certificate based > > binary authentication. Specifically an image who's digest of a > > certificate is found in dbx is now rejected. Fix the test accordingly > > Please not only fix the given test case, but also add more cases > if needed or appropriate for wider coverage of corner cases. > You mentioned in the previous commit that the order of certificates > should not affect the verification result. > If so, we need, at least, one more test case where the order of certificates > in db is different. > > I think that trying to maintain the test scenario that way will help improve > the robustness of verification logic. And we agree, but my concern right now is fix the existing use cases. There are some SCT tests wrt certification of binaries, so I intend to port more cases for those in the future. Cheers /Ilias > > -Takahiro Akashi > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > test/py/tests/test_efi_secboot/test_signed.py | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > index 0aee34479f55..7f5ec78261da 100644 > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): > > assert 'Hello, world!' in ''.join(output) > > > > with u_boot_console.log.section('Test Case 5c'): > > - # Test Case 5c, not rejected if one of signatures (digest of > > + # Test Case 5c, rejected if one of signatures (digest of > > # certificate) is revoked > > output = u_boot_console.run_command_list([ > > 'fatload host 0:1 4000000 dbx_hash.auth', > > @@ -195,7 +195,8 @@ class TestEfiSignedImage(object): > > output = u_boot_console.run_command_list([ > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > - assert 'Hello, world!' in ''.join(output) > > + assert '\'HELLO\' failed' in ''.join(output) > > + assert 'efi_start_image() returned: 26' in ''.join(output) > > > > with u_boot_console.log.section('Test Case 5d'): > > # Test Case 5d, rejected if both of signatures are revoked > > -- > > 2.32.0 > >
On Thu, Feb 10, 2022 at 09:14:25AM +0200, Ilias Apalodimas wrote: > Akashi-san > > On Thu, 10 Feb 2022 at 07:22, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > On Fri, Feb 04, 2022 at 09:32:02AM +0200, Ilias Apalodimas wrote: > > > The previous patch is changing U-Boot's behavior wrt certificate based > > > binary authentication. Specifically an image who's digest of a > > > certificate is found in dbx is now rejected. Fix the test accordingly > > > > Please not only fix the given test case, but also add more cases > > if needed or appropriate for wider coverage of corner cases. > > You mentioned in the previous commit that the order of certificates > > should not affect the verification result. > > If so, we need, at least, one more test case where the order of certificates > > in db is different. > > > > I think that trying to maintain the test scenario that way will help improve > > the robustness of verification logic. > > And we agree, but my concern right now is fix the existing use cases. But you have to verify the logic works in the same way whatever the order of certificates is. I think that is your intent in this patch. -Takahiro Akashi > There are some SCT tests wrt certification of binaries, so I intend > to port more cases for those in the future. > > Cheers > /Ilias > > > > -Takahiro Akashi > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > --- > > > test/py/tests/test_efi_secboot/test_signed.py | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > > index 0aee34479f55..7f5ec78261da 100644 > > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > > @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): > > > assert 'Hello, world!' in ''.join(output) > > > > > > with u_boot_console.log.section('Test Case 5c'): > > > - # Test Case 5c, not rejected if one of signatures (digest of > > > + # Test Case 5c, rejected if one of signatures (digest of > > > # certificate) is revoked > > > output = u_boot_console.run_command_list([ > > > 'fatload host 0:1 4000000 dbx_hash.auth', > > > @@ -195,7 +195,8 @@ class TestEfiSignedImage(object): > > > output = u_boot_console.run_command_list([ > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > - assert 'Hello, world!' in ''.join(output) > > > + assert '\'HELLO\' failed' in ''.join(output) > > > + assert 'efi_start_image() returned: 26' in ''.join(output) > > > > > > with u_boot_console.log.section('Test Case 5d'): > > > # Test Case 5d, rejected if both of signatures are revoked > > > -- > > > 2.32.0 > > >
Akashi-san On Thu, 10 Feb 2022 at 09:31, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Thu, Feb 10, 2022 at 09:14:25AM +0200, Ilias Apalodimas wrote: > > Akashi-san > > > > On Thu, 10 Feb 2022 at 07:22, AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > > > > > > On Fri, Feb 04, 2022 at 09:32:02AM +0200, Ilias Apalodimas wrote: > > > > The previous patch is changing U-Boot's behavior wrt certificate based > > > > binary authentication. Specifically an image who's digest of a > > > > certificate is found in dbx is now rejected. Fix the test accordingly > > > > > > Please not only fix the given test case, but also add more cases > > > if needed or appropriate for wider coverage of corner cases. > > > You mentioned in the previous commit that the order of certificates > > > should not affect the verification result. > > > If so, we need, at least, one more test case where the order of certificates > > > in db is different. > > > > > > I think that trying to maintain the test scenario that way will help improve > > > the robustness of verification logic. > > > > And we agree, but my concern right now is fix the existing use cases. > > But you have to verify the logic works in the same way whatever the order of > certificates is. I think that is your intent in this patch. Fair enough, I'll add a test case for that. FWIW I think this patch needs rework as is, because the last 2 cases reject the image. But we don't really know if the rejection comes from an x509 cert or it's sha256. Cheers /Ilias > > -Takahiro Akashi > > > > There are some SCT tests wrt certification of binaries, so I intend > > to port more cases for those in the future. > > > > Cheers > > /Ilias > > > > > > -Takahiro Akashi > > > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > --- > > > > test/py/tests/test_efi_secboot/test_signed.py | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > > > index 0aee34479f55..7f5ec78261da 100644 > > > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > > > @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): > > > > assert 'Hello, world!' in ''.join(output) > > > > > > > > with u_boot_console.log.section('Test Case 5c'): > > > > - # Test Case 5c, not rejected if one of signatures (digest of > > > > + # Test Case 5c, rejected if one of signatures (digest of > > > > # certificate) is revoked > > > > output = u_boot_console.run_command_list([ > > > > 'fatload host 0:1 4000000 dbx_hash.auth', > > > > @@ -195,7 +195,8 @@ class TestEfiSignedImage(object): > > > > output = u_boot_console.run_command_list([ > > > > 'efidebug boot next 1', > > > > 'efidebug test bootmgr']) > > > > - assert 'Hello, world!' in ''.join(output) > > > > + assert '\'HELLO\' failed' in ''.join(output) > > > > + assert 'efi_start_image() returned: 26' in ''.join(output) > > > > > > > > with u_boot_console.log.section('Test Case 5d'): > > > > # Test Case 5d, rejected if both of signatures are revoked > > > > -- > > > > 2.32.0 > > > >
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 0aee34479f55..7f5ec78261da 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): assert 'Hello, world!' in ''.join(output) with u_boot_console.log.section('Test Case 5c'): - # Test Case 5c, not rejected if one of signatures (digest of + # Test Case 5c, rejected if one of signatures (digest of # certificate) is revoked output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 dbx_hash.auth', @@ -195,7 +195,8 @@ class TestEfiSignedImage(object): output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert 'Hello, world!' in ''.join(output) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) with u_boot_console.log.section('Test Case 5d'): # Test Case 5d, rejected if both of signatures are revoked
The previous patch is changing U-Boot's behavior wrt certificate based binary authentication. Specifically an image who's digest of a certificate is found in dbx is now rejected. Fix the test accordingly Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- test/py/tests/test_efi_secboot/test_signed.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)