SEP-10: Make SIGNING_KEY required and change manage data op value

[JakeUrban]

resolves stellar/integration-meta#180

Makes a SEP-10 service's SIGNING_KEY required in their stellar.toml file. Also changes the key of the manage data operation from the anchor's "name" to the service's home domain.

If the home domain specified in the manage data operation doesn't match the home domain request by the client, or the SIGNING_KEY specified on the TOML file did not sign the transaction, the challenge is invalid and the client should not sign it.

[JakeUrban]

@nebolsin

[accordeiro]

This looks good to me, thanks for addressing this issue @JakeUrban! I left a small comment regarding the versioning, inline with what @nebolsin suggested.

By looking at the proposed changes, the updates should probably happen in the following sequence:

1. Server-side implementers can update anytime, since the backend change is backwards-compatible (we're still sending a tx with a 0 seqnum)
2. Client implementers should only update once the backends they integrate with are updated, otherwise they'll reject SEP-10 challenges that don't contain the server's home domain

Does that sound right?

With this in mind, should we pursue having an identifier on stellar.toml for the SEP-10 version the server implements? It might be an overkill, but just wanted to mention in case we think it might be a good idea.

[JakeUrban]

@accordeiro no problem! Regarding your points:

1. Its actually not a backwards compatible change. The anchorName parameter is being swapped for homeDomain, so the SDK's need to update their JWT generation helpers. SDK's, then server-side users of the SDK's, still need to update first.

2. I would imagine SEP-10 implementations slowly migrate to version 2.0 as we raise awareness, so I think clients will likely support both SEP-10 versions for a time. The other option is for anchors to communicate directly with known wallets and notify them of the SEP-10 update server-side, planning a coordinated effort to migrate at the same time.

Regarding your proposed stellar.toml variable, I think this can be detected by clients from the challenge. The updated SDK's will likely throw an exception if the Manage Data op contains an anchorName and not a homeDomain, which will signal to the clients which version is being used. However, I could be convinced it's worth it to have something on the stellar.toml.

[accordeiro]

1. Its actually not a backwards compatible change. The anchorName parameter is being swapped for homeDomain, so the SDK's need to update their JWT generation helpers. SDK's, then server-side users of the SDK's, still need to update first.

I agree, backwards compatible might've been the wrong term to use. What I meant to say was that the value of the key in the manage_data op was considered unimportant, so from the client perspective whatever data was inside the op couldn't have been used as validation criteria in the past.

So even if the backends update their implementations (i.e. we make our producers output more strict), it shouldn't break any consumers as long as they don't update their client SDKs (i.e. as long as we don't make our consumers validation more strict). Does that make sense? So the overall strategy of updating servers before clients seems sound to me.

2. I would imagine SEP-10 implementations slowly migrate to version 2.0 as we raise awareness, so I think clients will likely support both SEP-10 versions for a time. The other option is for anchors to communicate directly with known wallets and notify them of the SEP-10 update server-side, planning a coordinated effort to migrate at the same time.

Regarding your proposed stellar.toml variable, I think this can be detected by clients from the challenge. The updated SDK's will likely throw an exception if the Manage Data op contains an anchorName and not a homeDomain, which will signal to the clients which version is being used. However, I could be convinced it's worth it to have something on the stellar.toml.

Makes sense to me. I agree they can easily tell which version by looking at the manage_data op, so the stellar.toml change is redundant. We can defer that to whenever we make any changes that aren't easily detectable, if that happens in the future.

[nebolsin]

I believe manage_data key is defined in xdr as string<64> (as opposed to the value, which is opaque<64>). We can use all 64 characters here, and there's no need for encoding.

Also, <home domain> here is not the signing server's FQDN, but rather a home domain of the web service the client is authenticating to. Signing server might be on a completely different domain (as defined by the stellar.toml).

So I would suggest <home domain> auth, which is service's home domain (max. 59 chars), followed by a space and the word auth.

Actually, that makes me think that we need to pass the target web service home domain as an (optional?) parameter to the challenge endpoint, because the auth server might serve several different services (one.example.com and another.example.com both point to auth.example.com in their stellar.toml), or even offered as a service, similar to what stellarid.io does for federation.

[JakeUrban]

We can add an optional argument for the web services home domain, but it shouldn't be made mandatory. Signing servers that serve multiple other web services can use a default if not specified.

Making the changes regarding <home domain> encoding

[nebolsin]

Looks good to me 👍