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 : [email protected] [mailto:[email protected]] De la part de Arjen de Korte Envoyé : vendredi 7 janvier 2011 21:10 À : [email protected] Objet : Re: [Nut-upsdev] [nut-commits] svn commit r2804 - in branches/ssl-nss-port: clients server Citeren Emilien Kia <[email protected]>: > 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 [email protected] http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev -------------------------------------------------------------------------- _______________________________________________ Nut-upsdev mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
