Plugin deactivation

[Thomas DELHOMENIE]

Hello,

I wanted to add a way to disable SSH and Telnet access by configuration in crash. Currently, the only way to do that is to remove the jars, which is not very convenient and force to modify the packaging.

My first thought was to update the SSH and Telnet plugins to add a new parameter, but maybe it could be better if we do that on a higher level, by allowing to enable/disable plugins by configuration (with something like crash.plugins.<plugin_type>.enable=false in crash.properties).

WDYT ?

Thomas

[Julien Viet]

Hi Thomas,

that's how I would do it.

The properties would be used by the PluginDiscovery implementation ServiceLoaderDiscovery to filter the list. The Spring implementation would inherit this behavior and it seems reasonable.

However it means also we should add a naming to the plugin part of the CRaSHPlugin interface that would be used for that purpose.

could you start a wiki document about this so it becomes a visible proposal ?

I don't want to get into something procedural or complex however I would like to achieve something that could work later with different configuration system (i.e using JSON configuration). For that matter it's best to keep a synthetic specification for that.

So just make something simple that describes this and the entire list would be able to review and give their opinion.

At the moment chose any wiki you like that we can share, later we will see how to make this more formal if it's needed.

--
Vous recevez ce message, car vous êtes abonné au groupe Google Groupes CRaSH User Group.
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse crash-users...@googlegroups.com.
Pour plus d'options, visitez le site https://groups.google.com/groups/opt_out .
 
 

[Thomas DELHOMENIE]

Here is a first draft : https://github.com/crashub/crash/wiki/Plugins-deactivation

2013/4/3 Julien Viet <jul...@julienviet.com>

[Julien Viet]

good start.

We need for now to remove  the dynamic part because for now the plugin system is static and I don't plan to work on the plugin system until there is a more important requirement for it.

[Thomas DELHOMENIE]

ok

In fact I think the PluginManager should be responsible for filtering the activated plugins. ServiceLoaderDiscovery is only responsible for getting all the deployed plugins. WDYT ?

2013/4/4 Julien Viet <jul...@julienviet.com>

[Julien Viet]

PluginDiscovery indeed provides a way to retrieve plugins from a source (like classloader via ServiceLoader, Spring, etc…).

Having this in PluginManager would make the control over the deployed plugins more central and works with any PluginDiscovery implementation.

However the what worries me about that is the fact that it for some scenario it is better to avoid have control over the plugin presents or have a potential conflicts with such global configuration. I am thinking about unit testing that use the SimplePluginDiscovery that provides a specific list of plugins to use during testing.

That the reason why I would prefer it (at least for now) to be in the ServiceLoaderDiscovery.

At the moment there are three implementations:

SimplePluginDiscovery : a list

ServiceLoaderDiscovery : uses service loader -> it finds anything on the classpath

SpringPluginDiscovery : extends ServiceLoaderDiscovery and also retrieves plugin from the Spring BeanFactory

So having this in ServiceLoaderDiscovery will work better until the plugin / integration system gets serious R&D (here I'm referring to something that could be dynamic and retrieve plugins from a maven repository directly).

That being said there are other things to nail down for the current effort but I'll send that in another reply in this thread.

[Julien Viet]

So there is this requirements I was mentioning before about the fact that a plugin may have several implementations.

There are two kinds of plugin

1/ plugins which are their own implementation: class FooPlugin extends CRaSHPlugin<FooPlugin> { … }

2/ plugins implementing a common class interface such as the AuthenticationPlugin: class MyAuthenticationPlugin extends CRaSHPlugin<Authenticator> { … }

Now the choice of the plugin to use is not done via the plugin system but instead via the consumer of the plugin, for instance if you look at SSHPlugin you will see that it uses the configuration property crash.auth to browse all plugins providing the Authentication interface until it find one with the name of the property, for example in configuration.properties we do have:

crash.auth=simple

and if you change by

crash.auth=jaas

then it changes the plugin implementation used by SSHPlugin.

The effort about enabling / disabling the plugin system is a good opportunity to improve that and have the SSHPlugin not worry about determining which plugin to use. Instead it would only ask for a single plugin implementing the Authentication interface and simply get it.

So the convention to enable/disable a plugin based on property should handle more than just enable / disable, they need to provide a way to specify the plugin to use. Each plugin should have two values for identifying it:

1/ the plugin name like "auth" : the plugin that takes care of authentication

2/ the plugin qualifier like "simple" or "jaas" (as in IOC qualifier) 

(note we're staying away from the notion of Java type (i.e the class name of the plugin) because sysadmin simply don't know Java class. So it is more convenient to work on identifiers.)

Each plugin should provide the name and the qualifier in the CRaSHPlugin interface via abstract methods getName() and getQualifier() that a plugin must implement. The "default" qualifier is a convention to say that the plugin is the default implementation and used mostly when there is a single plugin implementation possible or when a plugin should be used by default. In case of Authentication plugin we would not use "default" because it should the plugin to use should be explicitly specified (crash.auth=simple).

Then we need to find a way to use configuration properties to:

1/ enable / disable a plugin

2/ specify the plugin qualifier to use (not specified means "default" qualifier).

So I'm thinking about something along the lines you proposed with crash.plugins prefix :

1/ specify plugin implementation to use

- crash.plugins.$NAME=$QUALIFIER (not specified then implementation to use is "default")

2/ control activation of plugin

- crash.plugins.$NAME.$QUALIFIER.active = false | true

- crash.plugins.$NAME.active = false | true (same but for "default" qualifier)

This way we could have following configuration:

a/ Disable telnet plugin

crash.plugins.telnet.active=false

b/ Use simple authentication

crash.plugins.auth=simple

c/ Use jaas authenticaton and disable simple authentication plugin

crash.plugins.auth=jaas

crash.plugins.auth.simple.active=false

WDYT ?

(I encourage you to review and challenge what I wrote because I often miss some scenario that could contradict the general case).

[Thomas DELHOMENIE]

I seems you already thought a little bit about this topic before ;) That sounds good.

