Skip to content

Commit

Permalink
Add support for threshold manifest updates
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Weiße <[email protected]>
  • Loading branch information
daniel-weisse committed Jan 7, 2025
1 parent 9b4066f commit ad56392
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 3 deletions.
16 changes: 16 additions & 0 deletions coordinator/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ type Config struct {
SealMode string
// FeatureGates is a list of additional features to enable on the Coordinator.
FeatureGates []string
// UpdateThreshold is the amount of acknowledgements required to perform a multi party manifest update.
// If set to 0, all users with the update permission are required to acknowledge an update before it is applied.
UpdateThreshold uint
}

// Marble describes a service in the mesh that should be handled and verified by the Coordinator.
Expand Down Expand Up @@ -496,6 +499,19 @@ func (m Manifest) Check(zaplogger *zap.Logger) error {
}
}

var manifestUpdaters int
for _, mrUser := range m.Users {
for _, roleName := range mrUser.Roles {
if m.Roles[roleName].ResourceType == "Manifest" && strings.ToLower(m.Roles[roleName].Actions[0]) == user.PermissionUpdateManifest {
manifestUpdaters++
break // Avoid counting the same user multiple times if they are assigned more than one role with update permission
}
}
}
if manifestUpdaters < int(m.Config.UpdateThreshold) {
return fmt.Errorf("not enough users with manifest update permissions (%d) to meet the threshold of %d", manifestUpdaters, m.Config.UpdateThreshold)
}

switch m.Config.SealMode {
case "", "ProductKey", "UniqueKey", "Disabled":
default:
Expand Down
31 changes: 30 additions & 1 deletion coordinator/manifest/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"encoding/json"
"encoding/pem"
"fmt"
"strings"
"testing"

"github.com/edgelesssys/marblerun/coordinator/quote"
Expand Down Expand Up @@ -268,11 +269,39 @@ func TestManifestCheck(t *testing.T) {
err = manifest.Check(log)
assert.NoError(err)

manifest.Roles["updateManifest"] = Role{
ResourceType: "Manifest",
ResourceNames: []string{},
Actions: []string{user.PermissionUpdateManifest},
}
adminUser := manifest.Users["admin"]
adminUser.Roles = append(adminUser.Roles, "updateManifest")
manifest.Users["admin"] = adminUser

manifest.Config.UpdateThreshold = 0
err = manifest.Check(log)
assert.NoError(err, "update threshold 0 should be accepted")

manifest.Config.UpdateThreshold = 1
err = manifest.Check(log)
assert.NoError(err, "update threshold equal to allowed users should be accepted")

adminUser2 := adminUser
adminUser2.Certificate = strings.ReplaceAll(adminUser.Certificate, "q", "b")
manifest.Users["admin2"] = adminUser2
err = manifest.Check(log)
assert.NoError(err, "update threshold lower than allowed users should be accepted")

manifest.Config.UpdateThreshold = 3
err = manifest.Check(log)
assert.Error(err, "update threshold higher than allowed users should be rejected")
manifest.Config.UpdateThreshold = 0

manifest.Users["anotherUser"] = User{
Certificate: manifest.Users["admin"].Certificate,
}
err = manifest.Check(log)
assert.Error(err)
assert.Error(err, "users without unique certificates should be rejected")
}

func TestCertificate(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions docs/docs/workflows/define-manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ The optional entry `Config` holds configuration settings for the Coordinator.
"Config":
{
"SealMode": "ProductKey",
"UpdateThreshold": 5,
"FeatureGates": []
}
//...
Expand All @@ -534,6 +535,8 @@ The optional entry `Config` holds configuration settings for the Coordinator.

See the section on [seal key types](../architecture/security.md#seal-key) for more information.

`UpdateThreshold` specifies the number of acknowledgments required for a [multi-party manifest update](./update-manifest.md#full-update). If not set, all users with roles that provide the `UpdateManifest` action need to acknowledge the update.

Check warning on line 538 in docs/docs/workflows/define-manifest.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Microsoft.ComplexWords] Consider using 'give' or 'offer' instead of 'provide'. Raw Output: {"message": "[Microsoft.ComplexWords] Consider using 'give' or 'offer' instead of 'provide'.", "location": {"path": "docs/docs/workflows/define-manifest.md", "range": {"start": {"line": 538, "column": 177}}}, "severity": "INFO"}

`FeatureGates` allows you to opt-in to additional features that may be useful for certain use cases. The following features are available:

* `SignQuoteEndpoint`: enables the [sign-quote endpoint](../reference/coordinator.md#verify-and-sign-an-sgx-quote)
Expand Down
5 changes: 3 additions & 2 deletions docs/docs/workflows/update-manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ Don't define other values except the `SecurityVersion` value for a package, as M
Some deployment scenarios require more flexibility regarding changes to the manifest. To this end, MarbleRun also allows uploading a full manifest. User-defined secrets and secrets of type `symmetric-key` are retained if their definition doesn't change.

To deploy a new manifest, your user must have a [role assigned that contains the `UpdateManifest` action](define-manifest.md#roles).
If multiple users have such a role assigned, each of them must [acknowledge](#acknowledging-a-multi-party-update) the new manifest.
If multiple users have such a role assigned, the manifest field `.Config.UpdateThreshold` specifies how many of of them must [acknowledge](#acknowledging-a-multi-party-update) the new manifest.

Check warning on line 57 in docs/docs/workflows/update-manifest.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Microsoft.ComplexWords] Consider using 'many' instead of 'multiple'. Raw Output: {"message": "[Microsoft.ComplexWords] Consider using 'many' instead of 'multiple'.", "location": {"path": "docs/docs/workflows/update-manifest.md", "range": {"start": {"line": 57, "column": 4}}}, "severity": "INFO"}

Check failure on line 57 in docs/docs/workflows/update-manifest.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Vale.Repetition] 'of' is repeated! Raw Output: {"message": "[Vale.Repetition] 'of' is repeated!", "location": {"path": "docs/docs/workflows/update-manifest.md", "range": {"start": {"line": 57, "column": 110}}}, "severity": "ERROR"}
If not set, all of them must acknowledge the new manifest.

Check warning on line 58 in docs/docs/workflows/update-manifest.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Microsoft.Wordiness] Consider using 'all' instead of 'all of'. Raw Output: {"message": "[Microsoft.Wordiness] Consider using 'all' instead of 'all of'.", "location": {"path": "docs/docs/workflows/update-manifest.md", "range": {"start": {"line": 58, "column": 13}}}, "severity": "WARNING"}

## Deploying an update

Expand All @@ -68,7 +69,7 @@ On success, no message will be returned and your MarbleRun logs should highlight

## Acknowledging a multi-party update

All users that have the `UpdateManifest` permission are required to acknowledge a full manifest update.
Users that have the `UpdateManifest` permission are required to acknowledge a full manifest update.

Check warning on line 72 in docs/docs/workflows/update-manifest.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Microsoft.Passive] 'are required' looks like passive voice. Raw Output: {"message": "[Microsoft.Passive] 'are required' looks like passive voice.", "location": {"path": "docs/docs/workflows/update-manifest.md", "range": {"start": {"line": 72, "column": 49}}}, "severity": "INFO"}
To this end, they need to upload the same manifest.
This proves that they have knowledge of and agree on this manifest.

Expand Down

0 comments on commit ad56392

Please sign in to comment.