style(cli): boundary connect output returns pasteable X.509 certificates #3232
Conversation
|
For reference, here is the unchanged
|
| switch iface := iface.(type) { | ||
| case nil: | ||
| out = append(out, fmt.Sprintf("%s %s:", prefixStr, k)) | ||
| case map[string]any: |
There was a problem hiding this comment.
The switch cases currently assume the PEM certificate will be located in the first json layer of a vault kv store i.e., data: <key_name>: <.crt here>. If it is possible for the certificate to be located in a different location i.e. data: <key_name>: <another_key_name>: <.crt here>, then the output will not display properly.
If it is possible for an 'unspecified' Boundary credential to come from a different vault path with a different expected format, then it might not be displayed properly either. Do you know what possible 'unspecified' Boundary credential formats I could expect to find, or who I can ask about this topic?
| out = append(out, fmt.Sprintf("%s %s:", prefixStr, k)) | ||
| certs := make(map[string]any, len(iface)) | ||
| for kk, vv := range iface { | ||
| if p, _ := pem.Decode([]byte(fmt.Sprintf("%v", vv))); p != nil { |
There was a problem hiding this comment.
I'd sooner we have another type assertion than this string formatting, to make it a bit more predictable
switch iface := iface.(type) {
case []byte:
if p, _ := pem.Decode(iface); p != nil {etc. Maybe one for string too. What do you think?
There was a problem hiding this comment.
That sounds like a better way to do it
There was a problem hiding this comment.
@johanbrandhorst would you expect a case where the unmarshalled interface is anything other than a []byte or a string? What would you recommend I do in that scenario? Does it sound reasonable to return an error in that case?
There was a problem hiding this comment.
We might expect it to be a numeric type if it's not one of those, so I don't think we should panic, in those cases we could fall back to fmt.Sprintf maybe?
|
Why 0.14 and not 0.13? |
|
@jefferai I need to format the metadata correctly again after adding the second case statement, and I still need to implement e2e tests. It should probably still be 0.13, but I wanted to avoid confusion with the feature freeze going on |
When a target accesses a session credential from a credential library with a vault path pointing to a .crt certificate, this credential is treated as an unspecified credential (which isn't username-password, ssh-private-key, or json type). Instead of being unmarshalled into a struct, the raw secret is decoded into a []byte by
base64.StdEncoding.DecodeString, and then sent to abytes.Bufferbyjson.Indentbefore being converted back into a string. This results in the certificate being returned in the CLI output as a json value containing\ncharacters like so (and in my environment there's some\rs as well):I had thought that the newline characters were a valid format for a crt file , but I was told by the reporter that this format rendered the certificate useless for their kubernetes workflow. Furthermore, the newline between "BEGIN" and "CERTIFICATE" and the tabs before each "CERTIFICATE" makes the output bad to copy/paste.
I understand that setting -format=json fixes the latter issue.
This PR attempts to unmarshal and print all unspecified credential values without indentation when
-format=table, so the certificates can be displayed like so in a nice-looking block.https://hashicorp.atlassian.net/browse/ICU-7421