Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If librepo is built with -DUSE_GPGME and RPM with -DWITH_INTERNAL_OPENPGP=ON -DWITH_OPENSSL=ON, test_gpg_check_signature fails: Assertion 'ret' failed #281

Closed
kloczek opened this issue Sep 12, 2023 · 14 comments
Assignees
Labels
Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take

Comments

@kloczek
Copy link

kloczek commented Sep 12, 2023

cmake setup

-- Cache values
CMAKE_BUILD_TYPE:STRING=RelWithDebInfo
CMAKE_INSTALL_PREFIX:PATH=/usr
ENABLE_DOCS:BOOL=ON
ENABLE_PYTHON:BOOL=ON
ENABLE_TESTS:BOOL=ON
USE_GPGME:BOOL=OFF
WITH_ZCHUNK:BOOL=ON

and test suite is failing with:

+ cd librepo-1.16.0
+ /usr/bin/ctest --test-dir x86_64-redhat-linux-gnu --output-on-failure --force-new-ctest-process -j48
Internal ctest changing into directory: /home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/x86_64-redhat-linux-gnu
Test project /home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/x86_64-redhat-linux-gnu
    Start 1: test_main
    Start 2: test_python
1/2 Test #1: test_main ........................***Failed    0.12 sec
Tests using directory: /tmp/librepoWXh22S
Running suite(s): checksum
 gpg
 handle

(process:685827): librepo-WARNING **: 02:07:36.158: WARNING! Deprecated LRO_MIRRORLIST used

(process:685827): librepo-WARNING **: 02:07:36.158: WARNING! Deprecated LRO_MIRRORLIST used
 internal_mirrorlist
 metalink
 mirrorlist
 package_downloader
 repoconf
 repomd
 url_substitution
 util
 version
95%: Checks: 47, Failures: 2, Errors: 0
/home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/tests/test_gpg.c:51:F:Main:test_gpg_check_signature:0: Assertion 'ret' failed
/home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/tests/test_gpg.c:169:F:Main:test_gpg_check_key_export:0: Assertion 'subkeys != NULL' failed: subkeys == 0

2/2 Test #2: test_python ......................   Passed   12.24 sec

50% tests passed, 1 tests failed out of 2

Total Test time (real) =  12.25 sec

The following tests FAILED:
          1 - test_main (Failed)
Errors while running CTest
@ppisar
Copy link
Contributor

ppisar commented Sep 14, 2023

I cannot reproduce it on current x86_64 Fedora 40.

What RPM and rpm-sequoia do you have? I have rpm-4.18.99-1.fc40.x86_64 and rpm-sequoia-1.5.0-1.fc40.x86_64.
Maybe your system does not support 2048kb RSA keys which are used in the tests.

@ppisar
Copy link
Contributor

ppisar commented Sep 14, 2023

This patch should make the test failure more verbose:

--- a/tests/test_gpg.c
+++ b/tests/test_gpg.c
@@ -48,8 +48,8 @@ START_TEST(test_gpg_check_signature)
                                  data_path,
                                  tmp_home_path,
                                  &tmp_err);
