Add support for client certificates to -output-curl-string #13660
Conversation
I did not write tests for this feature as -output-curl-string was not already tested and this is a simple change. Because the name of the certificates would be lost once loaded I added fields to Config to keep track of them. I did not add a public method for the user to set them explicitely as I don't think anyone would need this functionnality outside of the Vault CLI. Closes hashicorp#13376
| @@ -46,6 +48,22 @@ func (d *OutputStringError) parseRequest() { | |||
| if d.Request.Method != "GET" { | |||
| d.parsedCurlString = fmt.Sprintf("%s-X %s ", d.parsedCurlString, d.Request.Method) | |||
| } | |||
| if d.ClientCACert != "" { | |||
| clientCACert := strings.Replace(d.ClientCACert, "'", "'\"'\"'", -1) | |||
There was a problem hiding this comment.
| clientCACert := strings.Replace(d.ClientCACert, "'", "'\"'\"'", -1) | |
| clientCACert := strings.Replace(d.ClientCACert, "'", `'"'"'`, -1) |
I'm still not sure I understand it even with that change.
There was a problem hiding this comment.
This is so that if the filename contains a special character:
$ ls
clie'nt.crt client.key
then the ' is escaped in the output of -output-curl-string:
$ ../bin/vault read -client-cert clie\'nt.crt -client-key client.key -output-curl-string foo
curl --cert 'clie'"'"'nt.crt' --key 'client.key' -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" https://127.0.0.1:8200/v1/foo
The --cert option is the concatenation of three strings 'clie', "'" and nt.crt so it gives "clie'nt.crt".
For what it's worth this is very naive, it does not escape $(...) or `` which would also interpreted by the shell. I don't think this is a solvable problem thought as different shells will interpret different constructs. I used this method to escape ' because it is already used for the body.
remilapeyre
force-pushed
the
output-curl-string-certs
branch
from
b48e5b4
to
a6de90e
Compare
There was a problem hiding this comment.
Lgtm!
I've done some basic testing to verify that the cert and keys are included now in the output-curl-string. I also looked through the callers of ConfigureTLS to double-check if grabbing the modifyLock in the function could cause issues, but it all looked good to me.
Thanks for submitting this PR!
I did not write tests for this feature as -output-curl-string was not
already tested and this is a simple change. Because the name of the
certificates would be lost once loaded I added fields to Config to keep
track of them. I did not add a public method for the user to set them
explicitely as I don't think anyone would need this functionnality
outside of the Vault CLI.
Closes #13376