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

Add relay access requests #1079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

staab
Copy link
Member

@staab staab commented Feb 23, 2024

The use case here is private relays that can be joined by a user with minimal friction. The idea is that a user can auth using an invite code instead of a challenge. Relays should then save that code and use the normal challenge flow in the future. Currently implemented in Coracle and Triflector/Khatru.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Feb 23, 2024

Are you using claim just because the challenge has different semantics like the one below?

The challenge is valid for the duration of the connection or until another challenge is sent by the relay.

For clients, it doesn't matter if it is a claim or challenge. They could be just one tag. If the challenge comes from the relay in one connection, its duration is for the connection. If the challenge comes from an external source, the challenge might be long-lived.

I assume relays store the claim and as soon as somebody joins with a valid claim, it accepts that pub key as invited. That could work with challenge as well. As long as the relay has it stored somewhere, it doesn't need to be the temporary challenge it creates for the connection.

I am not sure what is the benefit of separating the two.

Otherwise, looks good.

@staab
Copy link
Member Author

staab commented Feb 23, 2024

Interesting. I've gone through about 4 designs with "claim" before getting here, so it hadn't occurred to me that they could be the same thing. But I think it would be good to keep them separate, because a claim grants access, while a challenge only prevents replay attacks.

@fiatjaf
Copy link
Member

fiatjaf commented Feb 24, 2024

Why not use a different dedicated event kind and send these claims with a simple ["EVENT", ...]?

I imagine the idea behind using AUTH is that these events won't be broadcasted, but a relay that supports the claim protocol would also not broadcast these events -- and even if it did the claim would already have been claimed at the time anyone else got it.

@monlovesmango
Copy link
Member

monlovesmango commented Feb 25, 2024

just created #1082 for authentication delegation. I think could accomplish what you are trying to doing with claim? but is also supposed to be more general purpose and extensible as we think of new things we want to delegate authentication for.

didn't properly understand claims

@staab
Copy link
Member Author

staab commented Feb 26, 2024

Why not use a different dedicated event kind and send these claims with a simple ["EVENT", ...]?

I imagine the idea behind using AUTH is that these events won't be broadcasted, but a relay that supports the claim protocol would also not broadcast these events -- and even if it did the claim would already have been claimed at the time anyone else got it.

I originally did add a new event kind, but switched to auth because of broadcasting. It's up to relays to decide whether they want to invalidate claims, the use case I'm working on uses static/reusable invite codes, so broadcasting the join even would be bad. It could be ok to use a new event kind though, since relays that support the nip wouldn't broadcast, and they'd be the only ones receiving such events unless there is a client bug. I'll look into making that work with khatru.

@monlovesmango
Copy link
Member

I think new event kind makes sense, but why not send it back in a ["AUTH", ...] message instead? would prevent non-supporting relays from storing these, and would fit into supporting relays' auth flow better.

@staab
Copy link
Member Author

staab commented Feb 26, 2024

I also went down that path, but it's much harder to add support to relays for new verbs than for new events.

@monlovesmango
Copy link
Member

for relays that have not implemented the "AUTH" verb, what use case do claims have?

@staab
Copy link
Member Author

staab commented Feb 26, 2024

for relays that have not implemented the "AUTH" verb, what use case do claims have?

None, I had originally used a new JOIN verb, should have explained that.

@fiatjaf I went back to using a new event kind. The re-broadcasting scenario is handled by making the client check the NIP 11 document for support before sending.

@monlovesmango
Copy link
Member

for relays that have not implemented the "AUTH" verb, what use case do claims have?

None

then why not use "AUTH" verb instead? if you use "EVENT", relays will have to add access handling to "EVENT" messages and clients are forced to check NIP11.

@staab
Copy link
Member Author

staab commented Feb 26, 2024

It actually adds a lot of complexity. Auth is invalid if there is no claim, and no challenge, but it can be valid if either is valid, but what if one is invalid? Using a separate verb or event is much cleaner.

@monlovesmango
Copy link
Member

how is "AUTH" with new event kind more complex than "EVENT" with new event kind? the only difference between the two is that "EVENT" handling will now need to inject itself into access control flow and for un-authenticated connections it will now have to check the event kind before being able to reject the event.

@staab
Copy link
Member Author

staab commented Feb 26, 2024

That's something that happens anyway when requesting DMs etc for a particular user, auth is triggered based on event inspection.

@monlovesmango
Copy link
Member

requesting DMs would use "REQ" message, not "EVENT" message.

