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

Convos::Core::Connection will generate custom certs for tls connections #525

Closed
wants to merge 2 commits into from

Conversation

jhthorsen
Copy link
Collaborator

This PR will generate custom certificates for each connection. I'm pretty sure this is required for #356 and #380.

@jhthorsen jhthorsen added this to the 5.xx milestone Nov 21, 2020
@jhthorsen jhthorsen self-assigned this Nov 21, 2020
@marcusramberg marcusramberg self-requested a review November 21, 2020 07:48
Copy link
Contributor

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

Some thoughts:

Do we need a separate CA for this purpose?

And, _openssl_detect reads openssl.cnf/%ENV and set misc certificate parameters, this could be a bit overkill in the case of Freenode. They say: "You will be prompted for various pieces of information about the certificate. The contents do not matter for our purposes, but openssl needs at least one of them to be non-empty."

Creating a self signed certificate using OpenSSL can be accomplished with an openssl oneliner:

openssl req -new -sha256 -newkey rsa:4096 -days 3650 -nodes -x509 -subj "/CN=$nick/O=Convos" -keyout self_signed_client.key -out self_signed_client.crt

If rsa is hard coded, we should default the key size to 3072 or 4096 bits. https://www.keylength.com/

Just to be sure, is this the correct flow we're going for?

  1. Generate a selfsigned cert for the connection
  2. Connect to the irc server with the client certificate
  3. Log in to NickServ
  4. Do /msg NickServ CERT ADD to pin the self signed cert

@jhthorsen
Copy link
Collaborator Author

jhthorsen commented Nov 21, 2020

Do we need a separate CA for this purpose?

I don't know to be honest. I hope not, since I think that would make the process a lot more difficult.

And, _openssl_detect reads openssl.cnf/%ENV and set misc certificate parameters, this could be a bit overkill in the case of Freenode. They say: "You will be prompted for various pieces of information about the certificate. The contents do not matter for our purposes, but openssl needs at least one of them to be non-empty."

Ah, I see. That makes the code a lot simpler 👍

If rsa is hard coded, we should default the key size to 3072 or 4096 bits. https://www.keylength.com/

Let's go for 4096.

Just to be sure, is this the correct flow we're going for?

I think that is the flow for #380. I haven't been able to try out #356. There's some buggy code in https://github.com/Nordaaker/convos/tree/jhthorsen/sasl-certp, but I was not able to connect with SASL.

@jhthorsen jhthorsen force-pushed the jhthorsen/tls-connect branch from 4446030 to 585061d Compare November 21, 2020 12:46
jhthorsen pushed a commit that referenced this pull request Dec 29, 2020
@jhthorsen
Copy link
Collaborator Author

I think I've done the changes @stigtsp suggested now in a8533ef.

Thanks for the input!

@jhthorsen jhthorsen closed this Dec 29, 2020
@jhthorsen jhthorsen deleted the jhthorsen/tls-connect branch December 29, 2020 04:38
@stigtsp
Copy link
Contributor

stigtsp commented Dec 29, 2020

Cool! 👍 Sorry for not being able to contribute with code for this PR :-/

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

Successfully merging this pull request may close these issues.

2 participants