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

GPG signature verification fails incorrectly for commits with empty message #1429

Closed
castedo opened this issue Nov 11, 2024 · 2 comments · Fixed by #1441
Closed

GPG signature verification fails incorrectly for commits with empty message #1429

castedo opened this issue Nov 11, 2024 · 2 comments · Fixed by #1441

Comments

@castedo
Copy link
Contributor

castedo commented Nov 11, 2024

I've hit this problem in my messy dev environment. I figure I should make a PR for a nice clean repo case as a unit test. Do you have a recommendation on where commit signature verification test cases should go?

An example of a GPG signed commit that I am 95% sure will fail incorrectly is this one:

https://archive.softwareheritage.org/api/1/revision/92b3f55bcec30906a39796c90018475bc1cbabb7/

I think the problem is the calling of _format_message with an argument of self._message to the body parameter:

return list(_format_message(headers, self._message))

When the commit message is empty, I believe this causes the newline on this line:

yield b"\n" # There must be a new line after the headers

that is inside the if statement (instead of above it) to not get output (when it should). But when a signature-less commit is generated on this line:

self_without_gpgsig.as_raw_string(),

the GPG signature verification will fail when the commit has an empty commit message, because a newline will be missing at the end.

@jelmer
Copy link
Owner

jelmer commented Nov 12, 2024

Tests for signatures should probably just live in dulwich.objects, probably as a separate testcase that calls skipTest in its setUp() if the gpg module is not present.

@castedo
Copy link
Contributor Author

castedo commented Nov 16, 2024

I've got a nice clean reproduction case for this issue based on the unit test case I just added for PR #1438.

STEPS:

From PR #1438, inside tests/compat/test_porcelain.py CommitCreateSignTestCase.test_verify, change the message string "foo" to simply an empty string "".

RESULT:
the commit.verify() and the unit test will fail.

castedo added a commit to castedo/dulwich that referenced this issue Nov 16, 2024
* Fixes incorrect signature verification for commits with an empty message,
  as encoded by CGit (jelmer#1429)
castedo added a commit to castedo/dulwich that referenced this issue Nov 16, 2024
* Fixes incorrect signature verification for commits with an empty message,
  as encoded by CGit (jelmer#1429)
castedo added a commit to castedo/dulwich that referenced this issue Nov 16, 2024
* Fixes incorrect signature verification for commits with an empty message,
  as encoded by CGit (jelmer#1429)
castedo added a commit to castedo/dulwich that referenced this issue Nov 16, 2024
* Fixes incorrect signature verification for commits with an empty message,
  as encoded by CGit (jelmer#1429)
jelmer added a commit that referenced this issue Nov 16, 2024
Fixes #1429, incorrect signature verification for commits with an empty
message, as encoded by CGit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants