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

Plugin system [Draft] #1073

Closed
2 of 3 tasks
Mygod opened this issue Jan 9, 2017 · 47 comments
Closed
2 of 3 tasks

Plugin system [Draft] #1073

Mygod opened this issue Jan 9, 2017 · 47 comments
Milestone

Comments

@Mygod
Copy link
Contributor

Mygod commented Jan 9, 2017

Plugin should be bundled as an apk. $PLUGIN_ID in this draft corresponds to the executable name for the plugin in order to be cross-platform, e.g. simple-obfs. An apk can have more than one plugins bundled. We don't care as long as they have different $PLUGIN_ID. Duplicated plugins will be disabled so user has to uninstall them until there's only not more than one left.

There is no restrictions/requirements on package name, but you're suggested to use com.github.shadowsocks.plugin.$PLUGIN_ID if it only contains a single plugin to prevent duplicate plugins.

There will be a library to implement the base framework contrived in this draft and some example plugins for the ease of developers. 😄

1. Plugin configuration

Plugins get their args configured via one of the following two options:

Configuration activity

  • Should this be implemented at all?

If the plugin provides a configuration activity, it will be started when user picks your plugin and taps configure. It:

  • MUST have action: com.github.shadowsocks.plugin.$PLUGIN_ID.ACTION_CONFIGURE;
  • SHOULD parse string extra com.github.shadowsocks.plugin.EXTRA_OPTIONS (all arg as a single string) and display the old settings;
  • MUST return the data Intent with the new com.github.shadowsocks.plugin.EXTRA_OPTIONS;
  • SHOULD distinguish between server settings and feature settings in some way, e.g. for simple_obfs, obfs is a server setting and obfs_host is a feature setting;
  • NEED NOT be consistent with shadowsocks-android styling - you don't need to use preferences UI at all if you don't feel like it - however it's recommended to use Material Design at minimum.

Help activity/callback

If the plugin doesn't provide a configuration activity, it's highly recommended to provide a help message in the form of an Activity. It:

  • MUST have action: com.github.shadowsocks.plugin.$PLUGIN_ID.ACTION_HELP;
  • CAN parse string extra com.github.shadowsocks.plugin.EXTRA_OPTIONS and display some more relevant information;
  • MUST be either:
    • Be invisible and return help message with CharSequence extra com.github.shadowsocks.plugin.EXTRA_HELP_MESSAGE in the data intent; (in this case, default dialog will be shown containing the message)
    • Be visible and return null as data intent.
  • SHOULD distinguish between server settings and feature settings in some way, e.g. for simple_obfs, obfs is a server setting and obfs_host is a feature setting.

2. Plugin implementation

Every plugin can be either in native mode or JVM mode.

Native mode

In native mode, plugins are provided as native executables and shadowsocks-libev's plugin mode will be used.

Every native mode plugin MUST have a content provider to provide the native executables (since they can exceed 1M which is the limit of Intent size) that:

  • MUST have android:label and android:icon; (may be configured by activity or application)
  • MUST have action: com.github.shadowsocks.plugin.ACTION_NATIVE_PLUGIN. (used for discovering plugins)
  • MUST have android:authorities set to com.github.shadowsocks.plugin.$PLUGIN_ID;
  • MUST implement query that returns the file list which MUST include $PLUGIN_ID when having these as arguments:
    • uri = "content://com.github.shadowsocks.plugin.$PLUGIN_ID";
    • projection = ["path"]; (relative path, for example obfs-local)
    • selection = null;
    • selectionArgs = null;
    • sortOrder = null;
  • MUST implement openFile that for files returned in query, openFile with mode = "r" returns a valid ParcelFileDescriptor for reading. For example, uri can be content://com.github.shadowsocks.plugin.kcptun/kcptun.

Native mode without binary copying

  • Should this be implemented at all?

By taking advantage of android:protectionLevel="signature", plugins that have the same signature as the base package (shadowsocks-android in this case) should be able to be accessed directly by ss-local. If any native mode plugin wishes to support this mode, it must have an Activity which:

  • MUST have action: com.github.shadowsocks.plugin.$PLUGIN_ID.ACTION_START;
  • MUST be invisible;
  • MUST return the com.github.shadowsocks.plugin.EXTRA_EXECUTABLE with executable path (e.g. /data/data/com.github.shadowsocks.plugin.kcptun/lib/libkcptun.so).

All plugins in this mode HAVE to support native mode with binary copy.

P.S. If this is implemented, "official" plugins will perform a tiny little bit better than "unofficial" ones.