My remarks :

- I am not sure about the "default" qualifier. If I create an implementation of a new plugin (so this is the first implementation), how can I decide that this will be the default implementation ? If another implementation of this plugin is then implemented, maybe it could be a better default one. Maybe we should use the following rules :

--- if there is only one active implementation, use it

--- if there are several active implementations, the implementation to use must be explicitly configured (like for the authentication plugin)

So the "default" qualifier is not a keyword anymore. The qualifier value does not have a particular meaning, it is just a qualifier.

- about "crash.plugins.$NAME.$QUALIFIER.active = false | true" it must be true if not configured, so plugins are activated by default

- about "crash.plugins.$NAME.active = false | true (same but for "default" qualifier)" it does not seem intuitive to me, I thought it would disable all the plugins of the type $NAME. If there are several implementations of a plugin, and I want to disable this type of plugin, it will be easier to use "crash.plugins.$NAME.active = false" instead of disabling all the qualifiers of this plugin.

2013/4/7 Julien Viet <jul...@julienviet.com>

[Julien Viet]

On Apr 7, 2013, at 10:40 PM, Thomas DELHOMENIE <thomas.d...@gmail.com> wrote:

I seems you already thought a little bit about this topic before ;) That sounds good.

My remarks :

- I am not sure about the "default" qualifier. If I create an implementation of a new plugin (so this is the first implementation), how can I decide that this will be the default implementation ?

Default qualifier should be used by plugins shipped by the CRaSH project itself, i.e it is the one that is used by default. Either because it does not make sense to have more than one (like the SSHPlugin itself) or because there can be several implementations but the default implementation is shipped with CRaSH and should be used without explicit configuration (like the ShellFactory that is overridden in the try.crashub.org runtime).

If another implementation exist and needs to be used then it must be explicitly configured with crash.plugins.foo=bar then this implementation becomes the new one used by the system.

If another implementation of this plugin is then implemented, maybe it could be a better default one. Maybe we should use the following rules :

--- if there is only one active implementation, use it

That lack of control (due to implicit configuration) is definitely an issue for the security reasons. let's suppose that a packaging mistake makes the simple plugin bundled instead of the jaas plugin. Then the simple plugin with admin/admin authentication is used instead of the jaas one. In the case of authentication we want to have no default existing implementation and the plugin to avoid to pick one by mistake. The authentication plugin to use should be always explicitly specified in the configuration.

[Thomas DELHOMENIE]

2013/4/7 Julien Viet <jul...@julienviet.com>

On Apr 7, 2013, at 10:40 PM, Thomas DELHOMENIE <thomas.d...@gmail.com> wrote:

I seems you already thought a little bit about this topic before ;) That sounds good.

My remarks :

- I am not sure about the "default" qualifier. If I create an implementation of a new plugin (so this is the first implementation), how can I decide that this will be the default implementation ?

Default qualifier should be used by plugins shipped by the CRaSH project itself, i.e it is the one that is used by default. Either because it does not make sense to have more than one (like the SSHPlugin itself) or because there can be several implementations but the default implementation is shipped with CRaSH and should be used without explicit configuration (like the ShellFactory that is overridden in the try.crashub.org runtime).

Do you mean that the default qualifier should be used only by plugins shipped by the CRaSH project itself ? What about external plugins (there is no external plugins right now, but it can happen, and I am sure it will ;)) ?

 

If another implementation exist and needs to be used then it must be explicitly configured with crash.plugins.foo=bar then this implementation becomes the new one used by the system. 

If another implementation of this plugin is then implemented, maybe it could be a better default one. Maybe we should use the following rules :

--- if there is only one active implementation, use it

That lack of control (due to implicit configuration) is definitely an issue for the security reasons. let's suppose that a packaging mistake makes the simple plugin bundled instead of the jaas plugin. Then the simple plugin with admin/admin authentication is used instead of the jaas one. In the case of authentication we want to have no default existing implementation and the plugin to avoid to pick one by mistake. The authentication plugin to use should be always explicitly specified in the configuration.

I understand for the authentication plugin but it would be convenient for the others. I package the implementation I want and I'm done.

[Julien Viet]

"default" is not reserved to CRaSH itself so anyone could use it.

Beside that you suggested : " if there is only one active implementation, use it" . This solution would not work because two default plugins could not be disabled at the same time with the property.

However it can be solved by removing the ambiguity by giving capability to declare which Java class is the default like crash.plugins.foo.default = org.foo.Bar .