-
Notifications
You must be signed in to change notification settings - Fork 34
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
EIP-0028 ErgoAuth #53
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Alexander Slesarenko <[email protected]>
use ErgoPay to send the address to the dApp automatically). | ||
|
||
2) The dApp determines that authenticating the user is needed. For this, the dApp prepares a unique | ||
message that the wallet app should sign with a user's private key, and a SigmaBoolean that the user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible vulnerability. If I understand correctly the link has to be generated on BE. In order to do that Client need to make a request. Here is a scenario:
- dApp A (Good dapp) and dApp B (Bad actor) implement the same authentication.
- Client want to sign to dApp B.
- dApp B makes a request to dApp A to get a unique message.
- dApp B ask client to sign that message. and client signs/submits it to dApp B
- dApp B submit the signature to dApp A in exchange for an authentication token, now dAppB's dev have an access to Client's dApp A protected routes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really a vulnerability, depending on context it could be wanted, but it could also be unwanted.
I see following possibility to work against an unwanted case:
- Wallet application adds the hostname of the replyto endpoint to the signed message. This way, a dApp can check if the message was directly sent or relayed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the hostname added to signedMessage
sounds like a really good idea, a timestamp can be also a good addition.
signedMessage
pattern suggestions:
We can enforce a pattern that can be simple to parse and verify, for this a semicolon delimited string seems like a good choice, a JSON formatted string is an alternative, a bit verbose one but even simpler to parse.
Proposed fields
origin
: domain, subdomains and TLDtimestamp
: Unix timestamp at the moment of the signature. Useful for dApps to guarantee message validity periods.
Semicolon delimited string
- Pattern:
"signingMessage;origin;timestamp;randomBytes"
- Example
"someRandomString;www.sigmavalley.org;1656538465;b955cba40ceb"
JSON formatted string
- Pattern:
"{signingMessage:string,origin:string,timestamp:string,randomBytes:string}"
- Example:
'{"signingMessage":"someRandomString","origin":"www.sigmavalley.org","timestamp":"1656538465","randomBytes":"b955cba40ceb"}'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think semicolon separation is okay here. To make it more random, a prefix could be used:
signingMessageRandomByteso:originRandomBytes
so dApp can check if o:(own host name) is present.
Timestamp does not hurt to have, but in my opinion it is not really necessary since the creation of the signingMessage can be persisted on dApp's side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, great! Not sure about the prefix tho. We can make it more random by just increasing the randomBytes
length, I'm using an UUID here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, my fear is that the ones using it are not aware of the implications
I don't see any problem on having it honestly.
EDIT: But of course that doesn't mean we can't add it. I just see more difficulties here for dApps in finding the timestamp within the signed message, while the hostname is pretty easy to find with a simple contains()
If we use the pattern I suggested above, a dev can check it as simple as signedMessage.split(";")[2]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MrStahlfelge @aslesarenko I'm not talking about the man in a middle attack, I'm talking about a much simpler case of dapp A propagating users' requests to server B to get access. Anyone can do this no hacking skills are needed and encryption will not help here. User thinks that he is signing a message of A, while in reality, it's a message of B. Once A has a signed message from the user it can submit it to B, now A have an access to B.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MrStahlfelge @capt-nemo429 , what really can solve it is a simple solution. And let me know if this is what you are guys were talking about the whole time. :D
-
The message must include the domain (origin)
-
Wallets (dapp connector) should prohibit from user to sign message if the
message.split(':')[0] !== <window.origin>
exactly how CORS policies on browser side works (or maybe just set a huge warning like in burn token case?)
-
Server should validate that the message includes the origin.
Without the wallet part, it will not work. If wallets enforces the message standard AppA will not be able to substitute the message with AppB's message. And again if this is what you were talking about all the time, sorry for the misunderstanding!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then again @MrStahlfelge why you want the server to generate that message and not the wallet itself?
I think server should be an option and if message is not provided the wallet could generate it using timestamp:origin:randombytes pattern or whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take discussion to the main PR thread
By sending the token, the user proofed to have access privileges to the token. | ||
However, sending token around is not always desirable. Especially for valuable tokens, users might | ||
not want to send it away, and doing two transactions (one to send it to the depositary address, one | ||
to refund it back) costs both time and transaction fees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User can just send a token to himself. And there is no need to submit that transaction either, just to check that the transaction is valid via transaction/check endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ErgoAuth should be based on asymmetric encryption of messages between dApp (which have pk/sk key pair) and wallet users (which also have at least one pk/sk key pair).
If all the messages of the described ErgoAuth protocol got encrypted with a public key of the receiver, then the whole protocol will be safe and the messages can be sent via public channels.
When dApp sends ErgoAuthRequest, it should be encrypted with user's pk. (so only owner of user sk can read it)
Similarly, when user sends ErgoAuthResponse is should be encrypted with dApp's pk (so only dAppA can read it)
Maybe we should step back and go to the root cause of the problem. We discuss at the moment a problem that a man in the middle getting knowledge of a signed message can use it to authenticate to another dapp. Why do we have this problem here, but not in signed transactions in the first place? Because for transactions, the user get revealed what he actually is going to sign, while for now this is not the case here for ErgoAuth. The message that is signed is hidden to the user and only the issueing dapp knows what it is used for. To overcome this, we can define that the message to sign must contain a plain text message to be shown to the user, that explains in simple language what he is going to sign for. This plain text message could be prefixed to the message to sign and separated by a 0 byte. "Login to getblok.io configuration @ 06 Jul 22 15:30" etc etc By this, the user knows what he is going to sign and we can get rid of the unsolvable problem of insecure transmission channels. Additionally, dapps can persist the signed message and signature and have a proof that the user actually signed to do an outlined action. |
…eplyToUrl to the same host as requestUrl
As an alternative to implementing encryption as part of this EIP, we can restrict this EIP to work only over secure https connections, also requiring the UI to show connection security status to the user (same as it is done in browsers) |
This is now part of the specification: To fetch the auth request:
For the replyToUrl:
So https is already enforced, I will check possibilities for showing the certificate information. |
I have checked how FIDO works and it has the same concept to prevent middleman attacks. So I am confident we are good to go with this one. On a side note, ZelID only uses short living signing messages. This is recommended to do for the dApps anyway. |
ErgoAuth: user authentication protocol between wallet applications and dApps