Config: Remove concept of hidden config entries and consistently use strings

[roosterfish]

This PR includes cherry picks from lxc/incus#62, lxc/incus#63 and lxc/incus#64.

Essentially the concept of hidden config entries is removed and the overall server configuration is normalized to use a map[string]string instead of map[string]any which was required to support the hiding of passwords using the special true indicator as value.

As we now have the fine grained permission concept in place a user requires at least the can_edit entitlement on the server entity type in order to view and edit the server configuration. Just by having the viewer entitlement on server doesn't return the server's configuration.
If the necessary permissions are granted we can also safely return the passwords.

[github-actions]

Heads up @ru-fu - the "Documentation" label was applied to this issue.

[roosterfish]

@tomponline Do we require an API extension when changing the server config (ServerPut) from map[string]any to map[string]string? From an end user perspective there shouldn't be a difference I guess.

[tomponline]

Do we require an API extension when changing the server config (ServerPut) from map[string]any to map[string]string? From an end user perspective there shouldn't be a difference I guess.

Hrm. Im not sure we can do that. Aside from apps that link to the LXD Go SDK needing to be updated, how will the LXD Go SDK then talk with older versions of the server? Is the underlying JSON API format staying the same? Presumably not if previously there were integer fields which would now be sent as string.

[roosterfish]

Is the underlying JSON API format staying the same?

We might be lucky here. Even though the server conf map currently is of type map[string]any, you cannot supply anything else than string:

julian@thinkpad:~$ lxc query -X PUT /1.0 -d '{"config":{"images.auto_update_interval":42}}'
Error: cannot set 'images.auto_update_interval': invalid type float64

(Same for the PATCH endpoint)

This is because we check using reflections that the value is actually of type string: https://github.com/canonical/lxd/blob/main/lxd/config/map.go#L63
Changing the map to map[string]string wouldn't break any existing setups as any user of the SDK is already required to pass strings only and LXD is storing everything as string anyway:

julian@thinkpad:~$ lxc config show 
config:
  core.https_allowed_credentials: "true"
  images.auto_update_interval: "42"

[tomponline]

Is the underlying JSON API format staying the same?

We might be lucky here. Even though the server conf map currently is of type map[string]any, you cannot supply anything else than string:

julian@thinkpad:~$ lxc query -X PUT /1.0 -d '{"config":{"images.auto_update_interval":42}}'
Error: cannot set 'images.auto_update_interval': invalid type float64

(Same for the PATCH endpoint)

This is because we check using reflections that the value is actually of type string: https://github.com/canonical/lxd/blob/main/lxd/config/map.go#L63 Changing the map to map[string]string wouldn't break any existing setups as any user of the SDK is already required to pass strings only and LXD is storing everything as string anyway:

julian@thinkpad:~$ lxc config show 
config:
  core.https_allowed_credentials: "true"
  images.auto_update_interval: "42"

OK thats nice to see.

[tomponline]

@markylaing are you happy with this change wrt to the trust password in the server config only being viewable if the user has sufficient permissions to do so?

[roosterfish]

The same concept would then apply for storage pools. Currently you can only view the pools configuration if you have the can_edit on the respective storage_pool. A simple user of the storage pool cannot view the configuration (and passwords like the PowerFlex gateway user password).

[markylaing]

@markylaing are you happy with this change wrt to the trust password in the server config only being viewable if the user has sufficient permissions to do so?

Sorry I missed this. Yep, sensitive information won't be viewable by anyone that doesn't have can_edit on server. That being said, before any fine-grained authorization was added, only administrators were able to view server config and hidden items were still hidden. I'm not sure what the initial intent was behind this.

[tomponline]

@roosterfish please can we add tests to this PR to ensure that the trust password isn't being revealed to non-admin users or users who dont have the correct oidc permission?

[roosterfish]

@tomponline @markylaing config modification tests are added for can_edit on server and storage_pool.

[markylaing]

Tests look good, thanks :)

[tomponline]

Can we explicitly set and check access to core.trust_password and loki.auth.password are not accessible please?

[tomponline]

Can we also check that a non-admin cert (without fine grained auth) cant access these settings please.

[roosterfish]

Yes sure. Do you think checking for one of them would be enough? They are all in the same map[string]string. Therefore I have added the length == 0 checks to be sure you can't actually see anything.

[roosterfish]

I'll add a follow up PR.

[tomponline]

Im just sort of paranoid about leaking passwords :)

[roosterfish]

Fixed in #13502

[MusicDin]

Hei, we have issues in Terraform provider using the latest version of LXD client due to the change of config: map[string]any -> config: map[string]string in api.ServerPut struct.

The issue appears to be that when core.trust_password is set, the response from the server cannot be unmarshaled from JSON because trust_password is of type bool instead of string.

---

Here is a reproducer. I've used LXD 5.0.3 (same issue occurrs on all older LXD versions except latest/edge), and have created an instance named c1 (does not matter what type/image is used).

// go.mod
module tmp

go 1.22.0

// 2024-05-18
require github.com/canonical/lxd v0.0.0-20240518082524-d8433ff62358

// main.go
package main

import (
	"fmt"
	"os"

	lxd "github.com/canonical/lxd/client"
)

func instanceInfo(instName string) error {
	// Connect to LXD over unix socket.
	lxdSocket := "/var/snap/lxd/common/lxd/unix.socket"
	instServer, err := lxd.ConnectLXDUnix(lxdSocket, nil)
	if err != nil {
		return fmt.Errorf("connect lxd unix: %v", err)
	}

	s, _, err := instServer.GetServer()
	fmt.Println("Server version:", s.Environment.ServerVersion)

	// Fetch instance with the given name.
	inst, _, err := instServer.GetInstance(instName)
	if err != nil {
		return fmt.Errorf("get instance: %v", err)
	}

	// Print instance's config.
	fmt.Println("Instance name:", inst.Name)
	fmt.Println("Instance arch:", inst.Architecture)
	fmt.Println("Instance type:", inst.Type)
	fmt.Println("Instance stat:", inst.Status)

	return nil
}

func main() {
	err := instanceInfo("c1")
	if err != nil {
		fmt.Fprintf(os.Stderr, "Error: %v", err)
		os.Exit(1)
	}
}

If trust_password is not configured:

$ lxc version
Client version: 5.0.3
Server version: 5.0.3

$ go run .
Server version: 5.0.3
Instance name: c1
Instance arch: x86_64
Instance type: container
Instance stat: Running

Now if we set the trust_password:

$ lxc config set core.trust_password=test
$ go run .
Error: connect lxd unix: json: cannot unmarshal bool into Go struct field Server.config of type string

---

The error occurs in https://github.com/canonical/lxd/blob/main/shared/api/response.go#L77-L80.

Trace would look like this:

1. client/connection.go#ConnectLXDUnixWithContext (Could be any ConnectLXD method)
2. client/lxd_server#GetServer
3. client/lxd.go#queryStruct
4. shared/api/response.go#MetadataAsStruct

---

Any suggestions how should we approach this issue?

[roosterfish]

@MusicDin thanks for the reproducer steps! I am looking into it now.

[MusicDin]

Thanks 🚀