On Wed, Jul 06, 2016 at 11:10:38AM +0200, Jakub Hrozek wrote:
On Wed, Jul 06, 2016 at 11:04:56AM +0200, Lukas Slebodnik wrote:
> Updated patch is attached.
it is not :-)
(for whatever reasons Lukas' reply with the patch was not delivered to
my inbox, so I picked it from the archive)
The patch looks good and works as expected. I only have a few comments:
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index d5442d0cd82fbf31db5e0aef5b7fd32d2b7fff94..4cd2e5727922cc32011c96e23ff9e70309405955
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -1126,6 +1126,86 @@ pam_account_locked_message = Account locked, please contact help
desk.
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>pam_p11_allowed_services (integer)</term>
+ <listitem>
+ <para>
+ A comma-separated list of PAM service names for
+ which it will be allowed to use smartcards.
+ </para>
+ <para>
+ It is possible to add another PAM service name to
+ the default set by using
+ <quote>+service_name</quote> or to explicitly
+ remove a PAM service name from the default set by
+ using <quote>-service_name</quote>. For
example,
+ in order to replace a default PAM service name for
+ authentication with smartcads
typo ^^^^^^^^^
+ (e.g. <quote>login</quote>)
with a custom pam
+ service name (e.g.
<quote>my_pam_service</quote>),
+ you would use the following configuration:
...
+ break;
+ default:
+ DEBUG(SSSDBG_OP_FAILURE,
I would suggest to make this SSSDBG_CRIT_FAILURE as well, to match the other
messages related to issues parsing the config option.
+ "The option
"CONFDB_PAM_P11_ALLOWED_SERVICES" must start"
+ "with either '+' (for adding service) or '-' (for
"
+ "removing service) got '%s'\n", conf_list[i]);
+ ret = EINVAL;
+ goto done;
+ }
+ }
+
...
+ /* Initialize smartcard allowed services just once */
+ if (pctx->smartcard_services == NULL) {
+ ret = get_sc_services(pctx, pctx, &pctx->smartcard_services);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to get p11 allowed services %d[%s]",
+ ret, sss_strerror(ret));
+ return false;
+ }
+ }
+
I wonder if it would be better to initialize pctx->smartcard_services during
the initialization of the PAM responder. Because the this point we can fail
completely and a broken configuration would be more obvious? If we keep it here
maybe a sss_log() messages would make sense to better indicate the config
error?
bye,
Sumit
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org