SSH CA support in qemu+ssh?

git-noise commented 2 years ago

Hello,

It seems that the plugin does not support the use of SSH CA (ssh public keys signed by a CA for SSH authentication). The same setup works with virt-manager. Maybe this is a gap in functionalities when the switch was done rewriting the SSH provider?

Many thanks for your insight.

System Information

Linux distribution

Ubuntu

Terraform version

Terraform v1.2.2
on linux_amd64
+ provider registry.terraform.io/dmacvicar/libvirt v0.6.14
+ provider registry.terraform.io/hashicorp/template v2.2.0

Provider and libvirt versions

Terraform v1.2.2
on linux_amd64
+ provider registry.terraform.io/dmacvicar/libvirt v0.6.14
+ provider registry.terraform.io/hashicorp/template v2.2.0

Checklist

[x] Is it a bug or something that does not work as expected? Please make sure you fill the version information below:

Description of Issue/Question

Setup

terraform {
  required_version = ">= 1.0.1"
  required_providers {
    libvirt = {
      source = "dmacvicar/libvirt"
      version = "0.6.14"
    }
  }
}

provider "libvirt" {
  uri = "qemu+ssh://username@fqdn/system&privkey=path_to_key&sshauth=privkey&no_verify=1&no_tty=1"
}

# cloud-init config pool and ISO disk
resource "libvirt_cloudinit_disk" "commoninit" {
  name           = "commoninit-test.iso"
  user_data      = data.template_file.user_data.rendered
  network_config = data.template_file.network_config.rendered
  pool           = "cloud-init"
}

data "template_file" "user_data" {
  template = file("${path.module}/cfg/cloud_init.cfg")
}
data "template_file" "network_config" {
  template = file("${path.module}/cfg/network_config.cfg")
}

This triggers

