Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore custom_tls client_cert, client_key are not always required for use with for example an AWS RDS cert #181

Open
david-heward-unmind opened this issue Nov 7, 2024 · 5 comments

Comments

@david-heward-unmind
Copy link

david-heward-unmind commented Nov 7, 2024

Hi there,

I believe client_cert and client_key are, in fact, not required and, in many cases, should be ignored. Therefore, I'm wondering if they are incorrectly configured and we could make them optional in both the provider and the go code?

Current behaviour

Forces you to set a value.

clientCert := make([]tls.Certificate, 0, 1)
var certs tls.Certificate
if strings.HasPrefix(customTLS.ClientCert, "-----BEGIN") {
certs, err = tls.X509KeyPair([]byte(customTLS.ClientCert), []byte(customTLS.ClientKey))
} else {
certs, err = tls.LoadX509KeyPair(customTLS.ClientCert, customTLS.ClientKey)
}
if err != nil {
return nil, diag.Errorf("error loading keypair: %v", err)
}

Where something like this is all thats required for referencing of a custom CA.

rootCertPool := x509.NewCertPool()
    pem, err := ioutil.ReadFile("/path/ca-cert.pem")
    if err != nil {
       log.Fatal(err)
    }
    if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
       log.Fatal("Failed to append PEM.")
    }
    mysql.RegisterTLSConfig("custom", &tls.Config{
                             ServerName: "qcaurora.cb556lynvxio.us-east-1.rds.amazonaws.com",
                             RootCAs: rootCertPool,
                            })
    db, err := sql.Open("mysql", "user:pass@tcp(qcrds.example.com:3306)/databasename?tls=custom")
@petoju
Copy link
Owner

petoju commented Nov 8, 2024

Therefore, I'm wondering if they are incorrectly configured and we could make them optional in both the provider and the go code?

Yes, that is possible - but it would have to be correctly handled in code and no one has needed it so far.
Once someone needs it, they can send a PR (or if I happen to need it, I will send a PR) and add this possibility.

@david-heward-unmind
Copy link
Author

Therefore, I'm wondering if they are incorrectly configured and we could make them optional in both the provider and the go code?

Yes, that is possible - but it would have to be correctly handled in code and no one has needed it so far. Once someone needs it, they can send a PR (or if I happen to need it, I will send a PR) and add this possibility.

I'll submit something this week 👍

@davehewy
Copy link

Raised #182 today 👍

@david-heward-unmind
Copy link
Author

@petoju I'm just doing some validation this morning. Then we can close this issue off.

@davehewy
Copy link

@petoju I just opened #183 which slightly refactors my previous modification to cert pool behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants