shadowsocks / shadowsocks-org

www.shadowsocks.org
MIT License
882 stars 541 forks source link

SIP002 - Optional extension configurations as query strings in ss URLs #27

Closed Mygod closed 7 years ago

Mygod commented 7 years ago

Shadowsocks Improvement Proposal 002 (actually why don't we just use issue number to refer to them instead)

Optional configurations as query strings in ss URLs

Since #26, there are at least two optional extension for shadowsocks as far as I can tell:

There may be more extensions in the future. So what about adding them to ss URLs? For example, we can have ss://...?kcpport=8839&kcpcli=--crypt+none......#a+name for easier configuration. Clients that don't support the extensions can safely ignore them.

What should be included:

What shouldn't:

Problems:

Final version:

SIP002 purposed a new URL schema, following RFC3986:

SS-URI = "ss://" userinfo "@" hostname ":" port [ "/" ] [ "?" query ] [ "#" fragment ]
userinfo = websafe-base64-encode-utf8(method  ":" password)

The last / should be appended if query or fragment is present. Example: ss://YmYtY2ZiOnRlc3Q@192.168.100.1:8888/?plugin=url-encoded-plugin-argument-value&unsupported-arguments=should-be-ignored#Dummy+profile+name. This kind of URIs can be parsed by standard libraries provided by most languages.

For plugin argument, we use the similar format as TOR_PT_SERVER_TRANSPORT_OPTIONS, which have the format like simple-obfs;obfs=http;obfs-host=www.baidu.com where colons, semicolons, equal signs and backslashes MUST be escaped with a backslash.

Mygod commented 7 years ago

Because in URL there is a field specifically for user info which is perfect for method and password. Using base64 prevents illegal characters in URL since we want the URL to be valid.

You are welcome to give compatibility suggestions here. I didn't think a lot about that.

riobard commented 7 years ago

Thanks for the explanation! Now I see how we ended up here. I have the following suggestions:

  1. We should deprecate the strange "matrix" URI that is ss://[base64-encoded] form. It's not legal URI and it doesn't make any sense, as discussed in #50.
  2. We should not base64 encode the userinfo field either. RFC 3986 section 2 already has a standard way (percent-encoding) to represent special characters and this is universally supported.

It's still open question where and how to include password/key in the URL. RFC 3986 section 3.2.1 says password in plaintext is deprecated due to obvious security reasons.

Additionally, we should keep it in mind that it's possible that future versions of Shadowsocks might be able to support multiple users on the same port. If we use the username of userinfo to specify cipher name, we'll lose the ability to support multiuser in the future. Maybe we should leave the userinfo empty for now and instead put the cipher method in query string (e.g. /?cipher=aes-128-gcm)?

riobard commented 7 years ago

By the way RFC 3986 section 7.6 is a real concern. URL userinfo is illy-designed and a social engineering nightmare.

madeye commented 7 years ago

To keep the density of the password or key in the URL, I think BASE64-URL is still the best choice.

To support multiple-user, if we follow the current approach that encoding both cipher and password in BASE64, we should be safe.

And surprisingly, with BASE64 encoded user info, we can even avoid semantic attacks...

hellofwy commented 7 years ago

Since JSON have been used as config format, why not just use JSON to exchange config info? It's more readable.

riobard commented 7 years ago

@madeye Can you explain a little bit about how the base64-encoded userinfo work with both password and key? I didn't find any detail above.

madeye commented 7 years ago

@riobard Still not defined... Currently, we only support password here.

riobard commented 7 years ago

@madeye I thought we only need base64 to encode random keys. If we only support passwords, wouldn't it be easier to just follow the original URL userinfo format?

madeye commented 7 years ago

I thought we only need base64 to encode random keys.

It'd be doable.

If we only support passwords, wouldn't it be easier to just follow the original URL userinfo format?

Right. We just encode this part for future change.

I'm thinking of pattern like BASE64(cipher:password:<password>) and BASE64(cipher:key:<key>). Any suggestion?

riobard commented 7 years ago

How about this? For traditional password-based deployment and backward compatibility

ss://cipher:password@host:port/?extension_parameters

For new key-based deployment

ss://[username@]host:port/?cipher=aes-128-gcm&key=base64:[base64-encoded-key]

The [username@] is optional and will be used only when future versions support multiuser.

We can distinguish between the two because the new format does not have userinfo field.

madeye commented 7 years ago

Currently we are using userinfo to identify if a URL is a legacy pattern. https://github.com/shadowsocks/shadowsocks-android/blob/master/mobile/src/main/scala/com/github/shadowsocks/utils/Parser.scala#L36

Move cipher and key out of the userinfo would make things much more complicated.

Mygod commented 7 years ago
  1. Encoding method hides unnecessary details that normal user doesn't need to know.
  2. I thought embedding key into URLs is not supported.
  3. I also thought multiuser isn't going to be supported.
Mygod commented 7 years ago

Also, method-key combination is used as authentication information. I don't see the point of moving it to query parameters.

For matrix URIs, I have only seen it used in OpenWRT web front-end and nowhere else. And it doesn't make symatic sense either.

riobard commented 7 years ago

Hmm, this looks more complicated than I originally thought. Let me think a bit more about the issue.

fortuna commented 7 years ago

+1 for using JSON (https://github.com/shadowsocks/shadowsocks-org/issues/27#issuecomment-281243181)

It's more readable, and can also be used for versioning:

{
  "v1": {
    ..fields for v1 configs...
  }
}

Parsing becomes very easy:

config = JSON.parse(input);
if ("v2" in config) {
  parseV2(config.v2);
} else if ("v1" in config) {
  parseV1(config.v1);
} else {
  throw "config version not supported";
}

To make the serialization concise, it's possible to use binary formats such as CBOR or BSON and base64 encode after that to make it url-safe.

madeye commented 7 years ago

@fortuna Yes, JSON strings should be much easier to parse. However, If we encode everything in a URL, we also lose all the semantic information.

Anyway, I think encoded JSON in a URL looks doable, if we don't really care about the semantic things here.

riobard commented 7 years ago

I'm not sure. But if we really have to embed JSON into the URL, I'd suggest that we use Data URIs, something like

ss://host:port/?config=data:application/json;base64,[base64-encoded JSON]

It would also allow people to use other format e.g. YAML by

ss://host:port/?config=data:text/x-yaml;base64,[base64-encoded YAML]

But then the question becomes what would be an acceptable JSON format as there are multiple implementations with different features.

riobard commented 7 years ago

Anyway the major concern of base64 in the URL is that it's not possible for a human to manually edit it (like change cipher/password) without using an external tool. Nor is it feasible to spot misspellings, which happen quite often.

Mygod commented 7 years ago

Why would anyone want to edit it manually anyways?

riobard commented 7 years ago

@Mygod when you just setup the server and want to try if everything works?

Mygod commented 7 years ago

URLs are only used to share between mobile devices. It's not meant for everything.

riobard commented 7 years ago

@Mygod In that case it's only copy-n-paste? Then URLs are not really necessary… they can just copy and paste JSON strings.

madeye commented 7 years ago

@Mygod Right, And most of time, URLs are for QR code only.

madeye commented 7 years ago

@riobard Yes, for server and PC clients, JSON configs work much better.

riobard commented 7 years ago

OK, I need to clarify my understanding so we are on the same page:

  1. URLs are for human to read and edit, so they must be semantically correct and easy to change and spot typing errors.
  2. For users to easily setup mobile clients, QR codes are better because users can just take a photo. We don't need to worry about semantics. In this case, QR code containg base64-encoded JSON might be a better solution.

Is it correct?

hellofwy commented 7 years ago

I think the initial design to use base64 has two purpose:

  1. The password can't been seen from the first sight.
  2. Since QR is used to exchange info, so put ss:// before the base64 part, make it more recognizable.

So it's just look like URL, but it's not URL.

The initial implement is simple, but now we have so many implements and the plugins and AEAD ciphers. I think we should redesign the configuration format used for sharing.

If use JSON,we can extends it like this to add implement specific configurations:


{
    ...,
    "implement": "shadowsocks-libev@3.0.2",
    "implement-config": {
        ...
    }
}
riobard commented 7 years ago

@hellofwy Maybe we should study the config format for major implementations first and see if there's any common pattern.

hellofwy commented 7 years ago

And I think users should not edit the URLs or JSONs directly, since not everyone is family with the format.

But readability is more helpful.

The format is just use as exchange format, clients should convert it to it's local format. If they can't recognized the implement specific configurations, they should just prompt errors and show the JSON to users. Servers just provide someway to generate the configrations.

We can just focus on the standard exchange format.

madeye commented 7 years ago

@riobard The motivation of URL here is actually for sharing server info with mobile devices. As both Android and iOS supports "intents" with URL schema, a user can configure the app by clicking the URL or scan a QR code.

For example, with shadowsocks-android installed, any shadowsocks QR code scanned by any QR scanner will let system send a intent to shadowsocks and get the server configured automatically.

riobard commented 7 years ago

@madeye I see. So the "ss://" prefix is for that use case, and it does not matter if the URL is actually semantically correct, right?

madeye commented 7 years ago

@riobard Exactly.

fortuna commented 7 years ago

Re: https://github.com/shadowsocks/shadowsocks-org/issues/27#issuecomment-281556896 @hellofwy , you can avoid a type + value field in json by making the field for the value be named after the type:

{
  "shadowsocks-libev": {
    ....shadowsocks-libev config...
  }
}
Mygod commented 7 years ago
  1. URI/URL is used for clicking;
  2. Most common type of data encoded in QR codes is URI/URLs;
  3. Using data URI scheme isn't acceptable because Android can't redirect this kind of URI to our app;
  4. Embedding data URI into ss URI is even uglier.

If there isn't any more argument, I think we can finalize this design.

madeye commented 7 years ago

Right. I think we need to finalize this issue now. Shadowsocks-android has been blocked by this SIP for a while.

Since we cannot come up with a better approach than @Mygod's, let's follow this SIP for now.

If there is any further enhancement to the URL, we may take a new schema like ss2://, and then we won't worry about the backward compatibility.

riobard commented 7 years ago

If there's no obviously better plan, it's better to stick with the original non-base64-encoded SS URL (ss://cipher:password@host:port) for now.

em0minate commented 7 years ago

As I've said:

ss://... ss-base64://...

And if you really prefer JSON and base64 encoding, just pick JWT, then you could have:

ss-jwt://...
Mygod commented 7 years ago

@riobard One should never use ss URLs as the main configuration. They weren't and won't be designed for this.

@em0minate Yep. Feel free to add more URL ideas that we are NOT going to implement.

@madeye I don't see anything better. So let's keep using "ss://" { (base64-encoded) cipher ":" password } "@" host ":" port. Plugin and other future features are passed (URL-encoded) as URI query and name/comment (also URL-encoded) is passed as URI fragment.

riobard commented 7 years ago

Why not? Multiple tools already do it that way, e.g. cow, shadowproxy.

Since we're not supporting keys in the URL anyway, I don't see why the original non-base64-encoded format would not work.

Mygod commented 7 years ago
  1. Plain text password is not a recommended practice and has been deprecated (by RFC) IIRC;
  2. Base64 encoding these prevents user that doesn't know a lot about the details of the protocol changing them;
  3. Again URIs are mainly used for sharing, not configuring.
riobard commented 7 years ago
  1. Agreed. But base64-encoding the userinfo does not solve this problem.
  2. I don't see why this is necessary.
  3. Please explain why.

Overall, I think if there's no obvious benefit, let's just keep using the original format since it's been adopted by other tools already.

Mygod commented 7 years ago
  1. It doesn't. There's no real solution to this problem either.
  2. It's not. It's good to have.
  3. URIs isn't semantically clear nor compact nor easy to memorize for new users.

Another benefit is this representation is more compact if passwords have illegal characters in it in which case using URL encoded strings is less convenient. Also this format is already used in shadowsocks-android.

I agree that there's no significant benefit for using this one but there's no significant drawback either. And the old-fashioned plain text SS urls are never used and/or supported in most widespread clients. Why prioritize other projects over our own?

riobard commented 7 years ago

Well, I was just hoping to avoid one more variant of URLs… Now we have three :|

em0minate commented 7 years ago

Yep. Feel free to add more URL ideas that we are NOT going to implement.

Oh, so it's not a proposal were one should bother to having discussion at the first place?

IMO, having rough formed configuration is BAD than to have it at all, especially when pulling off a spec.

Mygod commented 7 years ago

@em0minate We're definitely not going to support 3 kinds of URLs, or infinite possibilities of URL variants. Thank you.

@madeye What do you think which one we should use?

madeye commented 7 years ago

@Mygod Let's keep using your purposed one in shadowsocks-android, since only shadowsocks-android supports SIP003 plugins for now.

The legacy URL should still be supported, as most of the known clients support it.

BTW, I only know two kinds of URLs. What's the third one?

Mygod commented 7 years ago

@madeye He's talking about everything-plain-text URL used by other projects that supports Shadowsocks protocol.

madeye commented 7 years ago

@Mygod OK, that should not be a problem for us. At least all the official desktop and mobile clients support the legacy BASE64 URL.

Mygod commented 7 years ago

Should we also keep support for generating old format if the user wishes to use it?

madeye commented 7 years ago

@Mygod I think it should be useful today.

madeye commented 7 years ago

@Mygod What about generating legacy URL for configs without plugins? It makes sense and won't break backward compatibility.