automtls: fix bidirectional communication when AutoMTLS is enabled

[fairclothjm]

Currently enabling AutoMTLS when connecting from a plugin process back to the host results in the following error:

transport: authentication handshake failed: x509: “localhost” certificate is not standards compliant

This is similar but not identical to the error described in #109 and #179

This PR enables the client and server to successfully establish the mTLS connection for bidirectional communication.

[calvn]

Took a look at how we generate certs on Vault for mTLS via bootstrap + wrapping token, and it looks the same as this modulo the issuer addition. Do you get the error if issuer is not specified?

[fairclothjm]

Took a look at how we generate certs on Vault for mTLS via bootstrap + wrapping token, and it looks the same as this modulo the issuer addition. Do you get the error if issuer is not specified?

@calvn Yes, I do get the same error if the Issuer is not specified. But I did some more testing just now and if I don't set the Subject.Organization field then it works without setting the Issuer. generateCert in sdk/helper/pluginutil/tls.go also does not set the Subject.Organization.

Do you think that approach is preferable? I don't see any downside either way

[jbardin]

I haven't thought about this in a long time, so I'm not going to try and comment about what is the more "correct" method for cert generation, but I would be concerned with making sure the changes is compatible between client and server versions. I would verify that existing clients and servers on different minor versions can interoperate with this change in either direction, otherwise we will need some way to make it optional to prevent needing a new major version.

[calvn]

Yes, I do get the same error if the Issuer is not specified. But I did some more testing just now and if I don't set the Subject.Organization field then it works without setting the Issuer. generateCert in sdk/helper/pluginutil/tls.go also does not set the Subject.Organization.

Interesting, I wonder how mTLS is working on Vault today since it doesn't seem to specify Issuer on cert generation.

WRT the organization field in Subject, I'd say that it's fine to leave it since that's been always present on go-plugin and doesn't seem to interfere with mTLS.

[fairclothjm]

I wonder how mTLS is working on Vault today

Database plugins don't use the brokered connections back to the host. Using GRPCBroker is when I am seeing the Certificate errors occur.

[calvn]

Left a few small comments/questions that other may answer too, but overall the change looks good! Ran the tests locally and they all passed.

[cipherboy]

Looks good from a crypto perspective. :-)

[jbardin]

Tested with both before+after client+server combinations using Terraform providers, and all seemed to work well!