Re: [PATCH] Add support for nss_compat_ossl library (OpenSSL replacement).

Kamil Dudka <[email protected]> writes:

> as requested in src/network/ssl/TODO support for NSS was added to elinks.

Although it is listed in TODO, I am not sure it is a good idea.
Current versions of ELinks do not check the subject names of
certificates (bug 1024), do not display detailed error messages
when SSL or TLS fails, and do not provide a way to temporarily
trust a certificate.  I fear fixing these and maintaining the
resulting code will be more difficult if there are more
alternative libraries.

Does NSS enable ELinks to share the CA trust and client
certificate settings of Mozilla browsers?  If so, that would be
a reason to support NSS.

> From: Kamil Dudka <[email protected]>

Do you give permission to add this name and email address to our
public Git repository?  Please note the cia.vc and ohloh.net web
sites already collect profiles of people and others can do so in
the future.

ELinks was originally licensed under GNU GPL v2 or later, but
since ELinks 0.10.0, only GPLv2 is allowed.  If we ever want to
change the license back (e.g. because of LGPLv3 libraries), we'll
need permission from all copyright holders, I suppose.  Do you
give such permission?

> @@ -79,6 +83,8 @@ ssl_set_no_tls(struct socket *socket)
>  
>  		gnutls_protocol_set_priority(*(ssl_t *) socket->ssl, protocol_priority);
>  	}
> +#elif defined(CONFIG_NSS_COMPAT_OSSL)
> +#warning "ssl_setno_tls is not implemented while using nss_compat_ossl"
>  #endif
>  }

configure --enable-debug adds -Werror to CFLAGS if using GCC;
does this #warning then break the build?  Also, it's a
nonstandard directive and may hurt users of other compilers.
(There is a #warning in osdep/system.h too but it's in an #if
whose condition should never be true.)

If ssl_set_no_tls is not necessary because NSS automatically
selects the right protocol version (more reliably than GnuTLS),
then I don't think the warning should be there either.
Otherwise, ssl_set_no_tls should be implemented... or is it
impossible to implement with nss_compat_ossl?

> +#ifdef CONFIG_NSS_COMPAT_OSSL
> +	INIT_OPT_STRING("connection.ssl.client_cert", N_("Certificate nickname"),
> +		"file", 0, "",
> +		 N_("The nickname of the client certificate stored in NSS\n"
> +		    "database. If this value is unset, the file pointed to\n"
> +		    "by the X509_CLIENT_CERT variable is used instead. If\n"
> +		    "you have a PKCS#12 file containing client certificate,\n"
> +		    "you can import it into your NSS database with:\n"
> +		    "$ pk12util -i mycert.p12 -d /path/to/database\n\n"
> +		    "The NSS database location can be changed by SSL_DIR\n"
> +		    "environment variable.")),
> +#else
>  	INIT_OPT_STRING("connection.ssl.client_cert", N_("Certificate File"),
>  		"file", 0, "",
>  		 N_("The location of a file containing the client certificate\n"
>  		    "and unencrypted private key in PEM format. If unset, the\n"
>  		    "file pointed to by the X509_CLIENT_CERT variable is used\n"
>  		    "instead.")),
> +#endif

You make connection.ssl.client_cert.file mean a file name if
using OpenSSL, but a nickname if using NSS.  Can the user set up
an elinks.conf that works correctly under both interpretations?
(For example, by defining a nickname that looks like a file
name.)  If not, I think it would be better to use a different
name for the nickname option.  (An elinks binary built with NSS
might then warn about unrecognized connection.ssl.client_cert.file
in elinks.conf.  That could be fixed either by unconditionally
compiling the option definitions in ELinks, or by changing the
elinks.conf parser not to warn about unrecognized options by
default.  The latter solution seems nicer because it avoids bloat
and removes annoying warnings also when SSL support or some other
module is entirely disabled.)

Your current documentation string claims that X509_CLIENT_CERT
is a file name even with NSS.  However, the code seems to use
the value of X509_CLIENT_CERT simply as a default for the option,
thus as a nickname.

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIshCgHm9IGt60eMgRAlzzAKC/t8a+RwJ9Ybmu2j9YrhI9+pjGQwCeNlDV
YzzKBSc8Ounii6e4ygLXyWQ=
=Etmi
-----END PGP SIGNATURE-----