JVM mode

Note: This can actually be hybrid mode since I'm not caring what you do under the hood. But this should probably be a good idea if your plugin was written in a JVM language.

  • Should this be implemented at all?

Every JVM mode plugin MUST have a service intent filter that:

  • MUST have android:label and android:icon; (may be configured by activity or application)
  • MUST have action: com.github.shadowsocks.plugin.$PLUGIN_ID.ACTION_START;
  • MUST have action: com.github.shadowsocks.plugin.ACTION_JVM_PLUGIN; (used for discovering plugins)
  • After onStartCommand(intent, flags, startId), where caller package name, server_ip, server_port and com.github.shadowsocks.plugin.EXTRA_OPTIONS will be passed via intent extra. Within 5000ms, it MUST broadcast com.github.shadowsocks.plugin.ACTION_JVM_PLUGIN_READY with local_port (should be the result of new ServerSocket(0).getLocalPort()) as extra or com.github.shadowsocks.plugin.ACTION_JVM_PLUGIN_FAIL with optional error message as string extra to caller package.
  • After shutdown or startService timeout, stopService will be called on target service. The plugin MUST gracefully handle this and terminate.

Default configurations

If a plugin need to supply default configuration, it can put it under the ContentProvider/Service's meta-data tag, for example:

<meta-data android:name="com.github.shadowsocks.plugin.default_config"
           android:value="obfs=http"/>

If you need to provide different default configurations for different plugins, you have to use multiple ContentProvider/Service. It should be easy to implement with sub-classing.

3. Plugin security considerations

Plugins are certified using package signatures and shadowsocks-android will consider these signatures as trusted:

  • Signatures by trusted sources which includes:
    • @madeye, i.e. the signer of the main repo;
    • The main repo doesn't contain any other trusted signatures. Third-party forks should add their signatures to this trusted sources if they have plugins signed by them before publishing their source code.
  • Current package signature, which means:
    • If you get apk from shadowsocks-android releases or Google Play, this means only apk signed by @madeye will be recognized as trusted.
    • If you get apk from a third-party fork, all plugins from that developer will get recognized as trusted automatically even if its source code isn't available anywhere online.

A warning will be shown for untrusted plugins. No arbitrary restriction will be applied.

4. Plugin platform versioning

In order to be able to identify compatible and incompatible plugins, Semantic Versioning will be used.

Given a version number MAJOR.MINOR.PATCH, increment the:

  1. MAJOR version when you make incompatible API changes,
  2. MINOR version when you add functionality in a backwards-compatible manner, and
  3. PATCH version when you make backwards-compatible bug fixes.

Plugin app must include this in their application tag: (which should be automatically included if you are using our library)

<meta-data android:name="com.github.shadowsocks.plugin.version"
           android:value="1.0.0"/>

