lang/funcs: Add support for OpenSSH RSA key format#25112
Conversation
Codecov Report
|
lang/funcs/crypto.go
Outdated
| rawKey, err := ssh.ParseRawPrivateKey([]byte(key)) | ||
| if err != nil { | ||
| return cty.UnknownVal(cty.String), err | ||
| return cty.UnknownVal(cty.String), fmt.Errorf("failed to parse key: %s", err) |
There was a problem hiding this comment.
is the Errorf text needed here? Just wondering if we're going to get errors like failed to parse key: fail to parse SSH key
There was a problem hiding this comment.
I had kinda hoped that the encoding/asn1 package would produce machine-readable error values that we could use to present something more appropriate for the Terraform end-user audience, 'cause from what I see in #24970 these messages seem pretty poor, but sadly it looks like StructuralError and SyntaxError are both just wrappers around opaque strings formatted in the internals of the parser. 😖
With that said, I reviewed the parser code and it looks like this "tags don't match" error is a particularly ugly example but most of them are reasonable messages that suffer only from a total lack of useful context, like "base 128 integer too large". So the two forms this should end up producing are:
failed to parse key: asn1: structure error: tags don't match (16 vs {class:1 tag:15 length:112 isCompound:true}) {optional:false explicit:false application:false private:false defaultValue:<nil> tag:<nil> stringType:0 timeType:0 set:false omitEmpty:false} pkcs1PrivateKey @2.failed to parse key: asn1: syntax error: invalid boolean
I think the only way we could marginally improve these with the information we have is to recognize when the error type is either asn1.SyntaxError or asn1.StructuralError and generate a different prefix (replacing asn1: syntax error:) that is more in line with our usual writing style, like:
invalid ASN1 data in the given private key: invalid booleaninvalid ASN1 data in the given private key: tags don't match (16 vs {class:1 tag:15 length:112 isCompound:true}) {optional:false explicit:false application:false private:false defaultValue:<nil> tag:<nil> stringType:0 timeType:0 set:false omitEmpty:false} pkcs1PrivateKey @2.
(the second one isn't going to be nice whatever we do to it, of course...)
A further embellishment, though very much not necessary, would be to use function.NewArgErrorf to associate this error explicitly with the second argument (argument index 1), and then HCL should indicate that argument's experession in particular as being the problematic part of the configuration.
There was a problem hiding this comment.
Thanks for the suggestions!
I think it is important to process these errors rather than just pass them through, because the crypto errors are pretty obscure. Adding even basic error annotations was the first step to debugging this.
I've made the suggested changes and I think it's a big improvement:
velara:24970 $ tf apply -var-file=trunc.tfvars
Error: Invalid function argument
on example.tf line 13, in output "password_decrypted":
13: value=rsadecrypt(var.password_data, file("${path.module}/${var.rsa_private_key_file}") )
|----------------
| path.module is "."
| var.rsa_private_key_file is "id_rsa_trunc"
Invalid value for "privatekey" parameter: invalid ASN1 data in the given
private key: data truncated.
Previously this function only supported the x509 RSA private key format. More recent versions of OpenSSH default to generating a new PEM key format, which this commit now supports using the x/crypto/ssh package. Also improve the returned error messages for various invalid ciphertext or invalid private key errors.
alisdair
force-pushed
the
alisdair/support-openssh-rsa-key-encoding
branch
from
91a2722 to
a4f5e04
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
3 participants
Keys generated by recent versions of OpenSSH use a new OpenSSH PEM format which is incompatible with the
x509package. We can work around this by using theParseRawPrivateKeyfromx/crypto/sshand type asserting that we get the expected RSA key type back.ParseRawPrivateKeydocsParseRawPrivateKeysource, where you can see that it delegates depending on key typersa.PrivateKey, which is what ourrsadecryptfunction needs to proceedThe new OpenSSH private key fixture was generated by re-encoding the existing private key fixture using this command on macOS:
Fixes #24970.