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

Add Plugins #81

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Add Plugins #81

wants to merge 13 commits into from

Conversation

JU4N98
Copy link
Contributor

@JU4N98 JU4N98 commented Jul 4, 2023

Changes proposal:

Adds support for plugins according with issue #65, on each X.509 SVID, JWT SVID or JWT Bundle update, the helper will notify this change to all the loaded plugins. The block plugins is added to the config file where the configuration of every plugin is specified. For example:

agentAddress = "/var/run/api.sock"
cmd = "/run/client/assert.sh"
cmdArgs = ""
certDir = "/run/client/certs/"
renewSignal = "SIGUSR1"
svidFileName = "svid.crt"
svidKeyFileName = "svid.key"
svidBundleFileName = "root.crt"
jwt_audience = "example.org"
jwt_svid_file_name = "jwt.json"
jwt_bundle_file_name = "jwk.json"

plugins {
    "simple-plugin" {
        path="/opt/helper/simple-example"
        checksum="7ae182614c5b2f96b0c6655a6bf3e1e64fb0dbb9142fa50c8cf0002c5c5bb9c5"
        from="Bob"
        to="Alice"
        message="hi alice!"
    }
}

In this example a plugin called simple-plugin is going to be loaded into the spiffe-helper, there are two mandatory fields:

  • path: the path of the plugin binary.
  • checksum: the sha256 hash of the binary, needed to validate it.

But it is also possible to specify other string configurations for each plugin, in this case the simple-plugin has three: from, to and message. All the configurations will be shared on the plugin load.

syntax = "proto3";

package helperPlugin;
option go_package = "../../pkg/helperPlugin";
Copy link
Collaborator

Choose a reason for hiding this comment

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

this package must be in the same folder we are

}

service SpiffeHelper {
rpc PostConfigs(ConfigsRequest) returns (Empty) {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

what are those configs? only paths?
I feels like this plugin ends is to notify "something" about there are updated on bundles,
so we must know what is updated, and what to do based on each plugin config,
So the Service may be something like Notifier, and your PostConfig, may be Notify
what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, most of the configs are paths to certs. The current approach is to execute the plugins on each cert update (and let them do what they want with those certs), but yes, it is possible to keep them running and send a notification on updates.

message Empty {}

message ConfigsRequest {
map<string,string> configs = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what this configs are?
What data is the Plugin getting in order to understand where to find certs, or what kind of cert was received?
and how is each plugin configured? is it possible to provide data to this plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The configs contains most of the spiffe-helper .conf fields and the plugin specific configurations

@JU4N98
Copy link
Contributor Author

JU4N98 commented Jul 10, 2023

Hi @faisal-memon, it would be great if you can take a look to this proof of concept. It is related to 65.

}

service Notifier {
rpc LoadConfigs(ConfigsRequest) returns (Empty) {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

mmm generally we create messages for request or response, no matter if they are empty,
but when we want to add fields in a future, it keep working without back compatibility issues.
So I suggest to add messages for both requests and response

grpc "google.golang.org/grpc"
)

type Notifier interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are not you using notifier.NotifierServer? (the one autogenerated from proto)

@faisal-memon faisal-memon added this to the 0.9.0 milestone Dec 2, 2023
@kfox1111
Copy link
Contributor

Whats the status of this? I think it is useful to the helm-charts-hardened project.

@JU4N98
Copy link
Contributor Author

JU4N98 commented Dec 20, 2023

Whats the status of this? I think it is useful to the helm-charts-hardened project.

Hi @kfox1111 I'm going to keep working on this

@JU4N98 JU4N98 marked this pull request as ready for review December 27, 2023 19:02
@JU4N98 JU4N98 changed the title PoC: Add Plugins Add Plugins Dec 27, 2023
Signed-off-by: JU4N98 <[email protected]>
@JU4N98 JU4N98 requested a review from MarcosDY December 27, 2023 19:14
@kfox1111
Copy link
Contributor

A thought....

This plugin support requires spire-helper to execute the plugin. In a traditional environment, that works very well.

In a containerized environment such as Kubernetes though, that would require spire-helper along with the plugins to be in the same container, which is more difficult as generally those come from different sources. In that environment, its much easier for them to be different containers, have Kubernetes exec the containers, and share between them with unix sockets.

Could this pr be updated to support that mode of operation too?

@JU4N98
Copy link
Contributor Author

JU4N98 commented Dec 28, 2023

A thought....

This plugin support requires spire-helper to execute the plugin. In a traditional environment, that works very well.

In a containerized environment such as Kubernetes though, that would require spire-helper along with the plugins to be in the same container, which is more difficult as generally those come from different sources. In that environment, its much easier for them to be different containers, have Kubernetes exec the containers, and share between them with unix sockets.

Could this pr be updated to support that mode of operation too?

I think something like that is out of scope for this PR, the main idea was to add support for plugins that runs alongside the helper in the same container. But is the initial approach, maybe that could be added in other PR as other kind of plugins.

Signed-off-by: JU4N98 <[email protected]>
@kfox1111
Copy link
Contributor

Probably ok to do in a followon, but maybe make sure the api leaves room so that it can be done via socket instead of exec without having to move around config items?

@kfox1111
Copy link
Contributor

Looks like that could be done by adding a 'type' field or something to the plugin to specify that path is an executable (default) vs named socket, so, I think we're good already?

@JU4N98
Copy link
Contributor Author

JU4N98 commented Dec 29, 2023

Looks like that could be done by adding a 'type' field or something to the plugin to specify that path is an executable (default) vs named socket, so, I think we're good already?

Yes, a field like type should be enough to treat the differently. I think isn't necessary to add anything since I'm parsing each plugin into a map of string.

Signed-off-by: JU4N98 <[email protected]>
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.

4 participants