[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard
by fidencio
URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard
fidencio commented:
"""
So, I've just done some tests here and seems that we can just drop "authtok: check for NULL in sss_authtok_get_type()" patch, as long as we add the following fixup to the "p11_child: use options to select certificate for authentication" patch.
```
[ffidenci@pessoa x86_64]$ cat 0001-fixup-p11_child-use-options-to-select-certificate-fo.patch
From 0d2ef9ac950d3539fa1ef0044790799d900f6a21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
Date: Fri, 10 Nov 2017 14:04:15 +0100
Subject: [PATCH] fixup! p11_child: use options to select certificate for
authentication
---
src/responder/pam/pamsrv_p11.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c
index 7f7b02b0d..27aeb9833 100644
--- a/src/responder/pam/pamsrv_p11.c
+++ b/src/responder/pam/pamsrv_p11.c
@@ -428,10 +428,9 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
extra_args[arg_c++] = "--verify";
}
- if (pd->authtok != NULL
- && (sss_authtok_get_type(pd->authtok) == SSS_AUTHTOK_TYPE_SC_PIN
- || sss_authtok_get_type(pd->authtok) ==
- SSS_AUTHTOK_TYPE_SC_KEYPAD)) {
+ if (sss_authtok_get_type(pd->authtok) == SSS_AUTHTOK_TYPE_SC_PIN
+ || sss_authtok_get_type(pd->authtok) ==
+ SSS_AUTHTOK_TYPE_SC_KEYPAD) {
ret = sss_authtok_get_sc(pd->authtok, NULL, NULL, &token_name, NULL,
&module_name, NULL,
&key_id, NULL);
--
2.13.6
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/433#issuecomment-343471275
6 years, 5 months
[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard
by lslebodn
URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard
lslebodn commented:
"""
>@lslebodn, I suggested to add the check to sss_authtok_get_type() because most of the other sss_authtok_* call do a similar check. I agree that the check is not strictly needed, but I think it will help to make the sss_authok component more reliable with a minimal effort.
But other function distinguish between NULL end empty password
e.g.
```
errno_t sss_authtok_get_password(struct sss_auth_token *tok,
const char **pwd, size_t *len)
{
if (!tok) {
return EFAULT;
}
switch (tok->type) {
case SSS_AUTHTOK_TYPE_EMPTY:
return ENOENT;
```
>About the check in pam_check_cert_send(). I guess I added it automatically to make sure to not dereference a NULL pointer without checking if it is possible at all at this point. Do you want me to remove it?
Yes please it is a dead code in `pam_check_cert_send`
"""
See the full comment at https://github.com/SSSD/sssd/pull/433#issuecomment-343455545
6 years, 5 months
[sssd PR#432][comment] CACHE_REQ: Better debugging for email conflicts
by fidencio
URL: https://github.com/SSSD/sssd/pull/432
Title: #432: CACHE_REQ: Better debugging for email conflicts
fidencio commented:
"""
Just for the record. We had a meeting Yesterday and we discussed this PR face-to-face in order to speed the process up and do not block it.
@lslebodn's worries **do** make sense and **we agreed to work on his suggestions, but out of the scope of this PR**.
So, **this PR will be pushed** but firstly I'd like to have a review from a native English speaker, mainly in the manpage part. @justin-stephenson, could you help us here?
@mzidek-rh, please, could you open the ticket for @lslebodn's suggestions as we agreed in the meeting and put the link here?
"""
See the full comment at https://github.com/SSSD/sssd/pull/432#issuecomment-343409261
6 years, 5 months