right now authenticated relays can automatically reject any "EVENT" message if connection is not authenticated. this would add additional logic there. not the end of the world, but using new event kind with "AUTH" message would be simpler and make more sense from design perspective (since claim use case only applies to authenticated relays).

@staab
Copy link
Member Author

staab commented Feb 26, 2024

requesting DMs would use "REQ" message, not "EVENT" message.

Right, confused myself, but the same is true about selectively accepting events.

using new event kind with "AUTH" message

I finally understand what you're saying, this does actually make sense. You're saying use the AUTH verb with the new event, and the broadcast policy will already be in place?

@monlovesmango
Copy link
Member

yes!

@staab
Copy link
Member Author

staab commented Feb 28, 2024

Interesting. I suppose that doesn't seem any worse than anything else, but I don't think we can assume that relays will handle a new event kind properly. @fiatjaf told me, and I think I agree, that he thinks the AUTH verb was a mistake (at least the one that clients publish), because it doesn't really carry any semantics on its own. This is especially true if we add more kinds to the auth verb. I'm inclined to use EVENT, and not expand the weirdness of what AUTH means.

@alexgleason
Copy link
Member

I was planning to also do something very similar to this. But I was thinking of doing a DVM flow so you can get feedback events including the ability to charge for access.

@staab
Copy link
Member Author

staab commented Feb 28, 2024

You're saying the DVM would be in the relay? I like that a lot.

@alexgleason
Copy link
Member

You're saying the DVM would be in the relay? I like that a lot.

Yes exactly. It would work really well for this. Invite code is an optional param. Feedback events are also basically needed for a flow like this, because they could demand an access code (or not), or tell you if the access code expired, or any number of other things. Then you also get the "success" event baked into the flow.

Check also: nostr-protocol/data-vending-machines#25

In Ditto, a traditional "registration" would be a combination of a NIP05 request and a relay access request.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Feb 28, 2024

Doing authorization tasks via events makes more sense than coding separate schemes in the Nostr protocol. Claim is an authorization protocol, not authentication. Authentication probably should remain in the core protocol, but authorization can be managed by a separate DVM-like thing.

But maybe the DVM spec is too general and not general enough for authorization. Making a NIP designed to run "jobs" into a more general "bot that replies to stuff" is challenging and one of the reasons NIP-46 and NIP-47 are not DVMs.

@alexgleason
Copy link
Member

DVMs in the 59xx range are appropriate for just about anything that would normally be done over HTTP (request and response), except even better because you can get multiple responses over an extended period of time.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Feb 28, 2024

Yes, but HTTP-like calls fit just the bare minimum of computer science needs. In practice, it's more like forcing everything to be async, atomic, and UDP-like (DVMs might not reply to a request). Sure, devs can make anything work in that setting, you just need to deal with the side effects manually.

For instance, there is no concept of a transaction involving multiple calls. The way we solved it via DVMs was to have a temporary replaceable event kind to assemble the atomic call. Multiple DVM requests change that event and clients can watch the event changing to make sure the DVM processed the request. When everything is ready, the user can hit the button and make another DVM run the replaceable event atomically. It's like coding in Assembly in 2024.

Authorization in production is one of these things that frequently requires adjusting many permissions in an all-or-nothing way.

@alexgleason
Copy link
Member

@vitorpamplona I think I understand what you're saying, but is it actually a response to using a NIP-90 flow for registering an account on a relay? If so I don't get the objection to it, if any. It seems like you're just saying you can't use DVMs to do literally everything. But you can use it for this.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Feb 28, 2024

Yes. It's not an objection. It's just like sometimes we use the word DVM to represent any type of Bot in Nostr for lack of a better term. And blindly applying the concept of DVMs into these bots can sometimes create more problems than solutions.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Feb 28, 2024

However, I have to say that I don't like the idea of bloated relays (relay + DVM together) in general. I think they are inevitable for some applications but it's way too centralizing, giving too much power to relay operators, for my taste.

Case in mind: A hospital's private relay that no one outside the hospital has access to is not that different from the massively centralized EHRs we have today in the US. Nostr is not really helping anything over there.

@alexgleason
Copy link
Member

DVM design is modeled after AI use-cases, but the 59xx range is appropriate to solve common Nostr problems outside of what regular clients can do. We already have scheduled posting in there: https://github.com/nostr-protocol/data-vending-machines/blob/master/kinds/5905.md

There are only 100 numbers in the range. But I think "relay access request" and "NIP-05 registration" are widespread enough to deserve inclusion, and NIP-90 offers the best Nostr-native flow for doing it.

