Skip to content

Fix TPM fd leaks and OpenFGA patching issue #1469

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

Merged
merged 4 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 0 additions & 19 deletions cmd/incusd/patches.go
Original file line number Diff line number Diff line change
Expand Up @@ -686,25 +686,6 @@ func patchMoveBackupsInstances(name string, d *Daemon) error {
}

func patchGenericAuthorization(name string, d *Daemon) error {
// Only run authorization patches on the leader.
isLeader := false

leaderAddress, err := d.gateway.LeaderAddress()
if err != nil {
if !errors.Is(err, cluster.ErrNodeIsNotClustered) {
return err
}

isLeader = true
} else if leaderAddress == d.localConfig.ClusterAddress() {
isLeader = true
}

// If clustered and not running on a leader, skip the resource update.
if !isLeader {
return nil
}

return d.authorizer.ApplyPatch(d.shutdownCtx, name)
}

Expand Down
79 changes: 44 additions & 35 deletions internal/server/auth/driver_openfga.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,33 @@ func (f *fga) StopService(ctx context.Context) error {

// ApplyPatch is called when an applicable server patch is run, this triggers a model re-upload.
func (f *fga) ApplyPatch(ctx context.Context, name string) error {
// Upload a new model.
if name == "auth_openfga_viewer" {
// Add the public access permission if not set.
resp, err := f.client.Check(ctx).Body(client.ClientCheckRequest{
User: "user:*",
Relation: "authenticated",
Object: ObjectServer().String(),
}).Execute()
if err != nil {
return err
}

if !resp.GetAllowed() {
err = f.sendTuples(ctx, []client.ClientTupleKey{
{User: "user:*", Relation: "authenticated", Object: ObjectServer().String()},
}, nil)
if err != nil {
return err
}

// Attempt to clear the former version of this permission.
_ = f.sendTuples(ctx, nil, []client.ClientTupleKeyWithoutCondition{
{User: "user:*", Relation: "viewer", Object: ObjectServer().String()},
})
}
}

// Always refresh the model.
logger.Info("Refreshing the OpenFGA model")
return f.refreshModel(ctx)
}
Expand Down Expand Up @@ -176,10 +202,20 @@ func (f *fga) connect(ctx context.Context, certificateCache *certificate.Cache,
// Check if we need to upload an initial model.
if readModelResponse.AuthorizationModel == nil {
logger.Info("Upload initial OpenFGA model")

// Upload the model itself.
err := f.refreshModel(ctx)
if err != nil {
return fmt.Errorf("Failed to load initial model: %w", err)
}

// Allow basic authenticated access.
err = f.sendTuples(ctx, []client.ClientTupleKey{
{User: "user:*", Relation: "authenticated", Object: ObjectServer().String()},
}, nil)
if err != nil {
return err
}
}

if opts.resourcesFunc != nil {
Expand Down Expand Up @@ -830,6 +866,7 @@ func (f *fga) DeleteStorageBucket(ctx context.Context, projectName string, stora
return f.updateTuples(ctx, nil, deletions)
}

// updateTuples sends an object update to OpenFGA if it's currently online.
func (f *fga) updateTuples(ctx context.Context, writes []client.ClientTupleKey, deletions []client.ClientTupleKeyWithoutCondition) error {
// If offline, skip updating as a full sync will happen after connection.
if !f.online {
Expand All @@ -840,6 +877,11 @@ func (f *fga) updateTuples(ctx context.Context, writes []client.ClientTupleKey,
return nil
}

return f.sendTuples(ctx, writes, deletions)
}

// sendTuples directly sends the write/deletion tuples to OpenFGA.
func (f *fga) sendTuples(ctx context.Context, writes []client.ClientTupleKey, deletions []client.ClientTupleKeyWithoutCondition) error {
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()

Expand Down Expand Up @@ -916,43 +958,10 @@ func (f *fga) projectObjects(ctx context.Context, projectName string) ([]string,
return allObjects, nil
}

func (f *fga) applyPatches(ctx context.Context) ([]client.ClientTupleKey, []client.ClientTupleKeyWithoutCondition, error) {
func (f *fga) syncResources(ctx context.Context, resources Resources) error {
var writes []client.ClientTupleKey
var deletions []client.ClientTupleKeyWithoutCondition

// Add the public access permission if not set.
resp, err := f.client.Check(ctx).Body(client.ClientCheckRequest{
User: "user:*",
Relation: "authenticated",
Object: ObjectServer().String(),
}).Execute()
if err != nil {
return nil, nil, err
}

if !resp.GetAllowed() {
writes = append(writes, client.ClientTupleKey{
User: "user:*",
Relation: "authenticated",
Object: ObjectServer().String(),
})

// Attempt to clear the former version of this permission.
_ = f.updateTuples(ctx, nil, []client.ClientTupleKeyWithoutCondition{
{User: "user:*", Relation: "viewer", Object: ObjectServer().String()},
})
}

return writes, deletions, nil
}

func (f *fga) syncResources(ctx context.Context, resources Resources) error {
// Apply model patches.
writes, deletions, err := f.applyPatches(ctx)
if err != nil {
return err
}

// Helper function for diffing local objects with those in OpenFGA. These are appended to the writes and deletions
// slices as appropriate. If the given relation is relationProject, we need to construct a project object for the
// "user" field. The project is calculated from the object we are inspecting.
Expand Down
2 changes: 1 addition & 1 deletion internal/server/instance/drivers/driver_qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -4870,7 +4870,7 @@ func (d *qemu) addTPMDeviceConfig(cfg *[]cfgSection, tpmConfig []deviceConfig.Ru
}
}

fd, err := unix.Open(socketPath, unix.O_PATH, 0)
fd, err := unix.Open(socketPath, unix.O_PATH|unix.O_CLOEXEC, 0)
if err != nil {
return err
}
Expand Down
Loading