ssh-based provisioner: Re-enable support for PowerShell

[radeksimko]

Context (Why)

This is a further follow-up on #30661 and also to #28626 (shipped in v1.1.0).

PowerShell provisioning did work prior to the linked patch (confirmed with v1.0.11), though of course suffered from the command injection problem outlined in #28626

Further Testing

The functionality can be manually tested (beyond just trusting the unit tests) using the following repo I prepared: https://github.com/radeksimko/tf-azure-windows-ssh

What the PR does

The PR changes escaping for the command name (arg[0]) such that it remains compatible with both cmd.exe and PowerShell, i.e. avoids over-escaping it (see below). It still escapes all arguments, and also adds tests to confirm that is the case - i.e. confirms that we did not re-introduce issues that #28626 aimed to solve.

The initial command is always scp - so I updated the function name to also reflect this.

Before

"scp" -vt C:\Path

After

scp -vt C:\Path

TODO

• Raise docs PR in https://github.com/hashicorp/web-unified-docs to mention PowerShell support and maybe what expectations there remain for remote-exec inline syntax (still requires powershell ...)

Target Release

1.15.x

Rollback Plan

• If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

• This change is user-facing and I added a changelog entry.
• This change is not user-facing.

[radeksimko]

My understanding based on reading the upstream library and also manual testing in a Windows VM is that Windows does not let you break out of the command or at least not in the context of how we execute scp as the shell does not seem to interpret the individual arguments directly. That is why escaping of special characters on Windows does not need to be as rigorous in this particular context.

I'm happy to be corrected here. Either way the PR isn't introducing anything new here - it's only changing how the command name gets treated, all arguments are treated exactly the same as before.

[eastebry]

Understood. I don't have much experience with Powershell, so I'd defer to you judgement here.

[radeksimko]

Regarding escaping in general, it seems important to repeat that escaping was initially introduced as a precaution anyway and it does not change anything about the threat model. We only escape the scp command and not the code being executed. This PR does not change anything about that either.

Relatedly, while remote-exec provisioner does execute code, it concatenates all the contents into a file, uploads it to the server, and then executes that file. So a compromise would need to happen prior to the file being uploaded via scp. If we decided to somehow protect the user in that context it would imply pre-processing the entire script and trying to work out what the user meant. That carries its own trade offs and represents a different threat.

If this is actually a threat the user is concerned about they could for example apply restrictions to the account that ends up SSHing in and executing the code or ensure that the variable input that can make the script vulnerable is escaped in its own context.

Finally, the same kind of threat would apply to server-side provisioning, such as via cloud-config. If user allows passing of untrusted input in the context of remote-exec or cloud-config, it is their own responsibility to escape it correctly, though it's probably a bad idea to begin with.

[eastebry]

Great set of test cases!

[SarahFrench]

Approved after manual testing using the resources in the PR description (thank you!).
From my manual tests I can see that:

• I can use either of the two shells ok
• When I include strange values for the destination attribute such as destination = "C:/Users/azure; echo "FOOBAR; /hello-file.txt" I get an error instead of the file uploaded to C:/Users/azure and then the echo command being executed:

scp: C:/Users/azure; echo "FOOBAR"; : No such file or directory

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.