Skip to content

Conversation

@cretz
Copy link
Contributor

@cretz cretz commented Jul 11, 2018

This is to address issue #42. This uses the win32 crypt API which is what Go uses elsewhere when interfacing with the OS cert store. Also, though there is a "certutil.exe" in Windows, it's not the NSS one and therefore while I reference the FF paths, I left hasCertutil as false for this OS in cert.go.

I took care to make the smallest, self-contained changes I could here for minimal Windows support. However as I mention in #45, this code could benefit from a lot of refactoring and "library-ification". I don't like panicking on error, returning bools, hardcoding CA org name, etc.

@cretz
Copy link
Contributor Author

cretz commented Jul 11, 2018

By the way, this pops up dialogs. On install:

image

On uninstall:

image

@cretz cretz mentioned this pull request Jul 11, 2018
fatalIfErr(err, "open root store")
defer store.close()
// Do the deletion
deletedAny, err := store.deleteCertsFromOrg("mkcert development CA")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this just be pulled from the certificate?

https://github.com/FiloSottile/mkcert/blob/master/cert.go#L53

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I was just trying to avoid file access + parsing and that value is not in a property and when doing something like adding just a new OS to an existing codebase, I try to keep the alterations confined and leave refactorings (such as sharing this string) elsewhere. However, if necessary I can either extract this into a const or on uninstall I can read from file, parse the cert, and take the org from there. Pick which and I'll do it.

Copy link
Owner

Choose a reason for hiding this comment

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

This is what caUniqueName is for, and the CA is always loaded from disk. Removing all mkcert certificates is not ok because there might be multiple CAROOTs used at the same time.

Copy link
Contributor Author

@cretz cretz Jul 13, 2018

Choose a reason for hiding this comment

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

@FiloSottile - Can you help me understand the ideal approach here? Are you saying on install I should parse the cert from the file and put the result of caUniqueName somewhere like NSS? Or can I use serialNumber when enumerating the certs to find which one to delete?

Copy link
Owner

Choose a reason for hiding this comment

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

My bad, you don't need caUniqueName, you can use the serial number:

parsedCert.SerialNumber.Cmp(m.caCert.SerialNumber) == 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return deletedAny, fmt.Errorf("Failed enumerating certs: %v", err)
}
// Parse cert
certBytes := (*[1 << 20]byte)(unsafe.Pointer(cert.EncodedCert))[:cert.Length]
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unfortunate part of this is that it requires import "C" which then requires gcc on the PATH for Windows builds. The logic I used came from https://github.com/golang/go/blob/22e17d0ac7db5321a0f6e073bd0afb949f44dd70/src/crypto/x509/root_windows.go#L70. However, if you still want me to use cgo in this case, I will.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FiloSottile - Bump

Choose a reason for hiding this comment

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

I can confirm this is working for me.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, right, didn't think about avoiding cgo. A bit scary, but this will do.

@nickkaczmarek
Copy link

Really excited for this update. This seems like an awesome tool and unfortunately I work in a windows environment.

@yanlinaung30
Copy link

Thanks guys! Could you update README for using in Windows?

@arsamsarabi
Copy link

Would love this update as most of our devs work on windows systems 😄

@cretz
Copy link
Contributor Author

cretz commented Jul 26, 2018

@nickkaczmarek, @yanlinaung30, @arsamsarabi, et al...you can simply git clone + go build the windows-support branch of https://github.com/cretz/mkcert while waiting for this to merge.

@Geczy
Copy link

Geczy commented Jul 30, 2018

Tried this but the cert appears invalid
image

This page is not secure (broken HTTPS).
Certificate - missing
This site is missing a valid, trusted certificate (net::ERR_CERT_AUTHORITY_INVALID).
View certificate

@cretz
Copy link
Contributor Author

cretz commented Jul 30, 2018

@Geczy - What browser? I am a bit swamped atm, but will revisit this tomorrow.

@Geczy
Copy link

Geczy commented Jul 30, 2018

Chrome Version 68.0.3440.75 (Official Build) (64-bit)

@cretz
Copy link
Contributor Author

cretz commented Jul 31, 2018

@Geczy - This worked just fine for me.

Expand for details

First, I installed the cert:

mkcert -install

Which gave me a dialog which I accepted:
image

And outputted:

Using the local CA at "C:\Users\chadr\AppData\Local\mkcert" ✨
The local CA is now installed in the system trust store! ⚡️
Warning: "certutil" is not available, so the CA can't be automatically installed in Firefox! ⚠️
Install "certutil" with "<certutil unsupported on Windows>" and re-run "mkcert -install" 

Then, I ran mkcert to generate a localhost cert:

mkcert localhost

Success:

Using the local CA at "C:\Users\chadr\AppData\Local\mkcert" ✨
Warning: the local CA is not installed in the Firefox trust store! ⚠️
Run "mkcert -install" to avoid verification errors ‼️

Created a new certificate valid for the following names 📜
- "localhost"

The certificate is at "./localhost.pem" and the key at "./localhost-key.pem" ✅

Then I wrote the following Go file:

package main

import (
	"log"
	"net/http"
)

func main() {
	http.HandleFunc("/hello", func(w http.ResponseWriter, req *http.Request) {
		w.Header().Set("Content-Type", "text/plain")
		w.Write([]byte("Hello, World!"))
	})
	if err := http.ListenAndServeTLS(":443", "localhost.pem", "localhost-key.pem", nil); err != nil {
		log.Fatal(err)
	}
}

Then built it, ran it, and visited https://localhost/hello and it succeeded:
image

The cert:
image

The chain:
image

@Geczy
Copy link

Geczy commented Jul 31, 2018

Tried this again

Matt@Matt MINGW64 ~/mkcert (windows-support)
$ ./mkcert install
Using the local CA at "C:\Users\Matt\AppData\Local\mkcert" ✨
Warning: the local CA is not installed in the system trust store! ⚠️
Warning: the local CA is not installed in the Firefox trust store! ⚠️
Run "mkcert -install" to avoid verification errors ‼️

Created a new certificate valid for the following names 📜
 - "install"

The certificate is at "./install.pem" and the key at "./install-key.pem" ✅

I don't get that popup that you do, any idea why?

@cretz
Copy link
Contributor Author

cretz commented Jul 31, 2018

You typed mkcert install but you should type mkcert -install. Also, no need to be in MINGW, do this from cmd or powershell. If you run into other issues, can you open an issue on https://github.com/cretz/mkcert so support can be done there instead of in this PR?

@Geczy
Copy link

Geczy commented Jul 31, 2018

Thanks, works flawlessly

@FiloSottile FiloSottile merged commit 561c998 into FiloSottile:master Aug 13, 2018
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.

8 participants