5. Possible problems

  • In some places hyphens are not accepted, for example package name. In that case, hyphens - should be changed into underscores _. For example, the package name for obfs-local would probably be com.github.shadowsocks.plugin.obfs_local (if it turns out that we really can't use hyphens in package name).
  • In the current implementation, kcptun needs to be restarted whenever a network change is detected. We may need to put this into this platform somehow, e.g. a restart receiver for shadowsocks and a start/stop receiver for plugins...

References

Opinions are welcome!

@madeye
Copy link
Contributor

madeye commented Jan 9, 2017

The configuration activity looks a great idea.

I'm not sure if JVM plugin could work on Android 4.4, as it requires per-app proxy feature of VPN service.

Let me dig into the whole proposal tomorrow. 😪

@Mygod
Copy link
Contributor Author

Mygod commented Jan 9, 2017

Hmm yeah native runnable plugins and JVM plugins might not work. We could put those off for now.

UPDATE: Added "Plugin platform versioning" section.

UPDATE: Changed "Possible problems" section.

Btw I'm planning to implement this at least after kcptun is migrated to SIP003, probably even later.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 12, 2017

@madeye What do you think? If you need more time, meanwhile I can help you migrate kcptun to SIP003.

@madeye
Copy link
Contributor

madeye commented Jan 12, 2017

Just not sure JVM mode could work.No more questions about other part.

What about implement a SIP003 example for simple-obfs first?

@Mygod
Copy link
Contributor Author

Mygod commented Jan 12, 2017

Okay.

@Mygod Mygod added this to the 4.0 milestone Jan 12, 2017
@Mygod
Copy link
Contributor Author

Mygod commented Jan 12, 2017

Some questions towards compatibility:

  • When will major version increment, i.e. when do we introduce non-backward-compatible changes?
  • Should new host support old plugins?
  • Should new plugin support old host?

My answer:

  • We should drop compatibility only if it counteracts an security attack. Otherwise we deprecates old APIs instead of removing them.
  • Yes unless major version is different.
  • No. Users should keep their host up-to-date since unmaintained hosts should be relatively rare. Plugins can assume their host is at oldest latest stable version.

@madeye
Copy link
Contributor

madeye commented Jan 13, 2017

Surprisingly, your answer == my answer. 😃

@Mygod
Copy link
Contributor Author

Mygod commented Jan 13, 2017

Updated this draft to use the new SS_PLUGIN_OPTIONS in shadowsocks/shadowsocks-org#28.

Mygod added a commit that referenced this issue Jan 13, 2017
Mygod added a commit that referenced this issue Jan 13, 2017
@Mygod
Copy link
Contributor Author

Mygod commented Jan 13, 2017

Interface for plugins in shadowsocks-android

Interface for plugins in shadowsocks-android. See develop branch for more info.

Further development is blocked by shadowsocks/shadowsocks-libev#1070, etc.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 17, 2017

@madeye Please check out 70e0b39 and simple-obfs-android. There are >2k of code changes and at this point all the basic features are implemented and everything is just barely building. Nothing is guaranteed working. I wish to take a break here and please do some bugfixes for me. Thanks.

@madeye
Copy link
Contributor

madeye commented Jan 17, 2017

Thanks for the big changes... I will help to test and fix bugs.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 17, 2017

Yeah I think it's safe to say that I have nuked almost every part of this repo at least once now.

@madeye
Copy link
Contributor

madeye commented Jan 19, 2017

I tried this change locally, but failed to load plugins. I saw fetchPlugins() is correctly called but no plugin can be found in that function.

Am I missing anything here? (It's tested on Android 6.0 emulator and Google Pixel Android 7.1.1)

@Mygod
Copy link
Contributor Author

Mygod commented Jan 19, 2017

It's possible. I haven't even tested this code once. 🤣

@madeye
Copy link
Contributor

madeye commented Jan 19, 2017

Oh My God... 😓

@madeye
Copy link
Contributor

madeye commented Jan 19, 2017

Maybe querying a provider through category doesn't work? Let me try the old ways...

@Mygod
Copy link
Contributor Author

Mygod commented Jan 19, 2017

@madeye Okay let me get back to work now. 😅 Feel free to help me fix JNI related stuff since I'm not familiar with the native code base yet...

@Mygod
Copy link
Contributor Author

Mygod commented Jan 19, 2017

@madeye The problem is querying through category doesn't work. It seems only querying with action is supported.

EDIT: Updated doc.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 19, 2017

@madeye Okay check it out now. Everything looks fine now except connection isn't still working. There are no traffic until E/shadowsocks: accept: Too many open files after which huge outbound traffic is seen in the monitor. Full log. And I'm going to take another break now. 😛

I have also changed a lot of the original interface. Updating doc later.

P.S. Run sbt plugin/publish to use it in simple-obfs-android. We'll deal with publishing it to public repository later.

@madeye
Copy link
Contributor

madeye commented Jan 20, 2017

@Mygod Great! I think now it's working. Please try d734e9f.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

@madeye It's working now. We can make a new beta release when kcptun is ported as well.

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

@Mygod OK. BTW, I've added SIP003 to kcptun. It works well now with shadowsocks-libev.

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

@Mygod What about publishing android plugins in your own Google Play account? i think it would encourage more people developing plugins for this project.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

I don't think that would make a big difference though. 😛

I'm working on native mode without binary copying now. After that I'll start creating kcptun-android. The important thing is native mode without binary copying requires plugin app has the same signature as host, so I'd rather not.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

@madeye Hmm it turns out implementing native mode without binary copying is very convenient as long as the binary has execute permission for other (fortunately default mode for binary under lib is 755) and plugin app only need to implement 1 more method which returns executable path (it probably already does anyway). We don't even need shared user id and/or same signature. I guess we are done here for simple-obfs. (I will push the source code shortly after I post this comment)

For kcptun, there's one thing that makes it complicated. In 3.x we workaround a bug by restarting kcptun process whenever a network change is detected. This would be problematic when putting it in the current plugin platform. Here are some possible solutions:

  1. Fix this bug in kcptun itself, after this we can just copy simple-obfs-android and create another kcptun-android;
  2. Implement JVM mode and use it to start/stop/restart kcptun and detect network changes;
  3. Provide a restart plugin callback in main app? (this would be hard since plugin lifecycle is managed by ss-local and ss-tunnel)

Oh speaking of ss-local and ss-tunnel if we are using pdnsd for DNS, there will be two instances of plugin running simultaneously which is kind of a waste of resources. Any thoughts toward that?

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

  1. I'm still worrying about passing file descriptors between different UIDs. If it really works, should it be a security issue of Android? 😈

  2. A small enough autoexpire could be a workaround. Besides, I'm thinking a "need-to-restart" parameter for plugin that shadowsocks needs to restart ss-local/ss-tunnel when the plugin is enabled and network changes.

  3. The duplicates of one plugin is definitely a problem. I'm thinking of using other dns clients in the future, e.g. dns-over-https. If so, there is no need to use ss-tunnel anymore.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

  1. I think you're answering 2... It should work. Remember ParcelFileDescriptor?
  2. Now you're probably answering 1 and 3... Is this bug so hard to fix?
  3. Maybe DIY? How does DNS over https work though?

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

  1. OK. It looks reasonable.
  2. It should be a fundamental problem of kcptun on mobile devices...
  3. Yes. It's quite easy to implement and I think we can DIY here. In additional, with edns-client-subnet, we will never worry about CDN related geo IP issue anymore.

Some references for dns-over-https:

  1. https://github.com/wrouesnel/dns-over-https-proxy/blob/master/dns-over-https-proxy.go
  2. https://developers.google.com/speed/public-dns/docs/dns-over-https

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

So maybe we should do the JVM mode for now? sigh

I see. But that requires a DNS-over-HTTPS-capable server. Fortunately there is. 🙌

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

  1. Currently, let's just restart ss-local/ss-tunnel...

  2. I found something looks better: https://github.com/aarond10/https_dns_proxy

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

But I don't want to add a meta-data to a plugin named restart_ss_local_and_ss_tunnel_whenever_network_changes nor add kcptun as a special case. Both of them look ugly.

@madeye
Copy link
Contributor

madeye commented Jan 22, 2017

Hmm... Maybe JVM mode is more reasonable here.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

It'd be best if we can fix this in kcptun. I'm going to copy simple-obfs-android for now...

@Mygod
Copy link
Contributor Author

Mygod commented Jan 22, 2017

kcptun-android has been built. Next, TODOs before we can finally close this issue:

  • Debug kcptun-android so that it works; (@madeye, I'm not sure if it's my network issues or else)
  • Move this draft to plugin/doc.md (or else?) and refine it;
  • Finalize plugin library 0.0.1 and publish it to sonatype (or else?); (@madeye as well, we need to create a ticket on Sonatype first to get this new lib approved)
  • Create a plugin/README.md including fantastic "official" plugins and where to find them, developer's quick start and a link to doc.md for technical details;
  • Release 4.0.0 pre-release and mark kcptun network switching bug as a known bug and fix it some day.

Btw do you think we should create a configuration activity for kcptun as well? Currently this plugin looks exactly the same as 3.x, except that it's a plugin now.

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

It turns out to be a config related issue, fixed via shadowsocks/kcptun@b2d702f

Let's submit to Sonatype now. I think it's ready.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

@madeye Okay. Please follow this guide and create a ticket with your account (or maybe create a public one?). I think you should be able to do the plugin publishing since you're publishing the main app.

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

@Mygod Hmm.. How to create a public account?

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

@Mygod What about publishing the library in your account? I found it's quite complicated....

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

Let's just use your account to publish for now so that in the future we can push out updates to plugin lib and main app simultaneously... 😌

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

It seems that we need to merge the develop branch to master first. A pull request or I directly rebase to master?

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

Why's that?

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

It looks that I need to provide the path to git repo for Sonatype. Not sure if I can specify the branch name there.

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

Add a comment perhaps?

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

@madeye Okay I pushed everything to beta branch. We can make a beta release after publishing our 0.0.1 library...

@madeye
Copy link
Contributor

madeye commented Jan 23, 2017

I'm not sure if I have publish the package correctly. But it seems to work now.

https://oss.sonatype.org/content/repositories/snapshots/com/github/shadowsocks/plugin_2.11/0.0.1-SNAPSHOT/

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

LGTM. Please checkout beta branch and publish 0.0.1. 🦃

@Mygod
Copy link
Contributor Author

Mygod commented Jan 23, 2017

Nice. Closing this.

@Mygod Mygod closed this as completed Jan 23, 2017
bannedbook pushed a commit to bannedbook/SpeedUp.VPN that referenced this issue Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants