Skip to content

Commit

Permalink
Merge pull request #529 from MusicDin/fix/token-remote-cert
Browse files Browse the repository at this point in the history
Do not blindly accept remote certificate when using trust token
  • Loading branch information
simondeziel authored Sep 23, 2024
2 parents 047574e + d82682d commit 40eacc7
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 22 deletions.
18 changes: 8 additions & 10 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,13 @@ The following arguments are supported:
configuration. Defaults to `$HOME/snap/lxd/common/config` (and fallbacks to `$HOME/.config/lxc`).

* `generate_client_certificates` - *Optional* - Automatically generate the LXD
client certificate if it does not exist. Valid values are `true` and `false`.
This can also be set with the `LXD_GENERATE_CLIENT_CERTS` Environment
variable. Defaults to `false`.
client certificate if it does not exist. Defaults to `false`.

* `accept_remote_certificate` - *Optional* - Automatically accept the LXD
remote's certificate. Valid values are `true` and `false`. If this is not set
to `true`, you must accept the certificate out of band of Terraform. This can
also be set with the `LXD_ACCEPT_SERVER_CERTIFICATE` environment variable.
Defaults to `false`
remote's certificate during initial authentication. If this is not set
to `true`, you must accept the certificate out of band of Terraform or
use a trust token instead (recommended, see `token` in `remote`).
Defaults to `false`.

The `remote` block supports:

Expand All @@ -85,7 +83,6 @@ The `remote` block supports:

* `default` - *Optional* - Whether this should be the default remote.
This remote will then be used when one is not specified in a resource.
Valid values are `true` and `false`.
If you choose to _not_ set default=true on a `remote` and do not specify
a remote in a resource, this provider will attempt to connect to an LXD
server running on the same host through the UNIX socket. See `Undefined Remote`
Expand All @@ -108,12 +105,13 @@ socket.
## Environment Variable Remote

It is possible to define a single `remote` through environment variables.
The required variables are:

The supported variables are:
* `LXD_REMOTE` - The name of the remote.
* `LXD_ADDR` - The address of the LXD remote.
* `LXD_PASSWORD` - The password of the LXD remote.
* `LXD_TOKEN` - The trust token of the LXD remote.
* `LXD_GENERATE_CLIENT_CERTS` - Automatically generate the LXD client certificate if missing.
* `LXD_ACCEPT_SERVER_CERTIFICATE` - Automatically accept the LXD remote's certificate during initial authentication.

## PKI Support

Expand Down
28 changes: 22 additions & 6 deletions internal/provider-config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (p *LxdProviderConfig) server(remoteName string) (lxd.Server, error) {
}

// Try to accept the remote certificate.
err := p.acceptRemoteCertificate(remoteName, remote.Address)
err := p.acceptRemoteCertificate(remoteName, remote.Token, remote.Address)
if err != nil {
return nil, fmt.Errorf("Failed to accept server certificate for remote %q: %v", remoteName, err)
}
Expand Down Expand Up @@ -428,12 +428,14 @@ func (p *LxdProviderConfig) setRemote(remoteName string, remote LxdRemote) error

