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

macaroons: reject unknown macaroon versions #8132

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

Roasbeef
Copy link
Member

We've only ever made macaroons with the v2 versions, so we should explicitly reject those that aren't actually v2. We add a basic test along the way, and also add a similar check for the version encoded in the macaroon ID.

@Roasbeef Roasbeef added this to the v0.17.1 milestone Oct 30, 2023
@Roasbeef Roasbeef requested a review from Crypt-iQ October 30, 2023 23:49
@Roasbeef Roasbeef force-pushed the strict-macaroon-version branch from 0e2520f to 72647e4 Compare October 30, 2023 23:49
Copy link
Member

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Think we need to update the itest to reflect the latest error message.

macaroons/service.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@Crypt-iQ Crypt-iQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once @yyforyongyu comment is addressed 🥍

EDIT: The macaroon_authentication/invalid_macaroon itest is failing because the error string "cannot get macaroon" should be replaced by "invalid ID"

macaroons/service.go Outdated Show resolved Hide resolved
@Roasbeef Roasbeef force-pushed the strict-macaroon-version branch 2 times, most recently from 0152523 to f2562d7 Compare October 31, 2023 17:51
We've only ever made macaroons with the v2 versions, so we should
explicitly reject those that aren't actually v2. We add a basic test
along the way, and also add a similar check for the version encoded in
the macaroon ID.
@Roasbeef Roasbeef force-pushed the strict-macaroon-version branch from f2562d7 to 9287b75 Compare October 31, 2023 20:24
@Roasbeef Roasbeef merged commit b0261f7 into lightningnetwork:master Oct 31, 2023
23 of 25 checks passed
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