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

Warn if the client certificate is expired #211

Closed
mineo opened this Issue Jan 27, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@mineo
Copy link
Contributor

commented Jan 27, 2015

At the moment, using an expired client certificate to connect to an IRC server doesn't use the certificate (without a warning or anything) or uses it and the server ignores it. I've tested this on Rizon and I'm not sure which of the two it actually is, but I don't speak much C and OpenSSLs API, so I won't even try to make wild guesses :-) The only thing I've been able to figure out is that none of the checks in

if (! SSL_CTX_use_certificate_file(ctx, scert, SSL_FILETYPE_PEM))
g_warning("Loading of client certificate '%s' failed: %s", mycert, ERR_reason_error_string(ERR_get_error()));
else if (! SSL_CTX_use_PrivateKey_file(ctx, spkey ? spkey : scert, SSL_FILETYPE_PEM))
g_warning("Loading of private key '%s' failed: %s", mypkey ? mypkey : mycert, ERR_reason_error_string(ERR_get_error()));
else if (! SSL_CTX_check_private_key(ctx))
g_warning("Private key does not match the certificate");
error out. It would be great if irssi would at least show a warning in this case or didn't try to establish a connection to the server at all.

@ahf ahf added this to the 0.8.18 milestone Apr 8, 2015

@ahf ahf added the enhancement label Apr 8, 2015

@dequis

This comment has been minimized.

Copy link
Member

commented Sep 10, 2015

Looked a bit into this, tried to find other openssl clients that used client certificates... and none seemed to verify the client certificate in any way. You probably don't want to do the default verification, anyway, since it's self-signed, and that makes it invalid.

$ openssl verify mynick.cert
mynick.cert: C = AU, ST = Some-State, O = Internet Widgits Pty Ltd
error 18 at 0 depth lookup:self signed certificate
C = AU, ST = Some-State, O = Internet Widgits Pty Ltd
error 10 at 0 depth lookup:certificate has expired
OK

I guess the correct solution is to load the certificate using the X509 openssl functions and manually check for expiration and nothing else.

Also, if it helps anyone, to get an expired cert, do this (set days to -1)

openssl req -nodes -newkey rsa:2048 -days -1 -x509 -keyout mynick.key -out mynick.cert
@dequis

This comment has been minimized.

Copy link
Member

commented Oct 21, 2016

In #560 @Olathe provides some additional reasons to consider this:

SASL EXTERNAL uses a certificate file to prove who I am to the IRC server. Unfortunately, certificate files can expire. When this happens, the only thing I see when connecting is an uninformative message from the server that SASL failed with no reason why.

Because this happens out of the blue after SASL has been working properly for weeks or months, it can be quite difficult or even impossible to identify the source of the problem. It would be nice if irssi gave an error message if the certificate file had expired.

@ailin-nemui ailin-nemui modified the milestones: 0.8.21, 1.1.1 Jan 3, 2017

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jan 19, 2017

@ahf any updates here?

@ahf

This comment has been minimized.

Copy link
Member

commented Jan 19, 2017

Nope. Not something I've looked into as of now.

LemonBoy added a commit to LemonBoy/irssi that referenced this issue Jan 22, 2017

Check whether the client certificate is expired.
Right now we only warn the user, the connection keeps going.
Fixes irssi#211

LemonBoy added a commit to LemonBoy/irssi that referenced this issue Jan 22, 2017

Check whether the client certificate is expired.
Right now we only warn the user, the connection keeps going.
Fixes irssi#211
@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2017

@mineo are you interested in testing this

@ailin-nemui ailin-nemui removed this from the 1.1.1 milestone Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.