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

Clarify choice of passphrase in SEP-10 #345

Merged
merged 1 commit into from
Jul 10, 2019
Merged

Conversation

tomquisel
Copy link
Contributor

@tomquisel tomquisel commented Jul 9, 2019

  • Clarify choice of passphrase for signatures in SEP-10
  • Also add an optional response parameter to the challenge endpoint: network_passphrase to make debugging signature issues easier.
  • Replace outdated reference to SEP-6 with reference to SEP-12

Addresses problem mentioned in stellar/go#1468
Closes #339

@nebolsin I'd love your feedback.

@tomquisel tomquisel requested a review from bartekn July 9, 2019 21:38
* The value of key is not important, but can be the name of the anchor followed by `auth`. It can be at most 64 bytes.
* The value must be a 64 byte long base64 encoded cryptographic-quality random string
* signature by the web service signing account
* `network_passphrase`: (optional but recommended) Stellar network passphrase used by the server. This allows the client to verify that it's using the correct passphrase when signing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me. I'm a little bit worried that network_passphrase is optional. It requires code that handles this situation and if developer forgets about it it may result in null pointer exceptions or using empty passphrase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I think this is a necessary evil to make the change backwards compatible.

@nebolsin
Copy link
Contributor

I want to point out that server’s network passphrase is implicitly bound to the challenge transaction as part of server’s signature, and the client is required to “verify that transaction envelope has a correct signature by server's signing key“, which I think provides a reliable way to protect from network mismatch issues and halt the flow before it gets to client-side signing.

It might be convenient to extend the response with network_passphrase (or should it actually be specified in stellar.toml? looks like currently SEP-1 doesn’t define this field) to simplify debugging or improve error reporting, but I think that the server’s signature check should still act as the ultimate verification.

@tomquisel tomquisel merged commit af5cf14 into master Jul 10, 2019
@bartekn
Copy link
Contributor

bartekn commented Jul 10, 2019

I want to point out that server’s network passphrase is implicitly bound to the challenge transaction as part of server’s signature, and the client is required to “verify that transaction envelope has a correct signature by server's signing key“, which I think provides a reliable way to protect from network mismatch issues and halt the flow before it gets to client-side signing.

I think the problem we're trying to solve is having to guess the passphrase if the first signature is incorrect. I agree that in most cases it will be pubnet or testnet passphrase but it's not defined anywhere so in reality the passphrase can be anything. Also, in #339 (comment) I pointed out that we had very similar problem in the past (two components were talking to each other but using different passphrases) and it was really difficult to debug.

@tomquisel tomquisel deleted the tomquisel-patch-2 branch July 24, 2019 21:17
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.

SEP-10 should define a network passphrase used when signing challenge transaction
3 participants