search

yoctoproject / bmaptool

BMAP Tools
GNU General Public License v2.0
27 stars 13 forks source link

Rework of signature verification #1

Open jo-so-nx opened 8 months ago

jo-so-nx commented 8 months ago

Currently, the signature verification doesn't work, esp. the detection of inline signatures what causes the whole file can't be processed, because it's invalid. This rework joins both cases of inline and detached signatures, because the GPG API allows the passing and receive of buffers.

dedekind commented 8 months ago

Sorry, I wrote something here, but I was confused that this is still the old github URL, apologies, ignore my message (I also deleted it).

twoerner commented 8 months ago

Also, please add SoB lines to your commits, thanks!

twoerner commented 8 months ago

bmaptool doesn't have a lot of tests, but what little tests it does have does seem to test the signing feature. These signing tests seem to succeed both with and without this patch. Could you provide me with a clearer example of what is not working; or, better yet, provide a test that demonstrates the problem before your patch, and how your patch fixes it?

i.e. see tests/test_CLI.py

jo-so-nx commented 8 months ago

Isn't the test test_clearsign required to fail? The return code should be 1:

self.assertEqual(completed_process.returncode, 1, completed_process.stdout)

Here is what I tried:

% g clone 'https://github.com/yoctoproject/bmaptool.git' /tmp/bmt
Cloning into '/tmp/bmt'...
remote: Enumerating objects: 3077, done.
remote: Counting objects: 100% (398/398), done.
remote: Compressing objects: 100% (28/28), done.
remote: Total 3077 (delta 376), reused 371 (delta 370), pack-reused 2679
Receiving objects: 100% (3077/3077), 673.32 KiB | 3.83 MiB/s, done.
Resolving deltas: 100% (2021/2021), done.
% cd /tmp/bmt   
% venv_init venv requirements-test.txt
Activating venv in /tmp/bmt/venv
Ignoring backports.tempfile: markers 'python_version < "3.2"' don't match your environment
Ignoring mock: markers 'python_version < "3.3"' don't match your environment
Collecting six (from -r requirements-test.txt (line 1))
  Using cached six-1.16.0-py2.py3-none-any.whl.metadata (1.8 kB)
Collecting nose (from -r requirements-test.txt (line 2))
  Using cached nose-1.3.7-py3-none-any.whl.metadata (1.7 kB)
Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Using cached nose-1.3.7-py3-none-any.whl (154 kB)
Installing collected packages: nose, six
Successfully installed nose-1.3.7 six-1.16.0
% python setup.py install
running install
…
Processing dependencies for bmap-tools==3.7
Finished processing dependencies for bmap-tools==3.7
% bmaptool --version
bmaptool 3.7
% g apply 
diff --git i/tests/test_CLI.py w/tests/test_CLI.py
index 4683960..64946e3 100644
--- i/tests/test_CLI.py
+++ w/tests/test_CLI.py
@@ -106,11 +106,9 @@ class TestCLI(unittest.TestCase):
                 "tests/test-data/test.image.gz",
                 self.tmpfile,
             ],
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
             check=False,
         )
-        self.assertEqual(completed_process.returncode, 1, completed_process.stdout)
+        self.assertEqual(completed_process.returncode, 0)

     def setUp(self):
         os.environ["GNUPGHOME"] = "tests/test-data/gnupg/"
% python -m unittest -bv tests.test_CLI.TestCLI.test_clearsign
test_clearsign (tests.test_CLI.TestCLI.test_clearsign) ... Traceback (most recent call last):
  File "/tmp/bmt/venv/lib/python3.11/site-packages/bmap_tools-3.7-py3.11.egg/bmaptools/BmapCopy.py", line 401, in _parse_bmap
  File "/usr/lib/python3.11/xml/etree/ElementTree.py", line 1219, in parse
    tree.parse(source, parser)
  File "/usr/lib/python3.11/xml/etree/ElementTree.py", line 581, in parse
    self._root = parser._parse_whole(source)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
xml.etree.ElementTree.ParseError: syntax error: line 1, column 0

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/bmt/venv/bin/bmaptool", line 33, in <module>
    sys.exit(load_entry_point('bmap-tools==3.7', 'console_scripts', 'bmaptool')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/bmt/venv/lib/python3.11/site-packages/bmap_tools-3.7-py3.11.egg/bmaptools/CLI.py", line 780, in main
  File "/tmp/bmt/venv/lib/python3.11/site-packages/bmap_tools-3.7-py3.11.egg/bmaptools/CLI.py", line 490, in copy_command
  File "/tmp/bmt/venv/lib/python3.11/site-packages/bmap_tools-3.7-py3.11.egg/bmaptools/BmapCopy.py", line 287, in __init__
  File "/tmp/bmt/venv/lib/python3.11/site-packages/bmap_tools-3.7-py3.11.egg/bmaptools/BmapCopy.py", line 406, in _parse_bmap
TypeError: 'NamedFile' object is not iterable
FAIL

======================================================================
FAIL: test_clearsign (tests.test_CLI.TestCLI.test_clearsign)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/bmt/tests/test_CLI.py", line 111, in test_clearsign
    self.assertEqual(completed_process.returncode, 0)
AssertionError: 1 != 0

----------------------------------------------------------------------
Ran 1 test in 0.163s

FAILED (failures=1)

The first problem is the comparison of string clearsign_marker and the byte array from bmap_obj.read() in verify_bmap_signature which is never true, and the inline signature gets never detected.

bnavigator commented 8 months ago

Please also see https://github.com/intel/bmap-tools/issues/116 for a case where signature checking tests currently fail.

twoerner commented 8 months ago

Please also see intel#116 for a case where signature checking tests currently fail.

Thanks for the pointer @bnavigator So this PR fixes something that will break after 2024-06-12?

twoerner commented 8 months ago

Isn't the test test_clearsign required to fail? The return code should be 1:

self.assertEqual(completed_process.returncode, 1, completed_process.stdout)

@jo-so-nx Funny, I was just wondering the same thing the other day!

bnavigator commented 8 months ago

So this PR fixes something that will break after 2024-06-12?

It will break after 2024-06-12 with or without the PR. The embedded key expires.

[   19s] + pytest-3.9 --ignore=_build.python39 --ignore=_build.python310 --ignore=_build.python312 --ignore=_build.python311 -v -k 'not test_is_zfs_configuration_compatible_unreadable_file'
[   19s] ============================= test session starts ==============================
[   19s] platform linux -- Python 3.9.18, pytest-7.4.4, pluggy-1.3.0 -- /usr/bin/python3.9
[   19s] cachedir: .pytest_cache
[   19s] rootdir: /home/abuild/rpmbuild/BUILD/bmaptool-3.7
[   19s] collecting ... collected 19 items / 1 deselected / 18 selected
[   19s] 
[   19s] tests/test_CLI.py::TestCLI::test_clearsign FAILED                        [  5%]
[   19s] tests/test_CLI.py::TestCLI::test_unknown_signer PASSED                   [ 11%]
[   20s] tests/test_CLI.py::TestCLI::test_valid_signature FAILED                  [ 16%]
[   20s] tests/test_CLI.py::TestCLI::test_wrong_signature PASSED                  [ 22%]
[   20s] tests/test_CLI.py::TestCLI::test_wrong_signature_uknown_signer PASSED    [ 27%]
[   89s] tests/test_api_base.py::TestCreateCopy::test PASSED                      [ 33%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_get_file_system_type PASSED [ 38%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_get_file_system_type_no_fstype_found PASSED [ 44%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_get_file_system_type_symlink PASSED [ 50%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_compatible_file_system_ext4 PASSED [ 55%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_compatible_file_system_zfs_invalid PASSED [ 61%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_compatible_file_system_zfs_valid PASSED [ 66%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_zfs_configuration_compatible_disabled PASSED [ 72%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_zfs_configuration_compatible_enabled PASSED [ 77%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_zfs_configuration_compatible_invalid_read_value PASSED [ 83%]
[   89s] tests/test_bmap_helpers.py::TestBmapHelpers::test_is_zfs_configuration_compatible_notinstalled PASSED [ 88%]
[   89s] tests/test_compat.py::TestCompat::test PASSED                            [ 94%]
[   92s] tests/test_filemap.py::TestFilemap::test PASSED                          [100%]
[   92s] 
[   92s] =================================== FAILURES ===================================
[   92s] ____________________________ TestCLI.test_clearsign ____________________________
[   92s] 
[   92s] self = <tests.test_CLI.TestCLI testMethod=test_clearsign>
[   92s] 
[   92s]     def test_clearsign(self):
[   92s]         completed_process = subprocess.run(
[   92s]             [
[   92s]                 "bmaptool",
[   92s]                 "copy",
[   92s]                 "--bmap",
[   92s]                 "tests/test-data/signatures/test.image.bmap.v2.0.asc",
[   92s]                 "tests/test-data/test.image.gz",
[   92s]                 self.tmpfile,
[   92s]             ],
[   92s]             check=False,
[   92s]         )
[   92s] >       self.assertEqual(completed_process.returncode, 0)
[   92s] E       AssertionError: 1 != 0
[   92s] 
[   92s] tests/test_CLI.py:111: AssertionError
[   92s] ----------------------------- Captured stderr call -----------------------------
[   92s] bmaptool: info: discovered inline signature
[   92s] bmaptool: ERROR: An error occurred, here is the traceback:
[   92s] Traceback (most recent call last):
[   92s]   File "/home/abuild/rpmbuild/BUILDROOT/bmap-tools-3.7-0.x86_64/usr/lib/python3.9/site-packages/bmaptools/CLI.py", line 204, in verify_bmap_signature
[   92s]     plaintext, sigs = context.verify(bmap_data, det_sig_data)
[   92s]   File "/usr/lib64/python3.9/site-packages/gpg/core.py", line 558, in verify
[   92s]     raise errors.BadSignatures(results[1], results=results)
[   92s] 
[   92s] bmaptool: ERROR: discovered a BAD GPG signature: tests/test-data/signatures/test.image.bmap.v2.0.asc
[   92s] 
[   92s] 
[   92s] _________________________ TestCLI.test_valid_signature _________________________
[   92s] 
[   92s] self = <tests.test_CLI.TestCLI testMethod=test_valid_signature>
[   92s] 
[   92s]     def test_valid_signature(self):
[   92s]         completed_process = subprocess.run(
[   92s]             [
[   92s]                 "bmaptool",
[   92s]                 "copy",
[   92s]                 "--bmap",
[   92s]                 "tests/test-data/test.image.bmap.v2.0",
[   92s]                 "--bmap-sig",
[   92s]                 "tests/test-data/signatures/test.image.bmap.v2.0.valid-sig",
[   92s]                 "tests/test-data/test.image.gz",
[   92s]                 self.tmpfile,
[   92s]             ],
[   92s]             stdout=subprocess.PIPE,
[   92s]             stderr=subprocess.STDOUT,
[   92s]             check=False,
[   92s]         )
[   92s] >       self.assertEqual(completed_process.returncode, 0, completed_process.stdout)
[   92s] E       AssertionError: 1 != 0 : b'bmaptool: \x1b[91mERROR\x1b[0m: An error occurred, here is the traceback:\nTraceback (most recent call last):\n  File "/home/abuild/rpmbuild/BUILDROOT/bmap-tools-3.7-0.x86_64/usr/lib/python3.9/site-packages/bmaptools/CLI.py", line 204, in verify_bmap_signature\n    plaintext, sigs = context.verify(bmap_data, det_sig_data)\n  File "/usr/lib64/python3.9/site-packages/gpg/core.py", line 558, in verify\n    raise errors.BadSignatures(results[1], results=results)\n\nbmaptool: \x1b[91mERROR\x1b[0m: discovered a BAD GPG signature: tests/test-data/signatures/test.image.bmap.v2.0.valid-sig\n\n\n'
[   92s] 
[   92s] tests/test_CLI.py:43: AssertionError
[   92s] =========================== short test summary info ============================
[   92s] FAILED tests/test_CLI.py::TestCLI::test_clearsign - AssertionError: 1 != 0
[   92s] FAILED tests/test_CLI.py::TestCLI::test_valid_signature - AssertionError: 1 !...
[   92s] ============ 2 failed, 16 passed, 1 deselected in 72.43s (0:01:12) =============
bnavigator commented 8 months ago

Extending the test key is enough for https://github.com/intel/bmap-tools/issues/116. It's unrelated to this PR. Sorry for the noise. Althought I would not know where else to discuss. There is no issue tracker in the new repo, yet.

# extend signing key expiration for reproducible builds
export GNUPGHOME=$PWD/tests/test-data/gnupg
echo 'expire
50y
key 1
expire
50y
save' | gpg --command-fd=0 --batch --edit-key 927FF9746434704C5774BE648D49DFB1163BDFB4
jo-so-nx commented 8 months ago

@twoerner another question is if this should be added:

diff --git i/.github/workflows/ci.yml w/.github/workflows/ci.yml
index 7b61782..d2c3a20 100644
--- i/.github/workflows/ci.yml
+++ w/.github/workflows/ci.yml
@@ -30,7 +30,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.8", "3.9", "3.10"]
+        python-version: ["3.8", "3.9", "3.10", "3.11"]

     steps:
     - uses: actions/checkout@v3

And is the description in README.md about tests (bmaptool/README.md) still valid? I get the error AttributeError: module 'collections' has no attribute 'Callable' when running nosetests. Hence, I use python -m unittest -bv tests.test_CLI.TestCLI. Should this be updated?

jo-so-nx commented 8 months ago

Hint: There's also python3.12

moto-timo commented 8 months ago

Hint: There's also python3.12

Patches and pull requests are welcome.

josch commented 4 months ago

Extending the test key is enough for intel#116. It's unrelated to this PR

@bnavigator I fixed that unrelated thing in #31 by always re-creating $GNUPGHOME. This also allows this git repo to have fewer binary blobs stored in it.