[Nut-upsdev] [nut-commits] svn commit r2804 - in branches/ssl-nss-port: clients server

[EmilienKia at Eaton.com]

Hi Arjen, Hi all,

I agree with you to say that conf.c must do a minimal interpretation of SSL configuration, and should not set ssl related variables directly (I will change that soon), but I do understand the way you propose:

Do you mean that:
 - netssl.c provides a configuration function to call at the beginning of conf.c:parse_upsd_conf_args() and it parse all the ssl config ?
 - netssl.c provides some configuration functions, one per directive, and in the case of CERTREQUEST, parses and converts string argument into internal format (IMHO better way) ?
 - netssl.c provides some configuration function, one per directive, CERTREQUEST argument parsing is done in conf.c:parse_upsd_conf_args() and the dedicated nss configuration function is called with the corresponding value (IMHO best way because best decoupling between human readable configuration directive and functional storage) ?

BR,
Emilien

-----Message d'origine-----
De : nut-upsdev-bounces+emilienkia=eaton.com at lists.alioth.debian.org [mailto:nut-upsdev-bounces+emilienkia=eaton.com at lists.alioth.debian.org] De la part de Arjen de Korte
Envoyé : vendredi 7 janvier 2011 21:10
À : nut-upsdev at lists.alioth.debian.org
Objet : Re: [Nut-upsdev] [nut-commits] svn commit r2804 - in branches/ssl-nss-port: clients server

Citeren Emilien Kia <emilienkia-guest at alioth.debian.org>:

> Modified: branches/ssl-nss-port/server/conf.c
> ==============================================================================
> --- branches/ssl-nss-port/server/conf.c	Wed Jan  5 21:12:03 2011	(r2803)
> +++ branches/ssl-nss-port/server/conf.c	Thu Jan  6 10:27:55 2011	(r2804)
> @@ -178,6 +178,22 @@
>  		return 1;
>  	}
>
> +	/* CERTREQUEST ("NO" | "REQUEST" | "REQUIRE") */
> +	if (!strcmp(arg[0], "CERTREQUEST")) {
> +		if (strcasecmp(arg[1], "REQUEST") == 0) {
> +			certrequest = NETSSL_CERTREQ_REQUEST;
> +		} else if (strcasecmp(arg[1], "REQUIRE") == 0) {
> +			certrequest = NETSSL_CERTREQ_REQUIRE;
> +		} else if (strcasecmp(arg[1], "NO") == 0) {
> +			certrequest = NETSSL_CERTREQ_NO;
> +		} else {
> +			upslogx(LOG_WARNING, "CERTREQUEST in upsd.conf accept only values "
> +				"\"REQUEST\", \"REQUIRE\" or \"NO\", assuming \"NO\"");
> +			certrequest = NETSSL_CERTREQ_NO;
> +		}
> +		return 1;
> +	}
> +

 From a maintenance point of view, the validation of the CERTREQUEST  
parameter should be handled in 'netssl.c', not here. We really don't  
want to mess with this here, to prevent having to change 'conf.c' too  
often when something changes in the NSS code.

Likewise, it would be useful if this would only be compiled in if the  
NSS library is actually used (same for CERTPATH and CERTIDENT). It  
would be better to complain about invalid parameters than to fail  
later on.

Best regards, Arjen
-- 
Please keep list traffic on the list (off-list replies will be rejected)


_______________________________________________
Nut-upsdev mailing list
Nut-upsdev at lists.alioth.debian.org
http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev

--------------------------------------------------------------------------