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.

Sign up for GitHub

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

SVIDStore disk plugin #2647

Closed
wants to merge 12 commits into from
Closed

SVIDStore disk plugin #2647

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eb457e7
SVIDStore `disk` plugin
amartinezfayo Dec 3, 2021
adabc13
SVIDStore `disk` plugin
amartinezfayo Dec 3, 2021
4a450b4
SVIDStore `disk` plugin
amartinezfayo Dec 3, 2021
a3e4091
SVIDStore `disk` plugin
amartinezfayo Dec 3, 2021
1ed9fce
SVIDStore `disk` plugin
amartinezfayo Dec 3, 2021
f85e6cf
Use the `user` namespace in the extended attribute. See the "Extended…
amartinezfayo Dec 3, 2021
81c022d
Address PR comments
amartinezfayo Dec 7, 2021
b159137
- Provide atomicity to the write operation
amartinezfayo Jan 5, 2022
bbfab82
Merge remote-tracking branch 'origin/main' into svidstore-disk
amartinezfayo Jan 5, 2022
9c2c400
Update tests
amartinezfayo Jan 5, 2022
331f5dd
Update documentation
amartinezfayo Jan 5, 2022
22f659b
Address PR comments
amartinezfayo Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions conf/agent/agent_full.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,15 @@ plugins {
}
}

# SVIDStore "disk": An SVID store that stores the SVIDs on disk
SVIDStore "disk" {
plugin_data {
# directory: Base directory that is used to store the SVIDs.
# All stored files are under this path.
# directory = "/path/to/svids"
}
}

# WorkloadAttestor "docker": A workload attestor which allows selectors
# based on docker constructs such label and image_id.
WorkloadAttestor "docker" {
Expand Down
41 changes: 41 additions & 0 deletions doc/plugin_agent_svidstore_disk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Agent plugin: SVIDStore "disk"

The `disk` plugin stores in disk the resulting X509-SVIDs of the entries that the agent is entitled to.

### Format

The plugin stores the SVID in three different PEM encoded files: one for the certificate chain, one for the certificate key and one for the trust domain bundle. The file paths are specified through [selectors](#selectors).

_Note: federated bundles are not stored by this plugin._

### Configuration

| Configuration | Description | DEFAULT |
| -------------------- | ----------- | -------------- |
| directory | Base directory that is used to store the SVIDs. All stored files are under this path. | |

A sample configuration:

```
SVIDStore "disk" {
plugin_data {
directory = "/path/to/svids"
}
}
```

### Selectors

Selectors are used on `storable` entries to describre metadata that is needed by the `disk` plugin in order to store the SVIDs on disk. In case that a required selector is not provided, the plugin will return an error at execution time.

| Selector | Example | Required | Description |
| ----------------------------- | ------------------------------------------ | -------- | -------------------------------------------- |
| `disk:certchainfile` | `disk:certchainfile:tls.crt` | x | The file path relative to the base directory where the SVID certificate chain will be stored. Must be in the same directory as `keyfile` and `bundlefile`. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about setting path in each selector, we are forced to set all selectors with paths and make sure all paths are the same.
What do you think about creating a selector to set a folder in a single place?
that will allow us to have "default" file names, so entries will be easier, and we can have other selectors to set file names.
example using default names:

{  
    "spiffeID": "spiffe://example.org/w1",
    "parentID": "spiffe://example.org/agent",
    "selectors", [
         "disk:directory:/tmp"
    ]
}

example changing a single file name

{  
    "spiffeID": "spiffe://example.org/w1",
    "parentID": "spiffe://example.org/agent",
    "selectors", [
         "disk:certchainfile:tls.crt"
    ]
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about this and I didn't like the idea of adding a new selector.
Now that I think it more, this also has the benefit of being able to have default file names and end up with a less number of required selectors. I'll make the change to implement it in the suggested way.

| `disk:keyfile` | `disk:keyfile:key.crt` | x | The file path relative to the base directory where the SVID certificate key will be stored. Must be in the same directory as `certchainfile` and `bundlefile`. |
| `disk:bundlefile` | `disk:bundlefile:ca.crt` | x | The file path relative to the base directory where the CA certificates belonging to the Trust Domain of the SVID will be stored. Must be in the same directory as `certchainfile` and `keyfile`. |
| `disk:group` | `disk:group:my-workload` | x (if `gid` is not specified) | The group name that is set to the files written to disk. If set, `gid` cannot be specified. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

disk:group and disk:gid is confusing, may we use groupname or gname instead? and have a consistent name for gid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I like groupname and groupid.

| `disk:gid` | `disk:group:my-workload` | x (if `group` is not specified) | The group ID that is set to the files written to disk. If set, `group` cannot be specified. |

### Required permissions

In order to be able to set proper ownership of the written files, this plugin requires that the user that runs SPIRE Agent is a member of the group specified through the `disk:group` or `disk:gid` selectors.
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ require (
go.opencensus.io v0.23.0 // indirect
go.uber.org/atomic v1.9.0
go.uber.org/goleak v1.1.11 // indirect
go.uber.org/multierr v1.6.0 // indirect
go.uber.org/multierr v1.6.0
go.uber.org/zap v1.19.0 // indirect
golang.org/x/crypto v0.0.0-20210915214749-c084706c2272
golang.org/x/net v0.0.0-20210917221730-978cfadd31cf
Expand Down
2 changes: 2 additions & 0 deletions pkg/agent/catalog/svidstore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package catalog
import (
"github.com/spiffe/spire/pkg/agent/plugin/svidstore"
"github.com/spiffe/spire/pkg/agent/plugin/svidstore/awssecretsmanager"
"github.com/spiffe/spire/pkg/agent/plugin/svidstore/disk"
"github.com/spiffe/spire/pkg/agent/plugin/svidstore/gcpsecretmanager"
"github.com/spiffe/spire/pkg/common/catalog"
)
Expand All @@ -27,6 +28,7 @@ func (repo *svidStoreRepository) BuiltIns() []catalog.BuiltIn {
return []catalog.BuiltIn{
awssecretsmanager.BuiltIn(),
gcpsecretmanager.BuiltIn(),
disk.BuiltIn(),
}
}

Expand Down
Loading