[SSSD] [PATCH] Fix bad memory interaction at process exit

2010-03-18 Thread Simo Sorce 

These bugs were never much important as they tend to happen only when
the monitor exits. Yet analyzing them I found some problems that might
happen also on a normally running daemon. It is also bad to have the
monitor potentially crash on exit, as that may also be caught by abrt
and cause false positives in bugzilla later on.

Comments in the patches should be clear.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
>From 3bb5645b767910c0112e727a41dab5ec45061b3b Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Thu, 18 Mar 2010 18:52:36 -0400
Subject: [PATCH 1/2] Fix invalid read cause by premature free of tmpctx

---
 src/monitor/monitor.c |   23 ++-
 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 0ba3354..c6af606 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -1344,8 +1344,7 @@ static void process_config_file(struct tevent_context *ev,
 
 buf = talloc_size(tmp_ctx, event_size);
 if (!buf) {
-talloc_free(tmp_ctx);
-return;
+goto done;
 }
 
 total_len = 0;
@@ -1354,8 +1353,7 @@ static void process_config_file(struct tevent_context *ev,
event_size-total_len);
 if (len == -1 && errno != EINTR) {
 DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
-talloc_free(tmp_ctx);
-return;
+goto done;
 }
 total_len += len;
 }
@@ -1368,23 +1366,19 @@ static void process_config_file(struct tevent_context *ev,
  */
 name = talloc_size(tmp_ctx, len);
 if (!name) {
-talloc_free(tmp_ctx);
-return;
+goto done;
 }
 total_len = 0;
 while (total_len < in_event->len) {
 len = read(file_ctx->mt_ctx->inotify_fd, &name, in_event->len);
 if (len == -1 && errno != EINTR) {
 DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
-talloc_free(tmp_ctx);
-return;
+goto done;
 }
 total_len += len;
 }
 }
 
-talloc_free(tmp_ctx);
-
 for (cb = file_ctx->callbacks; cb; cb = cb->next) {
 if (cb->wd == in_event->wd) {
 break;
@@ -1392,7 +1386,7 @@ static void process_config_file(struct tevent_context *ev,
 }
 if (!cb) {
 DEBUG(0, ("Unknown watch descriptor\n"));
-return;
+goto done;
 }
 
 if (in_event->mask & IN_IGNORED) {
@@ -1413,7 +1407,7 @@ static void process_config_file(struct tevent_context *ev,
 DEBUG(0, ("Could not restore inotify watch. Quitting!\n"));
 close(file_ctx->mt_ctx->inotify_fd);
 kill(getpid(), SIGTERM);
-return;
+goto done;
 }
 rw_ctx->cb = cb;
 rw_ctx->file_ctx = file_ctx;
@@ -1424,12 +1418,15 @@ static void process_config_file(struct tevent_context *ev,
 close(file_ctx->mt_ctx->inotify_fd);
 kill(getpid(), SIGTERM);
 }
-return;
+goto done;
 }
 
 /* Tell the monitor to signal the children */
 cb->fn(file_ctx, cb->filename);
 file_ctx->needs_update = 0;
+
+done:
+talloc_free(tmp_ctx);
 }
 
 static void rewatch_config_file(struct tevent_context *ev,
-- 
1.6.6.1

>From 834194138cba5e390cdc5dab6be94417c182 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Thu, 18 Mar 2010 19:22:44 -0400
Subject: [PATCH 2/2] Fix multiple errors with destructors.

This commits cleans up 3 segfaults/valgrind errors due to access
to freed memory.

1. The spy wasn't clearing conn_spy causing the svc_destructor to try
   to clear the spy destructor when the spy was already freed

2. get_config_service was not setting the svc_destrcutor on services
   depending on the orderof frees at exit this was causing the spy
   destructor to try to access freed memory because it was not
   neutralized when the service was freed.

3. at exit the mt_ctx could be freed before services causing the
   svc_destrcutor to try to access freed memory when removing the
   service from the service list in the monitor context.
---
 src/monitor/monitor.c |   24 ++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index c6af606..ddf3de9 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -282,8 +282,10 @@ static int svc_destructor(void *mem)
 return 0;
 }
 
-/* always try to delist service */
-DLIST_REMOVE(svc->mt_ctx->svc_list, svc);
+/* try to delist service */
+if (svc->mt_ctx) {
+DLIST_REMOVE(svc->mt_ctx->svc_list, svc);
+}
 
 /* svc is beeing freed, neutralize the spy */
 if (svc->conn_spy) {
@@ -302,6 +304,7 @@ static int svc_spy_destructor(void *mem)
 }
 
 /* svc->conn has been freed, NULL the pointer in svc */
+spy->svc->conn_spy

[SSSD] [PATCH] Check for controls before using them

2010-03-18 Thread Simo Sorce 

Some time ago I added code to fetch the rootdse on connection, but
didn't publicize it too much.

Attached find 2 patches.

1) Rework the way we store data fetched from the rootdse so the it is
more useful and is actually attached to the ldap handle.

