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

feat(test): Add a new test to ensure proper signature checking #100

Merged

Conversation

vincenzopalazzo
Copy link
Collaborator

Fixes a bug in lnprototest by removing the problematic code outlined in patch [1].

During our investigation of the cln code, we discovered that the message verification was not performed correctly as the BOL 7 suggest. This commit includes a patch [2] that fixes the issue and introduces an integration test to validate that core lightning adheres to the signature verification guidelines outlined in BOLT7.

[1] #91
[2] ElementsProject/lightning#6384

Reported-by: lnprototest (#91)

P.S: now the integration testing should fails because the patch in core lightning is still open

@vincenzopalazzo vincenzopalazzo force-pushed the macros/verify-signature-announcement_signatures branch from d7b7e89 to ffaa96f Compare July 14, 2023 23:05
@vincenzopalazzo vincenzopalazzo added this to the lnprototest 0.1.0 milestone Jul 15, 2023
Fixes a bug in lnprototest by removing the problematic
code outlined in patch [1].

During our investigation of the cln code, we discovered that
the message verification was not performed correctly as the BOL 7
suggest. This commit includes a patch [2] that fixes the issue and
introduces an integration test to validate that core lightning adheres
to the signature verification guidelines outlined in BOLT7.

[1] #91
[2] ElementsProject/lightning#6384

Reported-by: lnprototest (#91)
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/verify-signature-announcement_signatures branch from ba026c3 to c518325 Compare July 15, 2023 00:59
@vincenzopalazzo vincenzopalazzo force-pushed the macros/verify-signature-announcement_signatures branch from c518325 to ecc260a Compare July 15, 2023 01:02
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Jul 15, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Jul 15, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Jul 25, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Jul 25, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Jul 25, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
rustyrussell pushed a commit to ElementsProject/lightning that referenced this pull request Jul 25, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo
Copy link
Collaborator Author

Merging now that cln contains the feature ElementsProject/lightning#6384

@vincenzopalazzo vincenzopalazzo merged commit 1ecb70c into master Jul 26, 2023
3 of 4 checks passed
@vincenzopalazzo vincenzopalazzo deleted the macros/verify-signature-announcement_signatures branch July 26, 2023 00:35
ddustin pushed a commit to ddustin/lightning that referenced this pull request Jul 26, 2023
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:

- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.

[1] rustyrussell/lnprototest#100

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
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.

None yet

1 participant