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

Not following protocol specification for multiple macaroons #143

Open
gofeuer opened this issue Jul 27, 2024 · 3 comments
Open

Not following protocol specification for multiple macaroons #143

gofeuer opened this issue Jul 27, 2024 · 3 comments

Comments

@gofeuer
Copy link

gofeuer commented Jul 27, 2024

The protocol specification here says that the macaroon portion of the authentication token, left side of : may contain multiple comma separated Macaroons.

Trying to send multiple comma separated macaroons will fail on line 72:

aperture/l402/header.go

Lines 70 to 76 in a308758

// Decode the content of the two parts of the header value.
macBase64, preimageHex := matches[2], matches[3]
macBytes, err := base64.StdEncoding.DecodeString(macBase64)
if err != nil {
return nil, lntypes.Preimage{}, fmt.Errorf("base64 "+
"decode of macaroon failed: %v", err)
}

I think this might actually be a specification problem and not a bug here.
Because what would be the use case for multiple macaroons to be passed to a request? They are all paid for my one invoice?

@guggero
Copy link
Member

guggero commented Jul 29, 2024

Thanks for the report!

I think there might be use cases, where you'll want to give out multiple macaroons with different base permissions, but all tied to the same payment hash (invoice). So I think we should update the code to allow for multiple macaroons.

@gofeuer
Copy link
Author

gofeuer commented Jul 29, 2024

Oh right, then minter.MintL402(...) should return a slice of macaroons here:

mac, paymentRequest, err := l.minter.MintL402(
context.Background(), service,
)

That would then be encoded with comma separators if the slice has more than one macaroon here:

str := fmt.Sprintf("macaroon=\"%s\", invoice=\"%s\"",
base64.StdEncoding.EncodeToString(macBytes), paymentRequest)

This will allow a use case where the minter provides a full permission macaroon followed by ever more attenuated versions of it with the properly set caveats. Allowing the client to respond with one of the received macaroons that have the caveats that grant enough access to the resource they require.

I get the client receiving multiple macaroons now.
But isn't allowing for them to provide multiple macaroons to authenticate a request an unnecessary complication?

@guggero
Copy link
Member

guggero commented Jul 31, 2024

But isn't allowing for them to provide multiple macaroons to authenticate a request an unnecessary complication?

I don't think it's too much of a hassle. We'd just validate more than one macaroon. But they would all need to be valid at the same time.

I don't remember exactly why we wrote the spec that way (it's been a while), but since it's quite explicit I think we might have had a specific use case in mind. So I think a first step would be to just validate multiple macaroons, changing this code as you mentioned initially:

aperture/l402/header.go

Lines 70 to 76 in a308758

// Decode the content of the two parts of the header value.
macBase64, preimageHex := matches[2], matches[3]
macBytes, err := base64.StdEncoding.DecodeString(macBase64)
if err != nil {
return nil, lntypes.Preimage{}, fmt.Errorf("base64 "+
"decode of macaroon failed: %v", err)
}

Then a second step would be to be able to issue multiple macaroons. Though I don't know how the client would tell the server that it wants multiple macaroons. So some feature shaping would be required here.

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

No branches or pull requests

2 participants