Skip to content

Add support for client certificates#89

Closed
jtdowney wants to merge 1 commit intoFiloSottile:masterfrom
jtdowney:client-certificate-support
Closed

Add support for client certificates#89
jtdowney wants to merge 1 commit intoFiloSottile:masterfrom
jtdowney:client-certificate-support

Conversation

@jtdowney
Copy link
Copy Markdown
Contributor

@jtdowney jtdowney commented Nov 2, 2018

This change adds a new flag for client certificates. When this flag is passed, the extended key usage for the generated certificate is for client authentication instead of server authentication.

I am not 100% sure this is a good idea, but I found myself needing to generate both server and client certificates for a project and thought that mkcert could fit both of those cases.

The change was remarkably easy to make so I figured I'd submit a PR.

This change adds a new flag for client certificates. When this flag is
passed, the extended key usage for the generated certificate is for
client authentication instead of server authentication.
Comment thread cert.go

var extKeyUsage []x509.ExtKeyUsage
if m.client {
extKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just add ClientAuth as a usage if the flag is set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In my case, I wanted separate certificates. If the certificate is meant for client auth, then it doesn't need the server auth extension. To me, it would seem weird that a client certificate could be used for server authentication as well.

@bombsimon
Copy link
Copy Markdown

Hi! I would like to see this kind of change as well. Will this this PR be merged or does it break your thoughts about simplicity?

Personally I don't mind adding the extended key usage for both client and server if that's more in line with your thoughts. This is the only diff I needed to fix my issues:

diff --git a/cert.go b/cert.go
index 13ed35f..8c40aa8 100644
--- a/cert.go
+++ b/cert.go
@@ -62,7 +62,7 @@ func (m *mkcert) makeCert(hosts []string) {
                NotBefore: time.Now(),

                KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-               ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+               ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
                BasicConstraintsValid: true,
        }
        for _, h := range hosts {

@FiloSottile
Copy link
Copy Markdown
Owner

Rebased and merged, thank you!

@erikh
Copy link
Copy Markdown

erikh commented Feb 2, 2019 via email

@jtdowney jtdowney deleted the client-certificate-support branch December 17, 2019 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants