[Dovecot] ssl-proxy: client certificates and crl check
Hi, I've attached a new version of my patch against ssl_proxy-openssl.c which: - disconnects when no clientcertificate is presented - checks the clientcertificate against the crl for our root cert. (so you can't use a revoked client cert.) - returns the CommonName from the client cert. in ssl_proxy_get_peer_name (this way it's easier to use dovecot as imap-proxy with a passwd-like userdb + ssl_require_client_cert + ssl_username_from_cert, it "binds" the emailuser to the clientcertificate, so a user / clientcert. can access only the account from the userdb) In order to use it, the CAfile must be a file which contains the CAcertificate (pem format) followed by the CRL (also in pem format). (servercert and the clientcerts are signed with a self-signed rootcert) changes in this version: - found the proper way to extract the CommonName from a certificate! - code cleanup there are some issues with the patch: - it needs openssl > 0.9.7. the way I do CRL loading/checking is new in 0.9.7. There are some examples on the openssl-devel list, but not much documentation. - ssl_verify_client_cert now returns 0 in case of an invalid cert. was there a reason why it always returned 1? - ssl_proxy_get_peer_name is changed, code depending on this function returning "X509_NAME_oneline" can break.. (at the moment it is only used for the ssl_username_from_cert config option, i think) I'd like comments / ideas etc. -- groeten, HenkJan Wolthuis --- ssl-proxy-openssl.c.orig 2006-04-04 10:32:58.000000000 +0200 +++ ssl-proxy-openssl.c 2006-06-01 09:24:57.000000000 +0200 @@ -498,7 +498,7 @@ const char *ssl_proxy_get_peer_name(struct ssl_proxy *proxy) { X509 *x509; - char buf[1024]; + char buf[256]; const char *name; if (!ssl_proxy_has_valid_client_cert(proxy)) @@ -508,10 +508,16 @@ if (x509 == NULL) return NULL; /* we should have had it.. */ - X509_NAME_oneline(X509_get_subject_name(x509), buf, sizeof(buf)); - name = t_strndup(buf, sizeof(buf)); + /* HJHJ */ + /* the X509_N_gtbN can return -1 without 0-terminating buf */ + /* if the call succeeds buf is 0-terminated (openssl 0.9.7e / 0.9.8b src) */ + buf[0] = '\0'; + if( X509_NAME_get_text_by_NID(X509_get_subject_name(x509),NID_commonName,buf,sizeof(buf)) < 0 ) + { buf[0] = '\0'; } + name = t_strndup(buf, sizeof(buf) ); X509_free(x509); - + /* HJHJ */ + return *name == '\0' ? NULL : name; } @@ -582,10 +588,22 @@ proxy = SSL_get_ex_data(ssl, extdata_index); proxy->cert_received = TRUE; + + /* HJHJ */ + char buf[1024]; + X509_NAME_oneline( X509_get_subject_name(ctx->current_cert),buf,sizeof(buf)); + if (!preverify_ok) + { proxy->cert_broken = TRUE; + i_warning("BAD CERT %s: %s",X509_verify_cert_error_string(ctx->error),buf); + } + else + { i_warning("CERT: %s",buf); } /* logging */ - return 1; + return preverify_ok; + /* HJHJ */ + } static int @@ -666,10 +684,20 @@ if (getenv("SSL_VERIFY_CLIENT_CERT") != NULL) { SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER | - SSL_VERIFY_CLIENT_ONCE, + SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_client_cert); } + /* HJHJ */ +#if OPENSSL_VERSION_NUMBER >= 0x00907000L + X509_STORE *store; + if( (store=SSL_CTX_get_cert_store(ssl_ctx)) != NULL ) + { X509_STORE_set_flags( store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); } + else + { i_warning("X509 get cert store failed..."); } +#endif + /* HJHJ */ + /* PRNG initialization might want to use /dev/urandom, make sure it does it before chrooting. We might not have enough entropy at the first try, so this function may fail. It's still been
On Thu, 2006-06-01 at 10:13 +0200, HenkJan Wolthuis wrote:
Hi,
I've attached a new version of my patch against ssl_proxy-openssl.c which:
Thanks, committed to CVS now although with some changes.
- ssl_verify_client_cert now returns 0 in case of an invalid cert. was there a reason why it always returned 1?
Yes. ssl_verify_client_cert=yes doesn't require the certificate to be valid. Only ssl_require_valid_client_cert=yes in auth settings does that. This allows for some people to authenticate with certificates and others to authenticate the usual way. So I dropped this part of your patch.
- if( (store=SSL_CTX_get_cert_store(ssl_ctx)) != NULL )
{ X509_STORE_set_flags( store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); }
- else
{ i_warning("X509 get cert store failed..."); }
Can it ever return NULL? Looking at the manual page it didn't seem so, so I dropped the NULL-check from here.
Hi Timo,
Thanks, committed to CVS now although with some changes.
Nice!
- ssl_verify_client_cert now returns 0 in case of an invalid cert. was there a reason why it always returned 1?
Yes. ssl_verify_client_cert=yes doesn't require the certificate to be valid. Only ssl_require_valid_client_cert=yes in auth settings does that. This allows for some people to authenticate with certificates and others to authenticate the usual way. So I dropped this part of your patch.
OK, you also changed SSL_VERIFY_FAIL_IF_NO_PEER_CERT back to SSL_VERIFY_CLIENT_ONCE, same reason? Maybe the valid-client-cert-feature can have a conf.file switch, or a #define in the sourcecode, what's your opinion?
- if( (store=SSL_CTX_get_cert_store(ssl_ctx)) != NULL )
Can it ever return NULL? Looking at the manual page it didn't seem so, so I dropped the NULL-check from here.
No, SSL_CTX_new() returns NULL (1) if the "store" can't be malloc'ed, ssl-proxy-openssl.c checks this and fails with i_fatal(). So the NULL-check can be dropped. (1: openssl 0.9.7d source)
--
groeten,
HenkJan Wolthuis
On Mon, 2006-06-12 at 12:30 +0200, HenkJan Wolthuis wrote:
- ssl_verify_client_cert now returns 0 in case of an invalid cert. was there a reason why it always returned 1?
Yes. ssl_verify_client_cert=yes doesn't require the certificate to be valid. Only ssl_require_valid_client_cert=yes in auth settings does that. This allows for some people to authenticate with certificates and others to authenticate the usual way. So I dropped this part of your patch.
OK, you also changed SSL_VERIFY_FAIL_IF_NO_PEER_CERT back to SSL_VERIFY_CLIENT_ONCE, same reason?
Yes. Or if it's FAIL_IF_NO_PEER_CERT and the cert is invalid, what happens? Does it disconnect immediately? I haven't tried.
Maybe the valid-client-cert-feature can have a conf.file switch, or a #define in the sourcecode, what's your opinion?
Well, at least I want to avoid adding more options to config file.. Why do you think it's so much better to disconnect immediately? Do clients then give good error messages if that happens?
One possibility would be to send also the ssl_require_valid_client_cert setting to the login process, and disconnect immediately if that's yes. One problem with that is however that it's possible to have multiple auth blocks with different ssl_require_valid_client_cert values, so the code would have to check that all of them have it.
Hi Timo,
Yes. Or if it's FAIL_IF_NO_PEER_CERT and the cert is invalid, what happens? Does it disconnect immediately? I haven't tried.
if ssl_verify_client_cert() in ssl-proxy-openssl.c return 0 the connection is immediately dropped, if it returns 1 the error (no client cert, cert revoked, crl expired etc.) is ignored. But I haven't experimented much with it, in particular, i'm not certain if it disconnects with SSL_VERIFY_CLIENT_ONCE and no peer certificate, i think it should, but i haven't tested it... (i'll test it tonight)
Maybe the valid-client-cert-feature can have a conf.file switch, or a #define in the sourcecode, what's your opinion?
Well, at least I want to avoid adding more options to config file.. Why do you think it's so much better to disconnect immediately? Do clients then give good error messages if that happens?
The main reason is that I thought it would be better to drop an unwanted connection as soon as possible...
Clients should receive errors like "certificate revoked", but I'll try generating some errors and see what really happens...
One possibility would be to send also the ssl_require_valid_client_cert setting to the login process, and disconnect immediately if that's yes. One problem with that is however that it's possible to have multiple auth blocks with different ssl_require_valid_client_cert values, so the code would have to check that all of them have it.
Another option is to leave it the way it is, and place a small comment in the sourcecode (or Wiki) which explains the other behaviour. ;-)
--
groeten,
HenkJan Wolthuis
Hi Timo,
Well, at least I want to avoid adding more options to config file.. Why do you think it's so much better to disconnect immediately? Do clients then give good error messages if that happens?
Tested with thunderbird 1.0.2 and a revoked user certificate, on connect I got the following results:
cvs-nightly-20060613 asks for a password, returns "login to server localhost failed" and asks for the password again.
modified cvs-nightly-20060613 (ssl_verify_client_cert() returning 'preverify_ok' instead of '1') returns "could not establish an encrypted connection with localhost because your certificate has been revoked" , then disconnects. The error messages on the client side are more useful in this case. (imho).....
One possibility would be to send also the ssl_require_valid_client_cert setting to the login process, and disconnect immediately if that's yes. ok....
One problem with that is however that it's possible to have multiple auth blocks with different ssl_require_valid_client_cert values, so the code would have to check that all of them have it.
I'm afraid I don't understand... In the config-file there's only "auth default {}" The wikipage MultipleAuth doesn't seem related to this. Can you explain?
PS: I also modified the i_info call in ssl_verify_client_cert() to:
i_info('"Invalid certificate: %s %s",
X509_verify_cert_error_string(ctx->error),buf);
This way the verification error is also logged.
--
groeten,
HenkJan Wolthuis
participants (2)
-
HenkJan Wolthuis
-
Timo Sirainen