(Neon people, i'm not on the ml, but i figured you'd want to read
this, since it contains a note about an openssl bug that should be worked
around. But if you respond, please include me on the cc. The main neon
version i'm using is 0.21.3, though i tested it with cvs over the time i
composed the message)
First, if i use the neon callbacks, rather than preload the
certificate, we can't get good error handling, because we can't return
errors from the provide_ccert callback (it's a void return).
Thus, rather than get something sensible when you mistype the password, by
the time we get ahold of an error, it's a connection closed from the
server. Which we take to be some other type of error, and eventually get
a fun:
[root_at_dberlin myrepo]# svn co https://localhost/repos
Enter PEM pass phrase:
Enter PEM pass phrase:
svn_error: #21095 : <Bad URL passed to RA layer>
No part of path '/repos' was found in repository
Cute, no?
we get two chances only because the provide functions exist for both neon
sessions we open, and it only makes one attempt.
If you get it right, it only asks once (I cache the password in the
password providing function).
Without the callbacks (IE preloading the certificates), i can give
sensible error messages, but it would always asks twice (one for each
session), so i had to cache the password in the ra_session structure to get around this.
Which should i keep in the patch?
The first, the second, or both (possibly with the callback setting
commented out, though it's not necessary, since neon notices it has a
cert already if you preload, and thus, never calls the provide function)?
(Side note:It's not a maintenance issue to keep both, the callback just calls the
get_cert routine, and ignores the return value, since it can't do anything with
it. So the difference is literally 6 lines of code, 4 for a one liner
function, and 2 to set the provide_ccert callback)
But just when you thought my troubles were over, guess what:
The PEM reading functions don't use the callback you provide for password
getting, only the PKCS12 reading ones do.
This is an openssl bug. Neon passes NULL for the callback to the pem
reader, rather than it's privkey_prompt function, but it does set the
default password getter for the SSL context to privkey_prompt (and i
verified it is set to this at the time of the pem_read call), but the PEM
reader seems to ignore it. The PKCS12
reader does the right thing, and uses the context's default password
getter for the context if you pass NULL for it's callback, rather than
it's own default.
This is easy to workaround in neon, even when it is fixed in openssl,
we'll need to workaround for people not using the "bleeding edge stable"
(sounds funny, no?) libraries, assuming it's fixed in 0.9.6<something>.
To workaround this, in ne_ssl_load_pem, use:
sess->client_cert = PEM_read_X509(fp, NULL, privkey_prompt, sess);
rather than
sess->client_cert = PEM_read_X509(fp, NULL, NULL, NULL);
and similarly for the line:
sess->client_key = PEM_read_PrivateKey(fp, NULL, privkey_prompt, sess);
But wait, that's not all.
With apache2+mod_ssl, if the authentication fails because the cacert
could not be found on the server side, the client crashes in an openssl
function, like so:
#0 0x402db600 in X509_get_pubkey () from /usr/lib/libcrypto.so.0
#1 0x40247c96 in SSL_use_PrivateKey_file () from /usr/lib/libssl.so.0
#2 0x40236983 in ssl3_connect () from /usr/lib/libssl.so.0
#3 0x40234053 in ssl3_connect () from /usr/lib/libssl.so.0
#4 0x4024193a in SSL_connect () from /usr/lib/libssl.so.0
#5 0x40216a01 in ne_sock_use_ssl_os (sock=0x809d110, ctx=0x8093220, sess=0x80b2ee8, ssl=0xbfffd088, appdata=0x809d110)
at ne_socket.c:617
Which is either an openssl bug or neon bug.
valgrind shows reads from uninited (and freed) memory in openssl
functions, no errors in neon or our side.
If authentication succeeds, life is happy, no crashes.
This pattern happens no matter if i preload, use the callbacks, use pkcs12
certs, use pem certs (with the private key in a seperate file, or in the
same file), etc.
I'm positive it's not my fault. :).
By the by, the crash is happening on the second neon session, not the
first. Maybe neon is reusing data it shouldn't between the two?
Recompiling openssl with -g on tells me the bug is in incrementing a lock
in X509_get_pubkey which is really pointing to memory that is already
freed, which makes me think neon is sharing data somehow, but freeing it
accidently.
Before i upgraded openssl from 0.9.6d to 0.9.6e , i was
running into a different segfault in this case, so i suspect openssl might
just not be all that robust.
As a followup, i've tried the neon CVS as of right now, and it gets
farther, but crashes doing an SSL_free on the
failed session.
#0 0x402debf0 in ASN1_STRING_free (a=0x9e20800) at asn1_lib.c:389
#1 0x402c9d8c in X509_VAL_free (a=0x80a5fd7) at x_val.c:105
#2 0x402cd4b7 in X509_CINF_free (a=0x809e178) at x_cinf.c:193
#3 0x402cda58 in X509_free (a=0x80a1980) at x_x509.c:154
#4 0x40248764 in SSL_SESSION_free (ss=0x80a5ef0) at ssl_sess.c:476
#5 0x40243874 in SSL_free (s=0x80aa520) at ssl_lib.c:366
It's freed first by OpenSSL, then an SSL_free call in neon causes it to
free it again:
Breakpoint 1, X509_free (a=0x80a4778) at x_x509.c:138
138 if (a == NULL) return;
(gdb) up
#1 0x40239dc3 in ssl3_send_client_certificate (s=0x80a4650) at
s3_clnt.c:1656
1656 if (x509 != NULL) X509_free(x509);
(gdb)
#2 0x40236fde in ssl3_connect (s=0x80a4650) at s3_clnt.c:317
317 ret=ssl3_send_client_certificate(s);
(gdb)
#3 0x4024432d in SSL_connect (s=0x80a4650) at ssl_lib.c:718
718 return(s->method->ssl_connect(s));
(gdb)
#4 0x4024112a in ssl23_get_server_hello (s=0x80a4650) at s23_clnt.c:471
471 return(SSL_connect(s));
(gdb) c
Continuing.
Breakpoint 1, X509_free (a=0x80a4778) at x_x509.c:138
138 if (a == NULL) return;
(gdb) up
#1 0x40246fe4 in ssl_cert_free (c=0x80a3c88) at ssl_cert.c:328
328 X509_free(c->pkeys[i].x509);
(gdb) up
#2 0x402438a2 in SSL_free (s=0x80a4650) at ssl_lib.c:371
371 if (s->cert != NULL) ssl_cert_free(s->cert);
(gdb)
#3 0x40218035 in ne_sock_use_ssl_os (sock=0x80ae970, ctx=0x8096318,
sess=0x0, ssl=0xbffff158, appdata=0x8094200)
at ne_socket.c:666
666 SSL_free(sock->ssl);
(gdb)
If i comment out the SSL_free in question in neon, i can complete without
crashing (though the error displayed is the same as above. It has nothing about the
real problem, which is that the server wouldn't validate our cert. This is
a subversion problem, not a neon one, as far as i can tell).
Note that if you don't make the privkey_prompt change, with neon cvs, you
get the same error you get with 21.3 (IE crash in X509_get_pubkey). This
is odd, and funky, but who the hell am i to question it.
I could have saved hours of trying to track this all down if i hadn't
bothered to test it against my server specifically not including the right
ca certs, to see if what it did.
That'll teach me to actually test my code in failure situations.
Just for completeness sake, here's a diff against neon cvs with the
changes i mentioned. The SSL_free change is likely bogus, but for my
purposes (testing whether the subversion end of all of this works right),
it's sufficient.
Once someone tells me what to do on the subversion side, i'll post the
patch to do client certificate authentication.
Index: src/ne_session.c
===================================================================
RCS file: /home/cvs/neon/src/ne_session.c,v
retrieving revision 1.65
diff -c -3 -p -w -B -b -r1.65 ne_session.c
*** src/ne_session.c 2002/08/04 18:11:15 1.65
--- src/ne_session.c 2002/08/05 05:17:34
*************** int ne_ssl_load_pem(ne_session *sess, co
*** 714,720 ****
return -1;
}
! sess->client_cert = PEM_read_X509(fp, NULL, NULL, NULL);
if (sess->client_cert == NULL) {
ne_set_error(sess, _("Could not read certificate"));
fclose(fp);
--- 714,720 ----
return -1;
}
! sess->client_cert = PEM_read_X509(fp, NULL, privkey_prompt, sess);
if (sess->client_cert == NULL) {
ne_set_error(sess, _("Could not read certificate"));
fclose(fp);
*************** int ne_ssl_load_pem(ne_session *sess, co
*** 732,738 ****
}
}
! sess->client_key = PEM_read_PrivateKey(fp, NULL, NULL, NULL);
fclose(fp);
if (sess->client_key == NULL) {
ne_set_error(sess,
--- 732,738 ----
}
}
! sess->client_key = PEM_read_PrivateKey(fp, NULL, privkey_prompt, sess);
fclose(fp);
if (sess->client_key == NULL) {
ne_set_error(sess,
Index: src/ne_socket.c
===================================================================
RCS file: /home/cvs/neon/src/ne_socket.c,v
retrieving revision 1.94
diff -c -3 -p -w -B -b -r1.94 ne_socket.c
*** src/ne_socket.c 2002/08/04 11:57:46 1.94
--- src/ne_socket.c 2002/08/05 05:17:34
*************** int ne_sock_use_ssl_os(ne_socket *sock,
*** 663,669 ****
sslfail:
SSL_shutdown(sock->ssl);
! SSL_free(sock->ssl);
sock->ssl = NULL;
return NE_SOCK_ERROR;
}
--- 663,669 ----
sslfail:
SSL_shutdown(sock->ssl);
! // SSL_free(sock->ssl);
sock->ssl = NULL;
return NE_SOCK_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 5 07:32:54 2002