2022-06-15T11:03:49.498-0400 [DEBUG] provider: plugin process exited: path=.terraform/providers/registry.terraform.io/hashicorp/template/2.2.0/linux_amd64/terraform-provider-template_v2.2.0_x4 pid=161675
2022-06-15T11:03:49.498-0400 [DEBUG] provider: plugin exited
2022-06-15T11:03:49.540-0400 [ERROR] vertex "provider[\"registry.terraform.io/dmacvicar/libvirt\"]" error: failed to dial libvirt: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
2022-06-15T11:03:49.541-0400 [INFO]  backend/local: plan operation completed
╷
│ Error: failed to dial libvirt: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
│ 
│   with provider["registry.terraform.io/dmacvicar/libvirt"],
│   on main.tf line 11, in provider "libvirt":
│   11: provider "libvirt" {
│ 
╵
2022-06-15T11:03:49.543-0400 [DEBUG] provider: plugin process exited: path=.terraform/providers/registry.terraform.io/dmacvicar/libvirt/0.6.14/linux_amd64/terraform-provider-libvirt_v0.6.14 pid=161688
2022-06-15T11:03:49.543-0400 [DEBUG] provider: plugin exited

Steps to Reproduce Issue

Just plan or apply:

terraform plan

Additional information:

Do you have SELinux or Apparmor/Firewall enabled? Some special configuration? Have you tried to reproduce the issue without them enabled?

Does not seem to be related - fails with or without.

git-noise commented 2 years ago

@dmacvicar Would you have interest in merging such a feature?

As golang.org/x/crypto/ssh supports it already, it would imply minor additions to at https://github.com/dmacvicar/terraform-provider-libvirt/blob/main/libvirt/uri/ssh.go as far as I could test.

It essentially consists in:

use the same signer from ssh.ParsePrivateKey()
parse the cert as a public key with ssh.ParseAuthorizedKey()
build a new ssh.Signer from 1 and 2 using ssh.NewCertSigner()

It could either be:

a sub-branch of privkey in https://github.com/dmacvicar/terraform-provider-libvirt/blob/main/libvirt/uri/ssh.go#L54 (as you still need to build the signer from the private key). In this case one would just check the presence of the cert passed in the URL and if present build the new signer. Otherwise things would unfold as usual.
a new AuthMethod case switch all together in https://github.com/dmacvicar/terraform-provider-libvirt/blob/main/libvirt/uri/ssh.go#L40 (but in that case you would duplicate the few lines from case privkey that builds the initial ssh.Signer from the private key.

I have a working code, but the main issue is that it works well against golang.org/x/crypto/ssh only (in my current setup), not the custom branch you're using. I could not quickly determine which commits to cherry-pick, but if I had to guess I'd say it's related to the changes in client_auth.go stemming from the RFC 8308 changes in golang.org/x/crypto/ssh.

I think the rational for using that fork was due to missing RFC 8308, but I believe the client part server-sig-algs has been merged upstream, so would it be enough to fall-back on golang.org/x/crypto/ssh?

Thanks,

dmacvicar commented 2 years ago

Yes. Definitely interested. Thanks for the detailed report.

If the commits are merged upstream, we could go back to the mainstream x/crypto/ssh.

git-noise commented 2 years ago

@dmacvicar great thanks, I'll send a PR soon. I'll take the following approach:

Sub-case of current privkey authentication method
Try to read a cert file by using SSH canonical names: adding -cert.pub to the privkey file name

Rationales are:

SSH Cert authentication still very much needs private key so one could say it should not be independent
Using a sub-case avoids adding an authentication methods and URI parameters which would not be compliant with the libvirt URI (in the spirit of staying close to the original client).
I would make the hypothesis that this should be close to the original client behavior: as an example, virt-manager used with the URI fragment system?sshauth=privkey&keyfile=/path/to/.ssh/id_ed25519_keyseems to connect successfully using certificate file /path/to/.ssh/id_ed25519_key-cert.pub if its present.

git-noise commented 2 years ago

PR opened as draft - as it is not using the golang.org/x/crypto/ssh custom branch - let me know if things look good.

git-noise commented 2 years ago

@dmacvicar Hello, I closed my draft PR, I had issues rebasing it against current master: it seems that at least github.com/hashicorp/go-getter dependency impedes building. Additionally it may also make sense to wait for the main branch to first switch back to upstream x/crypto, rather than having that change re-introduced by such a specific feature. Anyway let me know and I can always re-open a proper PR.

pimvh commented 2 years ago

Hi! I see you opened another PR related to this that is also closed. Excuse If I am asking for this in the wrong place, but do you perhaps have a status update? As I understand your fix is dependant on an external library?

I would love to contribute too, if necessary. Tell me where to start.

git-noise commented 2 years ago

@pimvh I ended up closing my PR because:

I had issues rebasing it due to recent changes in main - that seems solved for now
It requires to use the upstream go ssh and that is a big enough change that it may needs the maintainer input to see how to introduce it (maybe in a separated commit altogether, not a feature branch).

That being said I do have a working branch that I am using locally, just waiting for dmacvicar to see how to proceed.

@dmacvicar, pinging you as there is interest: if you are still interested in the feature, how would you wish to handle the switch to upstream golang.org/x/crypto/ssh? A separate branch? included in my feature branch?

pimvh commented 1 year ago

Hi all! I am still interested in this feature. Is there any progress? Can I contribute?

git-noise commented 1 year ago

Hello, I have not heard back from the maintainer. I still have my feature branch that I am using locally. I'll try to re-open a PR this week or the next.

pimvh commented 1 year ago

@dmacvicar great thanks, I'll send a PR soon. I'll take the following approach:

* Sub-case of current `privkey` authentication method

* Try to read a cert file by using SSH canonical names: adding `-cert.pub` to the `privkey` file name

Rationales are:

* SSH Cert authentication still very much needs private key so one could say it should not be independent

* Using a sub-case avoids adding an authentication methods and URI parameters which would not be compliant with the libvirt URI (in the spirit of staying close to the original client).

* I would make the hypothesis that this should be close to the original client behavior: as an example, virt-manager used with the URI fragment `system?sshauth=privkey&keyfile=/path/to/.ssh/id_ed25519_key`seems to connect successfully using certificate file `/path/to/.ssh/id_ed25519_key-cert.pub` if its present.

Hi, I have tried following the steps you outlined in this comment, but do not know which specific version of golang.org/x/crypto/sshwe need. I've tried just bumping the version, but that does not lead to a working version of the code, if my local tests are correct.

git-noise commented 1 year ago

I am just using plain upstream golang.org/x/crypto/ssh. I have a fork ready to be PR-ed again, I'll probably get to it next week if you can wait.

git-noise commented 1 year ago

@pimvh My fork is ready, but it seems I have issues with the latest 0.7.2 versions. I am trying to figure out where the issue is before I PR something.

scabala commented 2 months ago

Hi @git-noise, this is great feature! Would you be interested in crafting new PR with upstream SSH library?

git-noise commented 2 months ago

@scabala @dmacvicar Hello, I can have a look at what I had back in the days. I think something in the 0.7 version had broken it, but I'll see if I can make it work.

git-noise commented 1 month ago

Quick update, I extracted the code into a separate branch. I'll try and send a draft PR for reference, but I cannot really test it for now, so may take a few weeks/months until I can get something up and running.

dmacvicar / terraform-provider-libvirt