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

server: accept TLS certificate and key #2398

Merged
merged 4 commits into from
Jun 28, 2022
Merged

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Jun 24, 2022

Summary

Allow a user to specify the certificate and key to use for TLS.

Also fixes a couple bugs:

  • the CA we were generating was not a valid CA certificate
  • there was a bug in a cliopts hook for using a Set(string) error function to parse a config field from a string

Related Issues

This is part 1 of #2176

dnephin added 3 commits June 24, 2022 12:27
Fixes a bug in the cliopts hook for FromString. It was incorrectly returning the reflect.Value
instead of the underlying value.
BasicConstraintsValid: true,
}

caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096)
caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we were only creating the x509.Certificate manually, which does not give us any way to serialize it (as far as I can tell).

By calling CreateCertificate it creates the asn.1 DER bytes we need to serialize the certificate. When doing this I ran into an error which I believe is because we were not setting IsCA above.

I left a TODO below about this, but I could not find any other way to allow us to serialize this CA.

internal/server/server.go Outdated Show resolved Hide resolved
// certificate, or that will be used to generate a certificate if one was
// not provided.
CA types.StringOrFile
CAPrivateKey string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will the CAPrivateKey here be used for? It's only in a test at the moment and frequently people won't have access to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, it gets used in #2401 which is the follow up to this

Co-authored-by: Bruce MacDonald <[email protected]>
@dnephin dnephin merged commit 512430a into main Jun 28, 2022
@dnephin dnephin deleted the dnephin/server-tls-config branch June 28, 2022 18:01
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

Successfully merging this pull request may close these issues.

2 participants