-    ck_assert(ret);
-    ck_assert_ptr_null(tmp_err);
+    ck_assert_msg(ret, "Checking valid key and data failed with \"%s\"", (tmp_err && tmp_err->message) ? tmp_err->message : "");
+    ck_assert_msg(NULL == tmp_err, "Checking valid key and data passed but set error \"%s\"", (tmp_err && tmp_err->message) ? tmp_err->message : "");
 
     // Bad signature signed with unknown key
     ret = lr_gpg_check_signature(_signature_path,

@kloczek
Copy link
Author

kloczek commented Sep 14, 2023

Just FTR: I'm building librepo with -D USE_GPGME=OFF. My understanding is that in such case signature verification is done over rpm API/ABI (rpm is build with -D WITH_INTERNAL_OPENPGP=ON -D WITH_OPENSSL=ON)

Here is the result with instrumentation patch:

+ cd librepo-1.16.0
+ /usr/bin/ctest --test-dir x86_64-redhat-linux-gnu --output-on-failure --force-new-ctest-process -j48 ' '
Internal ctest changing into directory: /home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/x86_64-redhat-linux-gnu
Test project /home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/x86_64-redhat-linux-gnu
    Start 1: test_main
    Start 2: test_python
1/2 Test #1: test_main ........................***Failed    0.12 sec
Tests using directory: /tmp/librepoKNQszg
Running suite(s): checksum
 gpg
 handle

(process:1112696): librepo-WARNING **: 19:27:57.446: WARNING! Deprecated LRO_MIRRORLIST used

(process:1112696): librepo-WARNING **: 19:27:57.446: WARNING! Deprecated LRO_MIRRORLIST used
 internal_mirrorlist
 metalink
 mirrorlist
 package_downloader
 repoconf
 repomd
 url_substitution
 util
 version
95%: Checks: 47, Failures: 2, Errors: 0
/home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/tests/test_gpg.c:51:F:Main:test_gpg_check_signature:0: Checking valid key and data failed with "check_signature: Error during parsing OpenPGP pa
cket(s)"
/home/tkloczko/rpmbuild/BUILD/librepo-1.16.0/tests/test_gpg.c:169:F:Main:test_gpg_check_key_export:0: Assertion 'subkeys != NULL' failed: subkeys == 0

2/2 Test #2: test_python ......................   Passed   12.30 sec

50% tests passed, 1 tests failed out of 2

Total Test time (real) =  12.31 sec

The following tests FAILED:
          1 - test_main (Failed)
Errors while running CTest

@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

That's why I asked what RPM and Sequoia you have. I suspect the root is lies there. Can you tell me which system you run the tests in?

@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

You can also enable tracing RPM with RPM_TRACE=1 environemnt variable:

tests $ RPM_TRACE=1 ./run_tests.sh
Tests using directory: /tmp/librepoWvwNeh
Running suite(s): checksum
 gpg
_pgpParsePkts: entered
_pgpParsePkts: armor: & <- 0x1a26920
_pgpParsePkts: pkt: &mut <- 0x7ffe2a569210
_pgpParsePkts: pktlen: &mut <- 0x7ffe2a569208
_pgpParsePkts: -> success
_pgpPubKeyCertLen: entered
_pgpPubKeyCertLen: pkts: &[] <- 0x1a4b8d0
_pgpPubKeyCertLen: certlen: &mut <- 0x7ffe2a5691a8
_pgpPubKeyCertLen: Found a PublicKey at offset 0, length: Full(269)
_pgpPubKeyCertLen: Found a UserID at offset 272, length: Full(53)
_pgpPubKeyCertLen: Found a Signature at offset 327, length: Full(334)
_pgpPubKeyCertLen: Found a PublicSubkey at offset 664, length: Full(269)
_pgpPubKeyCertLen: Found a Signature at offset 936, length: Full(310)
_pgpPubKeyCertLen: -> success
_pgpPrtParams: entered
 _pgpPrtParams2: entered
  pgp_prt_params: pkts: &[] <- 0x1a4b8d0
  pgp_prt_params: paramsp: &mut <- 0x7ffe2a5691a0
  pgp_prt_params: PgpDigParams { obj, signid: buffer, userid: userid }: returning 0x1a2d220
 _pgpPrtParams2: -> success
_pgpPrtParams: -> success
_pgpDigParamsSignID: entered
_pgpDigParamsSignID: dig: & <- 0x1a2d220
_pgpDigParamsSignID: SignID: 97BDB7906441EDF5
_pgpDigParamsSignID: -> success
[...]

@kloczek
Copy link
Author

kloczek commented Sep 15, 2023

That's why I asked what RPM and Sequoia you have. I suspect the root is lies there. Can you tell me which system you run the tests in?

What you mean "which?" 🤔
I'm building all my packages in spawned for single package build LXC zones in which are installed only packages listed in BuildRequires (+ its dependencies) but in this case it should not relevant.

@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

What packages you use as build dependencies? I gave you exact package versions I use and where the failure does not occur. Without knowing your build environment to reproduce the failure I cannot help you.

@kloczek
Copy link
Author

kloczek commented Sep 15, 2023

What packages you use as build dependencies? I gave you exact package versions I use and where the failure does not occur.

Mostly latest versions (except openssl):
check 0.15.2, cmake 3.27.4, doxygen 1.9.8, glib2 2.78.0, libattr 2.5.1, libcurl 8.3.0, libxml2 2.11.5, openssl 3.0.8,
pkgconf 2.0.3, rpm 4.18.99, zchunk 1.3.1.

In your case you listed rpm-4.18.99-1.fc40.x86_64. Fedora uses sequoia (not openssl + internal gpg like it is in my case)

@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

Thanks for the specification. Now I can reproduce it (configuring RPM with -DWITH_INTERNAL_OPENPGP=ON -DWITH_OPENSSL=ON CMake options).

This is a bug in RPM's internal OpenPGP parser rpm-software-management/rpm#2414, probably introduced on purpose in rpm-software-management/rpm#2278 because RPM wants to remove the internal OpenPGP parser rpm-software-management/rpm#2414.

@ppisar ppisar changed the title 1.16.0: test suite is failing in test_main unit If librepo is built with -DUSE_GPGME and RPM with -DWITH_INTERNAL_OPENPGP=ON -DWITH_OPENSSL=ON, test_gpg_check_signature fails: Assertion 'ret' failed Sep 15, 2023
@ppisar ppisar added the Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take label Sep 15, 2023
@kloczek
Copy link
Author

kloczek commented Sep 15, 2023

This is a bug in RPM's internal OpenPGP parser

OK 👍
I'll subscribe to that ticket to be able test ASP possible solution.

@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

I pasted a wrong RPM issue number. The correct one is rpm-software-management/rpm#2512.

ppisar added a commit to ppisar/librepo that referenced this issue Sep 15, 2023
When debugging a test failure with RPM using internal OpenPGP+OpenSSL
implementation (that's a bug in RPM, not in librepo), I discovered
that librepo tests did not print error messages and that
lr_gpg_check_signature() did not forwarded an error message from RPM.

RPM before rpm-4.19.0-alpha2 did not provided provided any error
messages. That has changed with new functions pgpPrtParams2() and
pgpVerifySignature2().

This patch enhances librepo code to use the new RPM functions if
available and to propagate the RPM error messages via an already
existing GError argument.

This patch also enhances librepo tests to actually print the
unexpected error messages.

Both enhancements should help people to debug their failures.

Nonetheless, internal OpenPGP implementation in RPM does not set any
error messages and that will probably not change because RPM is going
to remove that implementation. On the other hand, Sequoia
implementation in RPM forwards the messages from Sequoia library. Yet
I was unbable to obtain any message. Sequoia promissed to improve
their error messaging, especially with a demise of SHA-1. So I believe
this librepo enhancement is useful.

Implementation details: I wrapped pgpPrtParams2() into a function
because it's called at multiple places. Contrary I did not wrap
pgpVerifySignature2() because it's called only at one place.

rpm-software-management#281
ppisar added a commit to ppisar/librepo that referenced this issue Sep 15, 2023
ppisar added a commit to ppisar/librepo that referenced this issue Sep 15, 2023
Also explain that RPM internal OpenPGP parser is buggy.

rpm-software-management#281
@ppisar
Copy link
Contributor

ppisar commented Sep 15, 2023

I will close the report as the bug is not in librepo. To prevent from confusion, we will enhance librepo documentation (#283) and error messages (#282).

@ppisar ppisar closed this as completed Sep 15, 2023
@kloczek
Copy link
Author

kloczek commented Sep 15, 2023

OK 👍
So my understanding is that similar effect in libdnf rpm-software-management/libdnf#1620 is result of the same cause ..

@ppisar
Copy link
Contributor

ppisar commented Sep 18, 2023

It's plausible. The "repomd.xml GPG signature verification error" error comes from librepo's lr_check_repomd_xml_asc_availability() which calls lr_gpg_check_signature() function which is exactly the same function investigated in this issue.

kontura pushed a commit that referenced this issue Sep 25, 2023
Also explain that RPM internal OpenPGP parser is buggy.

#281
ppisar added a commit to ppisar/librepo that referenced this issue Oct 3, 2023
When debugging a test failure with RPM using internal OpenPGP+OpenSSL
implementation (that's a bug in RPM, not in librepo), I discovered
that librepo tests did not print error messages and that
lr_gpg_check_signature() did not forwarded an error message from RPM.

RPM before rpm-4.19.0-alpha2 did not provided provided any error
messages. That has changed with new functions pgpPrtParams2() and
pgpVerifySignature2().

This patch enhances librepo code to use the new RPM functions if
available and to propagate the RPM error messages via an already
existing GError argument.

This patch also enhances librepo tests to actually print the
unexpected error messages.

Both enhancements should help people to debug their failures.

Nonetheless, internal OpenPGP implementation in RPM does not set any
error messages and that will probably not change because RPM is going
to remove that implementation. On the other hand, Sequoia
implementation in RPM forwards the messages from Sequoia library. Yet
I was unbable to obtain any message. Sequoia promissed to improve
their error messaging, especially with a demise of SHA-1. So I believe
this librepo enhancement is useful.

Implementation details: I wrapped pgpPrtParams2() into a function
because it's called at multiple places. Contrary I did not wrap
pgpVerifySignature2() because it's called only at one place.

rpm-software-management#281
jrohel pushed a commit that referenced this issue Oct 4, 2023
When debugging a test failure with RPM using internal OpenPGP+OpenSSL
implementation (that's a bug in RPM, not in librepo), I discovered
that librepo tests did not print error messages and that
lr_gpg_check_signature() did not forwarded an error message from RPM.

RPM before rpm-4.19.0-alpha2 did not provided provided any error
messages. That has changed with new functions pgpPrtParams2() and
pgpVerifySignature2().

This patch enhances librepo code to use the new RPM functions if
available and to propagate the RPM error messages via an already
existing GError argument.

This patch also enhances librepo tests to actually print the
unexpected error messages.

Both enhancements should help people to debug their failures.

Nonetheless, internal OpenPGP implementation in RPM does not set any
error messages and that will probably not change because RPM is going
to remove that implementation. On the other hand, Sequoia
implementation in RPM forwards the messages from Sequoia library. Yet
I was unbable to obtain any message. Sequoia promissed to improve
their error messaging, especially with a demise of SHA-1. So I believe
this librepo enhancement is useful.

Implementation details: I wrapped pgpPrtParams2() into a function
because it's called at multiple places. Contrary I did not wrap
pgpVerifySignature2() because it's called only at one place.

#281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take
Projects
None yet
Development

No branches or pull requests

2 participants