I will also add, the DVM doesn't necessarily need to be "baked-in" to the relay. It should just be a bot that updates a pubkey whitelist the relay reads from. It's pubkey should match the NIP-11 pubkey. But it could be baked into a relay.

@alexgleason
Copy link
Member

@mleku
Copy link

mleku commented Mar 12, 2024

just random thought but i already have got relay identities and a filter using khatru's extensible function arrays one is an AuthCheck for OnConnect and the second is a Chat that goes in the StoreEVent array

these can be seen at https://mleku.dev/git/replicatr in the main.go around line 190 you find these two methods referenced (they live inside the /app directory)

there is the requirement for the relay to have an identity key, and a user then follows the relay ID (just make a link to their account profile) and then you message it, it would be simpler and not require touching the NIPs at all to just make it into a chat control feature

the reason why i am constantly nagging everyone to get their clients supporting - PROPERLY - NIP-42 is precisely to enable this kind of control interface, first and foremost, the metadata leak prevention is secondary to my reasons

with this you then just plug in an ACL system through the chat (which i am also in the process of doing) and you could easily have it so you just message the relay ID with your invite code and it automatically adds you to the ACL and voila

no nips need to be tweaked for this

i considered making special events that would maybe need to eventually go into the NIPs but they can just be internal and not necessarily go on the NIPs at least not right away, that would be internal only, require NIP-42 to function outside of the chat interface, and let you store the ACL changes as special kind events

this has a universal applicability, i'd be really chuffed if we could get this to become a standard thing because it would make relay management so much simpler, you can do all kinds of cool stuff with a secured chat interface

also, yes, it does have a workaround to prevent replay attacks, if the user hasn't already been authed via NIP-42 in the DM it can send a challenge string with instructions to respond with it and then it auths the connection

@staab
Copy link
Member Author

staab commented Mar 12, 2024

It seems simpler, but you're really creating an unspecified sub-protocol nested inside chat. Which, if it's to be interoperable, needs to be specified anyway. So we may as well specify it at the top level.

@mleku
Copy link

mleku commented Mar 13, 2024

It seems simpler, but you're really creating an unspecified sub-protocol nested inside chat. Which, if it's to be interoperable, needs to be specified anyway. So we may as well specify it at the top level.

it's just a CLI interface

it's too simple to need complex specification when it can be self documenting as well

i say don't bloat the spec if you don't have to, the smaller the spec the better it is supported

@mikedilger
Copy link
Contributor

Using EVENT with an ephemeral kind and not having any searchable tags makes the event less likely to be broadcast to other client's REQs, but of course the NIP should also say not to do it (some relays will serve a request for just kind).

This logic transfers over to local delete. Instead of a new verb, it can also be an ephemeral event with no single-letter tags which the relay shouldn't broadcast (if it does it's no huge issue though).

I think this is a reasonable PR, so +1. Is it implemented somewhere?

@staab staab mentioned this pull request Oct 9, 2024
3 tasks
@staab staab force-pushed the claims branch 3 times, most recently from e8faa8b to fee9405 Compare October 16, 2024 16:52
@fiatjaf
Copy link
Member

fiatjaf commented Oct 16, 2024

I like this but now I don't understand what is the difference between 28934 and 28935. Also how can you claim access to anything with an invoice? If you're paying to join the relay shouldn't the relay be giving you the invoice and you be paying it?

Maybe add a more concrete flow example of how and when the events can be used to help others that are as stupid as I.

@staab
Copy link
Member Author

staab commented Oct 16, 2024

They're basically the same event, and you could use the same kind for both, but semantically they're different so I opted for a new kind. In the case of an invite, you'd do a REQ for a 28935, share the url+claim (rather than the event), and then it could be used as the payload of a 28934 sent to that same relay. For invoices, you'd do the same thing, but pay the invoice before sending the claim. The relay would then look up the invoice and see whether it was paid. But I'm not sure about that flow, it's not something I'm currently working on, and overloading the claim field would require parsing the claim, which seems like a bad idea to me.

@staab
Copy link
Member Author

staab commented Oct 16, 2024

I've removed the invoice stuff, we can add new use cases with new tags if we need them.

@fiatjaf
Copy link
Member

fiatjaf commented Oct 16, 2024

So Bob does a REQ for a 28935, the relay supposedly gives him a thing it just generated on the fly, Bob takes the claim tag out of that, then shares it with Alice, Alice makes a 28935 with that claim tag and sends to the relay as an EVENT?

Looks pretty good to me.

@staab staab mentioned this pull request Nov 19, 2024
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.

7 participants