Kalle Olavi Niemitalo <[EMAIL PROTECTED]> writes:
> Simon Josefsson <[EMAIL PROTECTED]> writes:
>
>> The patch to socket.? removes some never needed code with GnuTLS, and
>> removes the need for the no_tls variable and the ssl_set_no_tls()
>> function altogether by replacing each occurrence of setting the no_tls
>> variable, and calling the function if that variable is set, with the
>> actions done by that function (for the OpenSSL case, no such code is
>> needed for GnuTLS). I have not tested this with OpenSSL, so please
>> double check it.
>
> It seems wrong to me. Previously, ssl_connect() could set
> socket->no_tls = 1, and then call connect_socket(), which would
> indirectly call done_ssl_connection(), which sets socket->ssl =
> NULL; the next call to ssl_connect() would then construct a new
> ssl_t, make socket->ssl point to it, and see that socket->no_tls
> is 1. That is, socket->no_tls used to outlive socket->ssl.
> With your change, ssl_connect() now alters *socket->ssl directly,
> and this effect is lost in done_ssl_connection().
Ah, I think you are right. I did not properly understand the logic
here -- I only noticed that the GnuTLS code would never be necessary,
and then removed too much code.
> The no_tls flag was already present (as no_tsl) in connect.c when
> it was first imported to the ELinks CVS repository on 2001-10-27.
> I don't know why it was originally added, but I'm guessing it
> works around buggy servers that happily negotiate TLS but then
> fail to implement it properly. Could you explain why you think
> ELinks does not need such a workaround with GnuTLS?
Because trying to do the same handshake another time will not make it
work better.
If I understand the OpenSSL code part correctly, it disables TLS
(i.e., then only SSL 3.0 is supported). The GnuTLS code does not
disable TLS. The algorithm settings looks rather similar to those in
ssl.c. What you might want to do is to disable TLS and only use SSL
3.0 with GnuTLS too. However, unless there are documented examples of
web servers that need this workaround, I'm not sure it should be
added.
> Of course, it is possible that all the buggy servers have already
> been fixed and we can remove the workaround for OpenSSL too, but
> even in that case it should be done as a separate patch, so it's
> easier to revert if necessary.
Yeah, I agree, and I did not mean to touch existing OpenSSL code since
I have not tested it. I only meant to clean up the GnuTLS code.
Below is an updated patch that only touch GnuTLS code.
> If your patches are applied to ELinks, I would like to add your
> name and email address (as shown above) to the AUTHORS file and
> to the author field of the commits in the ELinks Git repository.
> From there, they would then propagate to an unknown number of
> mirrors and other systems (e.g. CIA.vc) worldwide. It would be
> difficult to remove this information afterwards. Do you consent
> to this, or would you rather like to e.g. obscure the email
> address?
No, my real address is fine.
Btw, if you have any GnuTLS-related problems in the future (I've
noticed one such report in your mailing list related to nordea.fi and
I'll try to debug it), please let me know and I can try to help.
Thanks,
Simon
diff --git a/src/network/ssl/socket.c b/src/network/ssl/socket.c
index 96caf8b..322a718 100644
--- a/src/network/ssl/socket.c
+++ b/src/network/ssl/socket.c
@@ -63,83 +63,6 @@ ssl_set_no_tls(struct socket *socket)
{
#ifdef CONFIG_OPENSSL
((ssl_t *) socket->ssl)->options |= SSL_OP_NO_TLSv1;
-#elif defined(CONFIG_GNUTLS)
- /* We do a little more work here, setting up all these priorities (like
- * they couldn't have some reasonable defaults there).. */
-
- {
- int protocol_priority[3] = {
- GNUTLS_TLS1,
- GNUTLS_SSL3,
- 0
- };
-
- gnutls_protocol_set_priority(*((ssl_t *) socket->ssl),
protocol_priority);
- }
-
- /* Note that I have no clue about these; I just put all I found here
- * ;-). It is all a bit confusing for me, and I just want this to work.
- * Feel free to send me patch removing useless superfluous bloat,
- * thanks in advance. --pasky */
-
- {
- int cipher_priority[5] = {
- GNUTLS_CIPHER_RIJNDAEL_128_CBC,
- GNUTLS_CIPHER_3DES_CBC,
- GNUTLS_CIPHER_ARCFOUR,
- GNUTLS_CIPHER_RIJNDAEL_256_CBC,
- 0
- };
-
- gnutls_cipher_set_priority(*((ssl_t *) socket->ssl),
cipher_priority);
- }
-
- {
- /* Does any httpd support this..? ;) */
- int comp_priority[3] = {
- GNUTLS_COMP_ZLIB,
- GNUTLS_COMP_NULL,
- 0
- };
-
- gnutls_compression_set_priority(*((ssl_t *) socket->ssl),
comp_priority);
- }
-
- {
- int kx_priority[5] = {
- GNUTLS_KX_RSA,
- GNUTLS_KX_DHE_DSS,
- GNUTLS_KX_DHE_RSA,
- /* Looks like we don't want SRP, do we? */
- GNUTLS_KX_ANON_DH,
- 0
- };
-
- gnutls_kx_set_priority(*((ssl_t *) socket->ssl), kx_priority);
- }
-
- {
- int mac_priority[3] = {
- GNUTLS_MAC_SHA,
- GNUTLS_MAC_MD5,
- 0
- };
-
- gnutls_mac_set_priority(*((ssl_t *) socket->ssl), mac_priority);
- }
-
- {
- int cert_type_priority[2] = {
- GNUTLS_CRT_X509,
- /* We don't link with -extra now; by time of writing
- * this, it's unclear where OpenPGP will end up. */
- 0
- };
-
- gnutls_certificate_type_set_priority(*((ssl_t *) socket->ssl),
cert_type_priority);
- }
-
- gnutls_dh_set_prime_bits(*((ssl_t *) socket->ssl), 1024);
#endif
}
diff --git a/src/network/ssl/ssl.c b/src/network/ssl/ssl.c
index 3c38765..c14ab67 100644
--- a/src/network/ssl/ssl.c
+++ b/src/network/ssl/ssl.c
@@ -107,9 +107,6 @@ static struct module openssl_module = struct_module(
gnutls_anon_client_credentials_t anon_cred = NULL;
gnutls_certificate_credentials_t xcred = NULL;
-const static int protocol_priority[16] = {
- GNUTLS_TLS1, GNUTLS_SSL3, 0
-};
const static int kx_priority[16] = {
GNUTLS_KX_RSA, GNUTLS_KX_DHE_DSS, GNUTLS_KX_DHE_RSA, GNUTLS_KX_SRP,
/* Do not use anonymous authentication, unless you know what that means
*/
@@ -119,8 +116,6 @@ const static int cipher_priority[16] = {
GNUTLS_CIPHER_RIJNDAEL_128_CBC, GNUTLS_CIPHER_ARCFOUR_128,
GNUTLS_CIPHER_3DES_CBC, GNUTLS_CIPHER_AES_256_CBC,
GNUTLS_CIPHER_ARCFOUR_40, 0
};
-const static int comp_priority[16] = { GNUTLS_COMP_ZLIB, GNUTLS_COMP_NULL, 0 };
-const static int mac_priority[16] = { GNUTLS_MAC_SHA, GNUTLS_MAC_MD5, 0 };
const static int cert_type_priority[16] = { GNUTLS_CRT_X509,
GNUTLS_CRT_OPENPGP, 0 };
static void
@@ -232,12 +227,10 @@ init_ssl_connection(struct socket *socket)
return S_SSL_ERROR;
}
+ gnutls_set_default_priority(*state);
gnutls_handshake_set_private_extensions(*state, 1);
gnutls_cipher_set_priority(*state, cipher_priority);
- gnutls_compression_set_priority(*state, comp_priority);
gnutls_kx_set_priority(*state, kx_priority);
- gnutls_protocol_set_priority(*state, protocol_priority);
- gnutls_mac_set_priority(*state, mac_priority);
gnutls_certificate_type_set_priority(*state, cert_type_priority);
gnutls_server_name_set(*state, GNUTLS_NAME_DNS, server_name,
sizeof(server_name) - 1);
_______________________________________________
elinks-dev mailing list
elinks-dev@linuxfromscratch.org
http://linuxfromscratch.org/mailman/listinfo/elinks-dev
