Adds support for sending a client certificate #44

sloum · 2019-10-03T02:47:07Z

sloum commented

2019-10-03 02:47:07 +00:00

This update adds support for gemini status 6. It is a minimal but reasonably simple approach:

User generates a certificate and key file on their own. Docs should eventually get updated to provide suggestions to users. I used the following command to generate mine: openssl req -x509 -newkey rsa:4096 -keyout key.pem -nodes -out cert.pem -days 365.
User updates the config options tlscertificate and tlskey to be the absolute path to those files.
It just works. Yay!

Notes:

The certificate that gets generated from the two input files should only get sent in the event that a server asks for the certificate. Otherwise it is not sent along with the request. It took awhile to figure out how this option worked, but I am glad I got it working.
If SET is called on either tlscertificate or tlskey Bombadillo will try to regenerate the certificate. If it fails to do so (due to invalid input), the user will not be notified. Instead, they will see the message [6] Client certificate required when they try to go to a page that requires a certificate. If we would really like the user to have a way to validate that their certificate is set up correctly I can add a command (validate or some such) to report back that the certificate is valid or not. Let me know if you think this is necessary.

This update adds support for gemini status 6. It is a minimal but reasonably simple approach: 1. User generates a certificate and key file on their own. Docs should eventually get updated to provide suggestions to users. I used the following command to generate mine: `openssl req -x509 -newkey rsa:4096 -keyout key.pem -nodes -out cert.pem -days 365`. 2. User updates the config options `tlscertificate` and `tlskey` to be the absolute path to those files. 3. It just works. Yay! Notes: - The certificate that gets generated from the two input files should only get sent in the event that a server _asks_ for the certificate. Otherwise it is not sent along with the request. It took awhile to figure out how this option worked, but I am glad I got it working. - If SET is called on either `tlscertificate` or `tlskey` Bombadillo will try to regenerate the certificate. If it fails to do so (due to invalid input), the user will not be notified. Instead, they will see the message `[6] Client certificate required` when they try to go to a page that requires a certificate. If we would really like the user to have a way to validate that their certificate is set up correctly I can add a command (`validate` or some such) to report back that the certificate is valid or not. Let me know if you think this is necessary.

sloum added the

enhancement

label 2019-10-03 02:47:07 +00:00

sloum reviewed 2019-10-03 02:54:53 +00:00

sloum left a comment

One other thing to note is that Bombadillo does not make and store the certificate itself. It makes it from files ont he user's system each time it is loaded. As a result we do not have to hang on to certificate information or come up with a good way to store them.

client.go

						
				@ -405,1 +405,4 @@

							c.Options[values[0]] = lowerCaseOpt(values[0], val)

							if values[0] == "tlskey" || values[0] == "tlscertificate" {

								c.Certs.LoadCertificate(c.Options["tlscertificate"], c.Options["tlskey"])

							}

sloum commented

2019-10-03 02:47:52 +00:00

This makes it so that if the option values are changed we try to regenerate the certificate with the new information.