// acceptRemoteCertificate retrieves the unverified peer certificate found at
// the remote address and stores it locally.
func (p *LxdProviderConfig) acceptRemoteCertificate(remoteName string, url string) error {
// Check if we are allowed to accept the remote certificate.
if !p.acceptServerCertificate {
func (p *LxdProviderConfig) acceptRemoteCertificate(remoteName string, token string, url string) error {
// Check if we are allowed to blindly accept the remote certificate.
// When the trust token is used, the fingerprint contained in the token
// is used to ensure we get the right certificate.
if token == "" && !p.acceptServerCertificate {
return errors.New("Unable to communicate with remote server. " +
`Either set "accept_remote_certificate" to true or add ` +
"the remote out of band of Terraform and try again.")
`You can set "accept_remote_certificate" to true, add ` +
"the remote out of band of Terraform, or use the trust token.")
}

// Try to retrieve server's certificate.
Expand All @@ -442,6 +444,20 @@ func (p *LxdProviderConfig) acceptRemoteCertificate(remoteName string, url strin
return err
}

if token != "" {
// Decode token.
decodedToken, err := shared.CertificateTokenDecode(token)
if err != nil {
return fmt.Errorf("Failed decoding trust token for remote %q: %v", remoteName, err)
}

// Compare token and certificate fingerprints.
certFingerprint := shared.CertFingerprint(cert)
if certFingerprint != decodedToken.Fingerprint {
return fmt.Errorf("Fingerprint mismatch between trust token and certificate from remote %q", remoteName)
}
}

certPath := p.config.ServerCertPath(remoteName)
certDir := filepath.Dir(certPath)

Expand Down
18 changes: 12 additions & 6 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func TestAccProvider_configDir(t *testing.T) {
},
},
})
resetLXDRemoteEnvVars()
}

func TestAccProvider_trustToken(t *testing.T) {
Expand All @@ -41,6 +40,13 @@ func TestAccProvider_trustToken(t *testing.T) {
token := acctest.ConfigureTrustToken(t)
configDir := t.TempDir()

invalidToken := `
ewogICJjbGllbnRfbmFtZSI6ICJ0bXBfdG9rZW4iLAogICJmaW5nZXJwcmludCI6ICJZb3VfaGF2
ZV9kZWNvZGVkX2FfdGVtcG9yYXJ5X3Rva2VuLkNvbmdyYXR1bGF0aW9ucyEiLAogICJhZGRyZXNz
ZXMiOiBbCiAgICAiMTI3LjAuMC4xOjg0NDMiCiAgXSwKICAic2VjcmV0IjogIlRoaXNfaXNfYV90
b3Bfc2VjcmV0LkRvX25vdF90ZWxsX2l0X3RvX2FueW9uZSEiLAogICJleHBpcmVzX2F0IjogIjAw
MDEtMDEtMDFUMDA6MDA6MDBaIgp9Cg==`

resource.Test(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(t)
Expand All @@ -50,8 +56,8 @@ func TestAccProvider_trustToken(t *testing.T) {
Steps: []resource.TestStep{
{
// Ensure authentication fails with incorrect token.
Config: testAccProvider_remoteServer(configDir, "", "invalid", true),
ExpectError: regexp.MustCompile(`not authorized`),
Config: testAccProvider_remoteServer(configDir, "", invalidToken, true),
ExpectError: regexp.MustCompile(`mismatch between trust token and certificate`),
},
{
// Ensure authentication succeeds with correct token.
Expand Down Expand Up @@ -154,7 +160,7 @@ func TestAccProvider_generateClientCertificate(t *testing.T) {
}

func TestAccProvider_acceptRemoteCertificate(t *testing.T) {
token := acctest.ConfigureTrustToken(t)
password := acctest.ConfigureTrustPassword(t)
configDir := t.TempDir()

resource.Test(t, resource.TestCase{
Expand All @@ -166,12 +172,12 @@ func TestAccProvider_acceptRemoteCertificate(t *testing.T) {
Steps: []resource.TestStep{
{
// Ensure authentication fails if remote server is not accepted.
Config: testAccProvider_remoteServer(configDir, "", token, false),
Config: testAccProvider_remoteServer(configDir, password, "", false),
ExpectError: regexp.MustCompile(`Failed to accept server certificate`),
},
{
// Ensure authentication succeeds if remote server is accepted.
Config: testAccProvider_remoteServer(configDir, "", token, true),
Config: testAccProvider_remoteServer(configDir, password, "", true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("lxd_noop.noop", "remote", "tf-remote"),
resource.TestCheckResourceAttr("lxd_noop.noop", "project", "default"),
Expand Down

0 comments on commit 40eacc7

Please sign in to comment.