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

fix verify_signed_message and tests/robots/1/signed_message #1354

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jerryfletcher21
Copy link
Contributor

@jerryfletcher21 jerryfletcher21 commented Jun 26, 2024

What does this PR do?

verify_signed_message was not checking for the validity of the signature. The documentation of python-gnupg is not clear about this, it says that the fields returned by verify are set just if the signature is valid. In this case tests/robots/1/signed_message was signed with the correct key but with the wrong digest-algo (SHA256 instead of SHA512) as expressed on the signature.
Running gpg --verify tests/robots/1/signed_message after importing the key returned:

gpg: WARNING: signature digest conflict in message
gpg: Can't check signature: General error

and a non zero error code. Changing Hash: SHA256 in tests/robots/1/signed_message returned no errors.

verify_signed_message called in test_verify_signed_message in api/tests/test_utils.py was not catching this because verified.fingerprint was set even though verified.valid was False.

I have recreated the signature with SHA512 by running:

gpg --pinentry-mode loopback --passphrase-file tests/robots/1/token --import tests/robots/1/enc_priv_key

cat tests/robots/1/signed_message | head -n 4 | tail -n 1 | gpg --armor --digest-algo SHA512 --clearsign --local-user "$(gpg --show-key tests/robots/1/pub_key | head -n 2 | tail -n 1)" --pinentry-mode loopback --passphrase-file tests/robots/1/token > tests/robots/1/signed_message

Checklist before merging

  • Install pre-commit and initialize it: pip install pre-commit, then pre-commit install. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.

verify_signed_message was not checking for the validity of the
signature. The documentation of python-gnupg is not clear about this, it
says that the fiels are set just if the signature is valid.
In this case tests/robots/1/signed_message was signed with the correct
key but with the wrong digest-algo (SHA256 instead of SHA512) as
expressed on the signature.
Running gpg --verify tests/robots/1/signed_message returned:
gpg: WARNING: signature digest conflict in message
gpg: Can't check signature: General error
and a non zero error code, but verify_signed_message was not catching
this because verified.fingerprint was set even though verified.valid was
False.
@Reckless-Satoshi Reckless-Satoshi mentioned this pull request Jul 7, 2024
1 task
@Reckless-Satoshi
Copy link
Collaborator

Thanks for figuring this super sneaky one @jerryfletcher21 !

This PR needs a lot of testing as we risk getting all invoices submitted rejected.

@Reckless-Satoshi Reckless-Satoshi force-pushed the main branch 2 times, most recently from 9054b87 to f9244e1 Compare September 10, 2024 01:28
@KoalaSat
Copy link
Member

@KoalaSat
Copy link
Member

KoalaSat commented Oct 19, 2024

I managed to run an entire trade workflow with the Coordinator image generated from this commit https://hub.docker.com/layers/recksato/robosats/9e442c3/images/sha256-64343a7dc5b7c7b8805fc9e60ee8d74020c34da4a028a5c02ca492d48d9ce81a?context=explore

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 this pull request may close these issues.

3 participants