gemini/gemini.go

						
				@ -33,0 +34,4 @@

				func (t *TofuDigest) LoadCertificate(cert, key string) {

					certificate, err := tls.LoadX509KeyPair(cert, key)

					if err != nil {

						t.ClientCert = tls.Certificate{}

sloum commented

2019-10-03 02:49:21 +00:00

If the attempt at making the certificate fails, set the certificate to an empty certificate.

gemini/gemini.go

						
				@ -144,6 +154,10 @@ func Retrieve(host, port, resource string, td *TofuDigest) (string, error) {

						InsecureSkipVerify: true,

					}

					conf.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {

sloum commented

2019-10-03 02:51:23 +00:00

This was the hardest bit to get figured out. It ended up being so simple. Using conf.Certificates, which is a []tls.Certificate, would have sent the certificate with every request. Using conf.GetClientCertificate` ensures that a client certificate is only sent when one is asked for and it is done as part of the tls request rather than as secondary logic that we need to set up manually.

This was the hardest bit to get figured out. It ended up being so simple. Using `conf.Certificates`, which is a `[]`tls.Certificate`, would have sent the certificate with every request. Using `conf.GetClientCertificate` ensures that a client certificate is only sent when one is asked for and it is done as part of the tls request rather than as secondary logic that we need to set up manually.

go.mod

						
				@ -1,3 +1,5 @@

				module tildegit.org/sloum/bombadillo

				go 1.10

				go 1.12

sloum commented

2019-10-03 02:53:23 +00:00

Using mailcap pushes us to needing 1.12 so I updated the go.mod here. However, I was talking to...Julien (I think it was Julien) about their client Asuka, which they are building in Rust and they are not using mailcap. They went a different way that takes into account each OSs natural way of opening an arbitrary file (think xdg-open). This may be something to look into. It would let us cut the one and only dependency loose.

main.go

						
				@ -139,2 +139,4 @@

					cui.SetCharMode()

					err := loadConfig()

					if bombadillo.Options["tlscertificate"] != "" && bombadillo.Options["tlskey"] != "" {

						bombadillo.Certs.LoadCertificate(bombadillo.Options["tlscertificate"], bombadillo.Options["tlskey"])

sloum commented

2019-10-03 02:53:59 +00:00

This just initializes Bombadillo, on load, with a certificate if the makings of one exist.

sloum reviewed 2019-10-03 02:54:54 +00:00

sloum left a comment

One other thing to note is that Bombadillo does not make and store the certificate itself. It makes it from files ont he user's system each time it is loaded. As a result we do not have to hang on to certificate information or come up with a good way to store them.

asdf was assigned by sloum

2019-10-03 02:55:00 +00:00

asdf commented

2019-10-03 21:54:52 +00:00

This is cool! A few initial things:

I take your point about bombadillo not making or storing the certificate, but I don't know what the key generation really does, or where and how the cert and key should be stored. The documentation could be like the ssh keys section of this document that covers nearly everything.
The only site I know that can use this is gemini.conman.org and suspect that any issues could be server issues too. I was able to get in to the private area successfully, and read the welcome notice, but was also able to get in to second private area...Files within those areas return unexpected EOF.

I'll spend some more time on this later today.

This is cool! A few initial things: 1. I take your point about bombadillo not making or storing the certificate, but I don't know what the key generation really does, or where and how the cert and key should be stored. The documentation could be like [the ssh keys section of this document](https://help.ubuntu.com/lts/serverguide/openssh-server.html) that covers nearly everything. 1. The only site I know that can use this is gemini.conman.org and suspect that any issues could be server issues too. I was able to get in to the private area successfully, and read the welcome notice, but was also able to get in to second private area...Files within those areas return unexpected EOF. I'll spend some more time on this later today.

sloum commented

2019-10-05 20:24:09 +00:00

Yeah, solid docs will likely need to be made to handle this. Part of the problem is that this is not really an area of expertise for me either. I'm not sure what all is happening when I run the openssl command that I listed above. I know that it works, but that is about it. It may be good to find someone that knows about this and see if they can be convinced to write a little bit about generating and storing keys for this kind of purpose. Or at the very least Bombadillo should provide links to outside resources (as keygen isnt really a part of the client). I know that it works, but that is about it.
I emailed Sean about this issue. He was unaware that the second "more secure" section was accessible with just any certificate. He has fixed his code and I have verified that I can now get into the regular private area but not the one that you need a specific cert for. As for the files, that seems to be something on his end that he is aware of (though he expressed surprised that it was still an issue).

1. Yeah, solid docs will likely need to be made to handle this. Part of the problem is that this is not really an area of expertise for me either. I'm not sure what all is happening when I run the openssl command that I listed above. I know that it works, but that is about it. It may be good to find someone that knows about this and see if they can be convinced to write a little bit about generating and storing keys for this kind of purpose. Or at the very least Bombadillo should provide links to outside resources (as keygen isnt really a part of the client). I know that it works, but that is about it. 2. I emailed Sean about this issue. He was unaware that the second "more secure" section was accessible with just any certificate. He has fixed his code and I have verified that I can now get into the regular private area but not the one that you need a specific cert for. As for the files, that seems to be something on his end that he is aware of (though he expressed surprised that it was still an issue).

asdf commented

2019-10-08 01:41:10 +00:00

Which spec are you reviewing for this implementation? I'm looking at gemini://zaibatsu.circumlunar.space:1965/spec-spec.txt in the section Transient client certificate sessions. It seems more like a replacement for cookies than a single cert for the client?

Just on the mailcap library, I tried to comment on the review but it wasn't processing the request. So here's the comment:
With a quick search I can't really see too much saying which way might be better. It looks like it would be a change in dependency (like to https://github.com/kyoh86/xdg) but maybe it's lighter and maybe it brings increased platform support? Plus, it would be good to keep compatibility with the version of Go commonly available in popular distros.
Looks like a fairly deep topic that should be a separate issue.

Which spec are you reviewing for this implementation? I'm looking at gemini://zaibatsu.circumlunar.space:1965/spec-spec.txt in the section Transient client certificate sessions. It seems more like a replacement for cookies than a single cert for the client? Just on the mailcap library, I tried to comment on the review but it wasn't processing the request. So here's the comment: With a quick search I can't really see too much saying which way might be better. It looks like it would be a change in dependency (like to https://github.com/kyoh86/xdg) but maybe it's lighter and maybe it brings increased platform support? Plus, it would be good to keep compatibility with the version of Go commonly available in popular distros. Looks like a fairly deep topic that should be a separate issue.

sloum commented

2019-10-09 02:29:43 +00:00

I had viewed that doc awhile back. Most of my implementation comes from conversations with Solderpunk and others ont he mailing list. This implementation does not attempt to solve the transient certificate issue. This system is designed for swapable long term use certificates. I would like to add transient certificates as well, but have not found a reasonably decent way to generate them nor a real world use case for them.

As described by the community, the idea of using client certificates is to be semi-identifying so-as to access information associated with that certificate. Enabling interaction at a greater level. There is definitely a disconnect between that concept and the concept presented in the doc. I will reach out to Solderpunk and see what they think is the best approach.

Should we hold on this PR for the time being? Or move forward with this concept and add transient certs (and messaging to allow a user to choose between a long term cert if present and a transient cert) at a later point?

Agreed re: the outside library stuff. I'll take a look at Julien's code for Asuka and see how he is handling opening files (he mentioned he was doing so).

I had viewed that doc awhile back. Most of my implementation comes from conversations with Solderpunk and others ont he mailing list. This implementation _does not_ attempt to solve the transient certificate issue. This system is designed for swapable long term use certificates. I would like to add transient certificates as well, but have not found a reasonably decent way to generate them nor a real world use case for them. As described by the community, the idea of using client certificates is to be semi-identifying so-as to access information associated with that certificate. Enabling interaction at a greater level. There is definitely a disconnect between that concept and the concept presented in the doc. I will reach out to Solderpunk and see what they think is the best approach. Should we hold on this PR for the time being? Or move forward with this concept and add transient certs (and messaging to allow a user to choose between a long term cert if present and a transient cert) at a later point? Agreed re: the outside library stuff. I'll take a look at Julien's code for Asuka and see how he is handling opening files (he mentioned he was doing so).

asdf commented

2019-10-09 08:36:24 +00:00

That is really interesting, I'm not on the mailing list so it explains the difference.

This function actually does something (loading that page on conman.org), so I recommend merging it in. Further changes can be made once there is clarification on the spec.

That is really interesting, I'm not on the mailing list so it explains the difference. This function actually does something (loading that page on conman.org), so I recommend merging it in. Further changes can be made once there is clarification on the spec.

sloum commented

2019-10-09 15:45:43 +00:00

Agreed. I will do so now and we can update as things become more clear.

sloum closed this pull request

2019-10-09 15:45:47 +00:00

Sign in to join this conversation.

No reviewers