2) Check controls are supported before using them.

Simo. 

-- 
Simo Sorce * Red Hat, Inc * New York
>From be64c4245db58200db783623a6eaa791fe4339d6 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Thu, 18 Mar 2010 16:58:02 -0400
Subject: [PATCH 1/2] Store rootdse supported features in sdap_handler

---
 src/providers/ipa/ipa_access.c |   10 +--
 src/providers/ldap/ldap_common.h   |3 -
 src/providers/ldap/ldap_id.c   |   15 ++---
 src/providers/ldap/ldap_id_enum.c  |   10 +--
 src/providers/ldap/sdap.c  |   84 ++--
 src/providers/ldap/sdap.h  |   23 +++-
 src/providers/ldap/sdap_async.h|5 +-
 src/providers/ldap/sdap_async_connection.c |   45 ++-
 8 files changed, 120 insertions(+), 75 deletions(-)

diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 7dfe1fd..6740f09 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -437,7 +437,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
 }
 
 subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
-   sdap_ctx->be, sdap_ctx->service, NULL);
+   sdap_ctx->be, sdap_ctx->service, false);
 if (!subreq) {
 DEBUG(1, ("sdap_cli_connect_send failed.\n"));
 ret = ENOMEM;
@@ -482,8 +482,7 @@ static void hbac_get_host_info_connect_done(struct tevent_req *subreq)
struct hbac_get_host_info_state);
 int ret;
 
-ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh,
-NULL);
+ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh);
 talloc_zfree(subreq);
 if (ret) {
 tevent_req_error(req, ret);
@@ -946,7 +945,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
 }
 
 subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
-   sdap_ctx->be, sdap_ctx->service, NULL);
+   sdap_ctx->be, sdap_ctx->service, false);
 if (!subreq) {
 DEBUG(1, ("sdap_cli_connect_send failed.\n"));
 ret = ENOMEM;
@@ -991,8 +990,7 @@ static void hbac_get_rules_connect_done(struct tevent_req *subreq)
  struct hbac_get_rules_state);
 int ret;
 
-ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh,
-NULL);
+ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh);
 talloc_zfree(subreq);
 if (ret) {
 tevent_req_error(req, ret);
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index ff1ffb7..cbacf83 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -39,9 +39,6 @@ struct sdap_id_ctx {
 struct fo_service *fo_service;
 struct sdap_service *service;
 
-/* what rootDSE returns */
-struct sysdb_attrs *rootDSE;
-
 /* global sdap handler */
 struct sdap_handle *gsh;
 
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 4bbc07a..7a646a3 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -106,7 +106,7 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
  * or SASL/GSSAPI, etc ... */
 subreq = sdap_cli_connect_send(state, ev, ctx->opts,
ctx->be, ctx->service,
-   &ctx->rootDSE);
+   false);
 if (!subreq) {
 ret = ENOMEM;
 goto fail;
@@ -143,8 +143,7 @@ static void users_get_connect_done(struct tevent_req *subreq)
  struct users_get_state);
 int ret;
 
-ret = sdap_cli_connect_recv(subreq, state->ctx,
-&state->ctx->gsh, &state->ctx->rootDSE);
+ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh);
 talloc_zfree(subreq);
 if (ret) {
 if (ret == ENOTSUP) {
@@ -329,7 +328,7 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
  * or SASL/GSSAPI, etc ... */
 subreq = sdap_cli_connect_send(state, ev, ctx->opts,
ctx->be, ctx->service,
-   &ctx->rootDSE);
+   false);
 if (!subreq) {
 ret = ENOMEM;
 goto fail;
@@ -366,8 +365

[SSSD] [PATCHES] Add missing option ldap_tls_cacertdir to SSSDConfig API

2010-03-18 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Patch 0001: Add the missing option to the API.  Pushed to master and
1-1-0 under the one-liner rule. This is needed by authconfig.

Patch 0002: Add the translated help text to the API. Pushed to master.
Not pushed to 1-1-0 because of string freeze.
- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkuiiXwACgkQeiVVYja6o6NepACgnEPfHkJpXgMFcZRmdOtwCrfe
G2sAnRrwHLNfEAriFh7SpzWJcBhQ3flP
=GgvV
-END PGP SIGNATURE-
From 0f13d4204b53be92f34347c9b1b9d3db6809fd77 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Thu, 18 Mar 2010 16:01:58 -0400
Subject: [PATCH 1/2] Add missing ldap_tls_cacertdir option to SSSDConfig API

---
 src/config/etc/sssd.api.d/sssd-ldap.conf |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf
index 6758ab4..d2b47e1 100644
--- a/src/config/etc/sssd.api.d/sssd-ldap.conf
+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf
@@ -9,6 +9,7 @@ ldap_network_timeout = int, None, false
 ldap_opt_timeout = int, None, false
 ldap_offline_timeout = int, None, false
 ldap_tls_cacert = str, None, false
+ldap_tls_cacertdir = str, None, false
 ldap_tls_reqcert = str, None, false
 ldap_sasl_mech = str, None, false
 ldap_sasl_authid = str, None, false
-- 
1.6.6.1

From 5b0da925807fbe2b90788b3479776e2b70c04ed8 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Thu, 18 Mar 2010 16:10:33 -0400
Subject: [PATCH 2/2] Add translated help text for ldap_tls_cacertdir

---
 src/config/SSSDConfig.py |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/config/SSSDConfig.py b/src/config/SSSDConfig.py
index 7dd90e7..d073a68 100644
--- a/src/config/SSSDConfig.py
+++ b/src/config/SSSDConfig.py
@@ -112,7 +112,8 @@ option_strings = {
 'ldap_network_timeout' : _('Length of time to attempt connection'),
 'ldap_opt_timeout' : _('Length of time to attempt synchronous LDAP operations'),
 'ldap_offline_timeout' : _('Length of time between attempts to reconnect while offline'),
-'ldap_tls_cacert' : _('file that contains CA certificates'),
+'ldap_tls_cacert' : _('File that contains CA certificates'),
+'ldap_tls_cacertdir' : _('Path to CA certificate directory'),
 'ldap_tls_reqcert' : _('Require TLS certificate verification'),
 'ldap_sasl_mech' : _('Specify the sasl mechanism to use'),
 'ldap_sasl_authid' : _('Specify the sasl authorization id to use'),
-- 
1.6.6.1



0001-Add-missing-ldap_tls_cacertdir-option-to-SSSDConfig-.patch.sig
Description: PGP signature


0002-Add-translated-help-text-for-ldap_tls_cacertdir.patch.sig
Description: PGP signature
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Fix error message for ldap_start_tls

2010-03-18 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/18/2010 02:39 PM, Stephen Gallagher wrote:
> Patch is obvious.
> 

Pushed to master and 1-1-0 under the one-liner rule.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkuihJ8ACgkQeiVVYja6o6OSvQCfdxEPIgC+Cc46OXFX4vJqCORA
ujkAniGXRrHBG8xt9fB1DXjaXYmMBVqW
=uvpl
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [Transifex] File submitted via email to SSSD | master

2010-03-18 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/18/2010 12:48 PM, ad...@transifex.net wrote:
> Hello sssd, this is Transifex at http://www.transifex.net.
> 
> The following attached files were submitted to SSSD | master by ruigo 
>  
> 
> Please, visit Transifex at http://www.transifex.net/projects/p/sssd/c/master/ 
> in order to see the component page.
> 
> Thank you,
> Transifex
> 

Ack and pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkuihJIACgkQeiVVYja6o6PMqACbBxIWHHENvTCikJFnn0x6dyyg
uRIAn0ijAjPeMYD2Gk1E+dBTck7wMLuU
=yJyW
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] Fix error message for ldap_start_tls

2010-03-18 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Patch is obvious.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkuic0oACgkQeiVVYja6o6NpjgCfVA95FltfPVySorsCcL37cxKp
DTcAoJcGFP1vY+qMDWvhRB51iAsK1B+L
=6Ek5
-END PGP SIGNATURE-
From 91e83474ef313015f587df0875e08216ca80228f Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Thu, 18 Mar 2010 14:24:07 -0400
Subject: [PATCH] Fix error message for ldap_start_tls

---
 src/providers/ldap/sdap_async_connection.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index fe8a501..586733f 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -162,7 +162,7 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
 
 lret = ldap_start_tls(state->sh->ldap, NULL, NULL, &msgid);
 if (lret != LDAP_SUCCESS) {
-DEBUG(3, ("ldap_start_tls failed: [%s]", ldap_err2string(ret)));
+DEBUG(3, ("ldap_start_tls failed: [%s]\n", ldap_err2string(lret)));
 goto fail;
 }
 
-- 
1.6.6.1



0001-Fix-error-message-for-ldap_start_tls.patch.sig
Description: PGP signature
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [Transifex] File submitted via email to SSSD | master

2010-03-18 Thread admin 
Hello sssd, this is Transifex at http://www.transifex.net.

The following attached files were submitted to SSSD | master by ruigo 
 

Please, visit Transifex at http://www.transifex.net/projects/p/sssd/c/master/ 
in order to see the component page.

Thank you,
Transifex
# SOME DESCRIPTIVE TITLE.
# Copyright (C) YEAR Red Hat, Inc.
# This file is distributed under the same license as the PACKAGE package.
# Rui Gouveia , 2010.
msgid ""
msgstr ""
"Project-Id-Version: sssd.master.sss_daemon\n"
"Report-Msgid-Bugs-To: sssd-de...@lists.fedorahosted.org\n"
"POT-Creation-Date: 2010-03-15 08:12-0400\n"
"PO-Revision-Date: 2010-02-23 13:59+0100\n"
"Last-Translator: Rui Gouveia \n"
"Language-Team: fedora-trans...@redhat.com\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Language: pt\n"
"Plural-Forms: nplurals=2; plural=(n != 1);\n"
"X-Poedit-Language: Portuguese\n"
"X-Poedit-Country: PORTUGAL\n"
"X-Generator: Virtaal 0.5.2\n"

#: config/SSSDConfig.py:39
msgid "Set the verbosity of the debug logging"
msgstr "Definir a verbosidade dos registos de depuração"

#: config/SSSDConfig.py:40
msgid "Include timestamps in debug logs"
msgstr "Incluir data e hora nos registos de depuração"

#: config/SSSDConfig.py:41
msgid "Write debug messages to logfiles"
msgstr "Gravar as mensagens de depuração em ficheiros de registo"

#: config/SSSDConfig.py:42
msgid "Ping timeout before restarting service"
msgstr "Foi excedido o tempo do ping antes de reiniciar o serviço"

#: config/SSSDConfig.py:43
msgid "Command to start service"
msgstr "Comando para iniciar serviço"

#: config/SSSDConfig.py:44
msgid "Number of times to attempt connection to Data Providers"
msgstr "Número de vezes para tentar ligação aos Fornecedores de Dados"

#: config/SSSDConfig.py:47
msgid "SSSD Services to start"
msgstr "Serviços SSSD a iniciar"

#: config/SSSDConfig.py:48
msgid "SSSD Domains to start"
msgstr "Domínios SSSD a iniciar"

#: config/SSSDConfig.py:49
msgid "Timeout for messages sent over the SBUS"
msgstr "Limite de tempo para mensagens enviadas sobre SBUS"

#: config/SSSDConfig.py:50
msgid "Regex to parse username and domain"
msgstr "Expressão regular para obter nome do utilizar e domínio"

#: config/SSSDConfig.py:51
msgid "Printf-compatible format for displaying fully-qualified names"
msgstr "Formato compatível com o printf para apresentar nomes completos"

#: config/SSSDConfig.py:54
msgid "Enumeration cache timeout length (seconds)"
msgstr "Validade da cache de enumeração (segundos)"

#: config/SSSDConfig.py:55
msgid "Entry cache background update timeout length (seconds)"
msgstr "Validade da actualização da cache em segundo plano (segundos)"

#: config/SSSDConfig.py:56
msgid "Negative cache timeout length (seconds)"
msgstr "Validade da cache negativa (segundos)"

#: config/SSSDConfig.py:57
msgid "Users that SSSD should explicitly ignore"
msgstr "Utilizadores que o SSSD devem explicitamente ignorar"

#: config/SSSDConfig.py:58
msgid "Groups that SSSD should explicitly ignore"
msgstr "Grupos que o SSSD devem explicitamente ignorar"

#: config/SSSDConfig.py:59
msgid "Should filtered users appear in groups"
msgstr "Devem os utilizadores filtrados aparecer em grupos"

#: config/SSSDConfig.py:60
msgid "The value of the password field the NSS provider should return"
msgstr "O valor do campo da senha que o fornecedor NSS deve retornar"

#: config/SSSDConfig.py:63
msgid "How long to allow cached logins between online logins (days)"
msgstr "Durante quanto tempo devem ser permitidas as caches de sessões entre 
sessões online (dias)"

#: config/SSSDConfig.py:64
msgid "How many failed logins attempts are allowed when offline"
msgstr "Quantas tentativas falhadas de inicio de sessão são permitidas quando 
offline"

#: config/SSSDConfig.py:65
msgid "How long (minutes) to deny login after offline_failed_login_attempts has 
been reached"
msgstr "Quanto tempo (minutos) para negar a sessão após 
offline_failed_login_attempts ter sido atingido"

#: config/SSSDConfig.py:68
msgid "Identity provider"
msgstr "Fornecedor de identidade"

#: config/SSSDConfig.py:69
msgid "Authentication provider"
msgstr "Fornecedor de autenticação"

#: config/SSSDConfig.py:70
msgid "Access control provider"
msgstr "Fornecedor de controle de acesso"

#: config/SSSDConfig.py:71
msgid "Password change provider"
msgstr "Fornecedor de Alteração de Senha"

#: config/SSSDConfig.py:74
msgid "Minimum user ID"
msgstr "ID de utilizador mínimo"

#: config/SSSDConfig.py:75
msgid "Maximum user ID"
msgstr "ID de utilizador máximo"

#: config/SSSDConfig.py:76
msgid "Ping timeout before restarting domain"
msgstr "Duração do ping antes de reiniciar o domínio"

#: config/SSSDConfig.py:77
msgid "Enable enumerating all users/groups"
msgstr "Permitir enumeração de todos os utilizadores/grupos"

#: config/SSSDConfig.py:78
msgid "Cache credentials for offline login"
msgstr "Efectuar cache de credenciais para sessões em modo desligado"

#: config/SSSDConfig.py:79
ms

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Ralf Haferkamp 
Am Donnerstag 18 März 2010 17:15:39 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> >> Ralf Haferkamp wrote:
> >>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
>  On Wed, 17 Mar 2010 15:33:38 +0100
>  
>  Ralf Haferkamp  wrote:
> > Hi,
> > 
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module
> > now issues warning when either the grace counter or the expire
> > counter of the LDAP Ppolicy Control in > 0.
> > 
> > Addtionally I changed the detection for Ppolicy support of the
> > LDAP Server a bit. If the Server returned the Ppolicy Control in
> > the Bind Response  ppolicy support is assumed.
> > 
> > I left the original check for "pwdAttribute" intact, though I
> > think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> > Attribute is usually not part of the user's entry, it's part of
> > the Entry that contains the Policy. Addtionally it might be
> > protected by ACLs and not be returned for anonymous (without
> > losing any functionality).
>  
>  Ralf,
>  patch looks mostly good, but there are some heavy coding style
>  violations.
>  
>  if ( condition ){ is not good, it should be: is (condition) {
>  
>  same for some functions foo( ccc ); is not good, use foo(ccc);
>  
>  Ie, no space in parenthesis, a space only after keywords, and
>  always before the opening {
>  
>  Don't use ++x, but x++ if possible.
>  
>  Also there is at least one place where the return of talloc is
>  not checked.
>  
>  Finally please try to keep the 80 columns limit where possible.
> >>> 
> >>> Updated patch attached. I think I fixed the coding style issues.
> >>> Additionally I just noticed that my orginal patch broke password
> >>> resets via LDAP password policies. This should be fixed now as
> >>> well.
> >>> 
> >>> regards,
> >>> 
> >>>   Ralf
> >> 
> >> +data = talloc_size(pd, 2* sizeof(uint32_t));
> >> +if (*data == NULL) {
> >> +DEBUG(1, ("talloc_size failed.\n"));
> >> +return ENOMEM;
> >> +}
> >> 
> >> 
> >> The returned value check does not look right.
> >> I do not know if there are other places with similar logic.
> > 
> > There weren't (The one above was embarrasing enough. I must have
> > overlooked the compiler warning :| ). Updated patch attached.
> 
> Shouldn't it be freed somewhere down within the same function?
As Sumit already pointed, that will happen when the associated
struct pam_data is talloc_free'd after the response was sent to the 
client.

Ralf
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Sumit Bose 
On Thu, Mar 18, 2010 at 12:15:39PM -0400, Dmitri Pal wrote:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> >   
> >> Ralf Haferkamp wrote:
> >> 
> >>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> >>>   
>  On Wed, 17 Mar 2010 15:33:38 +0100
> 
>  Ralf Haferkamp  wrote:
>  
> > Hi,
> >
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module now
> > issues warning when either the grace counter or the expire counter
> > of the LDAP Ppolicy Control in > 0.
> >
> > Addtionally I changed the detection for Ppolicy support of the
> > LDAP Server a bit. If the Server returned the Ppolicy Control in
> > the Bind Response  ppolicy support is assumed.
> >
> > I left the original check for "pwdAttribute" intact, though I
> > think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> > Attribute is usually not part of the user's entry, it's part of
> > the Entry that contains the Policy. Addtionally it might be
> > protected by ACLs and not be returned for anonymous (without
> > losing any functionality).
> >   
>  Ralf,
>  patch looks mostly good, but there are some heavy coding style
>  violations.
> 
>  if ( condition ){ is not good, it should be: is (condition) {
> 
>  same for some functions foo( ccc ); is not good, use foo(ccc);
> 
>  Ie, no space in parenthesis, a space only after keywords, and
>  always before the opening {
> 
>  Don't use ++x, but x++ if possible.
> 
>  Also there is at least one place where the return of talloc is not
>  checked.
> 
>  Finally please try to keep the 80 columns limit where possible.
>  
> >>> Updated patch attached. I think I fixed the coding style issues.
> >>> Additionally I just noticed that my orginal patch broke password
> >>> resets via LDAP password policies. This should be fixed now as
> >>> well.
> >>>
> >>> regards,
> >>>
> >>>   Ralf
> >>>   
> >> +data = talloc_size(pd, 2* sizeof(uint32_t));
> >> +if (*data == NULL) {
> >> +DEBUG(1, ("talloc_size failed.\n"));
> >> +return ENOMEM;
> >> +}
> >>
> >>
> >> The returned value check does not look right.
> >> I do not know if there are other places with similar logic.
> >> 
> > There weren't (The one above was embarrasing enough. I must have 
> > overlooked the compiler warning :| ). Updated patch attached.
> >   
> 
> Shouldn't it be freed somewhere down within the same function?
> 

No, this data will be send to a client in a later call and freed by a
talloc_free at the end to the client request.

bye,
Sumit

> >   
> >> My eye just caught it when I was scrolling through the patch...
> >> 
> >
> > Ralf
> >   
> > 
> >
> > ___
> > sssd-devel mailing list
> > sssd-devel@lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
> 
> 
> -- 
> Thank you,
> Dmitri Pal
> 
> Engineering Manager IPA project,
> Red Hat Inc.
> 
> 
> ---
> Looking to carve out IT costs?
> www.redhat.com/carveoutcosts/
> 
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Dmitri Pal 
Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
>   
>> Ralf Haferkamp wrote:
>> 
>>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
>>>   
 On Wed, 17 Mar 2010 15:33:38 +0100

 Ralf Haferkamp  wrote:
 
> Hi,
>
> here's another set of enhancements to the LDAP Password Policy
> support in the PAM module and the LDAP backend. The PAM module now
> issues warning when either the grace counter or the expire counter
> of the LDAP Ppolicy Control in > 0.
>
> Addtionally I changed the detection for Ppolicy support of the
> LDAP Server a bit. If the Server returned the Ppolicy Control in
> the Bind Response  ppolicy support is assumed.
>
> I left the original check for "pwdAttribute" intact, though I
> think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> Attribute is usually not part of the user's entry, it's part of
> the Entry that contains the Policy. Addtionally it might be
> protected by ACLs and not be returned for anonymous (without
> losing any functionality).
>   
 Ralf,
 patch looks mostly good, but there are some heavy coding style
 violations.

 if ( condition ){ is not good, it should be: is (condition) {

 same for some functions foo( ccc ); is not good, use foo(ccc);

 Ie, no space in parenthesis, a space only after keywords, and
 always before the opening {

 Don't use ++x, but x++ if possible.

 Also there is at least one place where the return of talloc is not
 checked.

 Finally please try to keep the 80 columns limit where possible.
 
>>> Updated patch attached. I think I fixed the coding style issues.
>>> Additionally I just noticed that my orginal patch broke password
>>> resets via LDAP password policies. This should be fixed now as
>>> well.
>>>
>>> regards,
>>>
>>> Ralf
>>>   
>> +data = talloc_size(pd, 2* sizeof(uint32_t));
>> +if (*data == NULL) {
>> +DEBUG(1, ("talloc_size failed.\n"));
>> +return ENOMEM;
>> +}
>>
>>
>> The returned value check does not look right.
>> I do not know if there are other places with similar logic.
>> 
> There weren't (The one above was embarrasing enough. I must have 
> overlooked the compiler warning :| ). Updated patch attached.
>   

Shouldn't it be freed somewhere down within the same function?

>   
>> My eye just caught it when I was scrolling through the patch...
>> 
>
> Ralf
>   
> 
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Ralf Haferkamp 
Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> >> On Wed, 17 Mar 2010 15:33:38 +0100
> >> 
> >> Ralf Haferkamp  wrote:
> >>> Hi,
> >>> 
> >>> here's another set of enhancements to the LDAP Password Policy
> >>> support in the PAM module and the LDAP backend. The PAM module now
> >>> issues warning when either the grace counter or the expire counter
> >>> of the LDAP Ppolicy Control in > 0.
> >>> 
> >>> Addtionally I changed the detection for Ppolicy support of the
> >>> LDAP Server a bit. If the Server returned the Ppolicy Control in
> >>> the Bind Response  ppolicy support is assumed.
> >>> 
> >>> I left the original check for "pwdAttribute" intact, though I
> >>> think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> >>> Attribute is usually not part of the user's entry, it's part of
> >>> the Entry that contains the Policy. Addtionally it might be
> >>> protected by ACLs and not be returned for anonymous (without
> >>> losing any functionality).
> >> 
> >> Ralf,
> >> patch looks mostly good, but there are some heavy coding style
> >> violations.
> >> 
> >> if ( condition ){ is not good, it should be: is (condition) {
> >> 
> >> same for some functions foo( ccc ); is not good, use foo(ccc);
> >> 
> >> Ie, no space in parenthesis, a space only after keywords, and
> >> always before the opening {
> >> 
> >> Don't use ++x, but x++ if possible.
> >> 
> >> Also there is at least one place where the return of talloc is not
> >> checked.
> >> 
> >> Finally please try to keep the 80 columns limit where possible.
> > 
> > Updated patch attached. I think I fixed the coding style issues.
> > Additionally I just noticed that my orginal patch broke password
> > resets via LDAP password policies. This should be fixed now as
> > well.
> > 
> > regards,
> > 
> > Ralf
> 
> +data = talloc_size(pd, 2* sizeof(uint32_t));
> +if (*data == NULL) {
> +DEBUG(1, ("talloc_size failed.\n"));
> +return ENOMEM;
> +}
> 
> 
> The returned value check does not look right.
> I do not know if there are other places with similar logic.
There weren't (The one above was embarrasing enough. I must have 
overlooked the compiler warning :| ). Updated patch attached.

> My eye just caught it when I was scrolling through the patch...

Ralf
From 1bcc807693212861807849b582bae3bb75e889ca Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp 
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support

Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.

Improved detection if LDAP Password policies are supported by
LDAP Server.
---
 src/providers/ldap/ldap_auth.c |   52 +-
 src/providers/ldap/sdap.h  |5 ++
 src/providers/ldap/sdap_async.h|6 ++-
 src/providers/ldap/sdap_async_connection.c |   53 +++
 src/sss_client/pam_sss.c   |   82 
 src/sss_client/sss_cli.h   |   23 ++---
 6 files changed, 201 insertions(+), 20 deletions(-)

diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..8c77e3a 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
 Sumit Bose 
 
 Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
 return EOK;
 }
 
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+   struct sdap_ppolicy_data *ppolicy,
+   enum sdap_result *result)
+{
+if (ppolicy->grace > 0 || ppolicy->expire > 0) {
+uint32_t *data;
+uint32_t *ptr;
+
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
+
+ptr = data;
+if (ppolicy->grace > 0) {
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+ptr++;
+*ptr = ppolicy->grace;
+} else if (ppolicy->expire > 0) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+ptr++;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
+ (uint8_t*)data);
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
 static errno_t string_to_shadowpw_days(const char *s, long *d)
 {
 long l;
@@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
 struct auth_state *state = tevent_req_data(req,

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Ralf Haferkamp 
Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
[..]
> > 
> > Updated patch attached. I think I fixed the coding style issues.
> > Additionally I just noticed that my orginal patch broke password
> > resets via LDAP password policies. This should be fixed now as
> > well.
> > 
> > regards,
> > 
> > Ralf
> 
> +data = talloc_size(pd, 2* sizeof(uint32_t));
> +if (*data == NULL) {
> +DEBUG(1, ("talloc_size failed.\n"));
> +return ENOMEM;
> +}
> 
> 
> The returned value check does not look right.
> I do not know if there are other places with similar logic.
> My eye just caught it when I was scrolling through the patch...
Oops, thanks for catching that. I'll update the patch once more.

-- 
Ralf
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Dmitri Pal 
Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
>   
>> On Wed, 17 Mar 2010 15:33:38 +0100
>>
>> Ralf Haferkamp  wrote:
>> 
>>> Hi,
>>>
>>> here's another set of enhancements to the LDAP Password Policy
>>> support in the PAM module and the LDAP backend. The PAM module now
>>> issues warning when either the grace counter or the expire counter
>>> of the LDAP Ppolicy Control in > 0.
>>>
>>> Addtionally I changed the detection for Ppolicy support of the LDAP
>>> Server a bit. If the Server returned the Ppolicy Control in the Bind
>>> Response  ppolicy support is assumed.
>>>
>>> I left the original check for "pwdAttribute" intact, though I think
>>> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
>>> usually not part of the user's entry, it's part of the Entry that
>>> contains the Policy. Addtionally it might be protected by ACLs and
>>> not be returned for anonymous (without losing any functionality).
>>>   
>> Ralf,
>> patch looks mostly good, but there are some heavy coding style
>> violations.
>>
>> if ( condition ){ is not good, it should be: is (condition) {
>>
>> same for some functions foo( ccc ); is not good, use foo(ccc);
>>
>> Ie, no space in parenthesis, a space only after keywords, and
>> always before the opening {
>>
>> Don't use ++x, but x++ if possible.
>>
>> Also there is at least one place where the return of talloc is not
>> checked.
>>
>> Finally please try to keep the 80 columns limit where possible.
>> 
>
> Updated patch attached. I think I fixed the coding style issues. 
> Additionally I just noticed that my orginal patch broke password resets 
> via LDAP password policies. This should be fixed now as well.
>
> regards,
>   Ralf
>   

+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (*data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}


The returned value check does not look right.
I do not know if there are other places with similar logic.
My eye just caught it when I was scrolling through the patch... 





> 
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Ralf Haferkamp 
Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> On Wed, 17 Mar 2010 15:33:38 +0100
> 
> Ralf Haferkamp  wrote:
> > Hi,
> > 
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module now
> > issues warning when either the grace counter or the expire counter
> > of the LDAP Ppolicy Control in > 0.
> > 
> > Addtionally I changed the detection for Ppolicy support of the LDAP
> > Server a bit. If the Server returned the Ppolicy Control in the Bind
> > Response  ppolicy support is assumed.
> > 
> > I left the original check for "pwdAttribute" intact, though I think
> > it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> > usually not part of the user's entry, it's part of the Entry that
> > contains the Policy. Addtionally it might be protected by ACLs and
> > not be returned for anonymous (without losing any functionality).
> 
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
> 
> if ( condition ){ is not good, it should be: is (condition) {
> 
> same for some functions foo( ccc ); is not good, use foo(ccc);
> 
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
> 
> Don't use ++x, but x++ if possible.
> 
> Also there is at least one place where the return of talloc is not
> checked.
> 
> Finally please try to keep the 80 columns limit where possible.

Updated patch attached. I think I fixed the coding style issues. 
Additionally I just noticed that my orginal patch broke password resets 
via LDAP password policies. This should be fixed now as well.

regards,
Ralf
From 0b1f123f20f4944425d70b02bb346b78176d0abe Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp 
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support

Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.

Improved detection if LDAP Password policies are supported by
LDAP Server.
---
 src/providers/ldap/ldap_auth.c |   52 +-
 src/providers/ldap/sdap.h  |5 ++
 src/providers/ldap/sdap_async.h|6 ++-
 src/providers/ldap/sdap_async_connection.c |   53 +++
 src/sss_client/pam_sss.c   |   82 
 src/sss_client/sss_cli.h   |   23 ++---
 6 files changed, 201 insertions(+), 20 deletions(-)

diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..b572bf2 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
 Sumit Bose 
 
 Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
 return EOK;
 }
 
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+   struct sdap_ppolicy_data *ppolicy,
+   enum sdap_result *result)
+{
+if (ppolicy->grace > 0 || ppolicy->expire > 0) {
+uint32_t *data;
+uint32_t *ptr;
+
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (*data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
+
+ptr = data;
+if (ppolicy->grace > 0) {
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+ptr++;
+*ptr = ppolicy->grace;
+} else if (ppolicy->expire > 0) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+ptr++;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
+ (uint8_t*)data);
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
 static errno_t string_to_shadowpw_days(const char *s, long *d)
 {
 long l;
@@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
 struct auth_state *state = tevent_req_data(req,
 struct auth_state);
 int ret;
-
-ret = sdap_auth_recv(subreq, &state->result);
+struct sdap_ppolicy_data *ppolicy;
+
+ret = sdap_auth_recv(subreq, state, &state->result, &ppolicy);
+if (ppolicy != NULL) {
+DEBUG(9,("Found ppolicy data, "
+ "assuming LDAP password policies are active.\n"));
+state->pw_expire_type = PWEXPIRE_LDAP_PASSWORD_POLICY;
+state->pw_expire_data = ppolicy;
+}
 talloc_zfree(subreq);
 if (ret) {
 tevent_req_error(req, ret);
@@ -960,6 +1001,13 @@ static void sdap_pam_auth_done(struct tevent_req *req)
 }
 break;
 case PWEXPI

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/18/2010 07:42 AM, Simo Sorce wrote:
> On Wed, 17 Mar 2010 15:33:38 +0100
> Ralf Haferkamp  wrote:
> 
>> Hi,
>>
>> here's another set of enhancements to the LDAP Password Policy
>> support in the PAM module and the LDAP backend. The PAM module now
>> issues warning when either the grace counter or the expire counter of
>> the LDAP Ppolicy Control in > 0. 
>>
>> Addtionally I changed the detection for Ppolicy support of the LDAP 
>> Server a bit. If the Server returned the Ppolicy Control in the Bind 
>> Response  ppolicy support is assumed.
>>
>> I left the original check for "pwdAttribute" intact, though I think
>> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
>> usually not part of the user's entry, it's part of the Entry that
>> contains the Policy. Addtionally it might be protected by ACLs and
>> not be returned for anonymous (without losing any functionality).
>>
> 
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
> 
> if ( condition ){ is not good, it should be: is (condition) {
> 
> same for some functions foo( ccc ); is not good, use foo(ccc);
> 
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
> 
> Don't use ++x, but x++ if possible.
> 
> Also there is at least one place where the return of talloc is not
> checked.
> 
> Finally please try to keep the 80 columns limit where possible.
> 
> 
> Simo.
> 
Ralf, for details, please see our style guide at
http://www.freeipa.org/page/Coding_Style

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkuiJCAACgkQeiVVYja6o6MW3QCeImzq9lNDhilHdYqSWXdeoXSX
hEAAnioECFUSxvbn6eb1jYTs4Pn5jTgi
=qGfe
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support

2010-03-18 Thread Simo Sorce 
On Wed, 17 Mar 2010 15:33:38 +0100
Ralf Haferkamp  wrote:

> Hi,
> 
> here's another set of enhancements to the LDAP Password Policy
> support in the PAM module and the LDAP backend. The PAM module now
> issues warning when either the grace counter or the expire counter of
> the LDAP Ppolicy Control in > 0. 
> 
> Addtionally I changed the detection for Ppolicy support of the LDAP 
> Server a bit. If the Server returned the Ppolicy Control in the Bind 
> Response  ppolicy support is assumed.
> 
> I left the original check for "pwdAttribute" intact, though I think
> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> usually not part of the user's entry, it's part of the Entry that
> contains the Policy. Addtionally it might be protected by ACLs and
> not be returned for anonymous (without losing any functionality).
> 

Ralf,
patch looks mostly good, but there are some heavy coding style
violations.

if ( condition ){ is not good, it should be: is (condition) {

same for some functions foo( ccc ); is not good, use foo(ccc);

Ie, no space in parenthesis, a space only after keywords, and
always before the opening {

Don't use ++x, but x++ if possible.

Also there is at least one place where the return of talloc is not
checked.

Finally please try to keep the 80 columns limit where possible.


Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel