[PATCHES] PAC responder and client

[Jan Zelený]

Sumit sent me his set of patches last Friday. I've done some merges, filled 
commit messages and other cosmetic adjustments. I've also done initial review 
and I'm sending my comments:

get_rid() doesn't need to be in the header file, it can be static in the module

Defining idmap_talloc() and idmap_free() twice, please remove one of them.

The condition if (pac_ctx->idmap_ctx == NULL) in domsid_rid_to_uid() will be 
always false, won't it? If it was NULL, the entire responder would fail way 
earlier.

Missing debug_level initialization in main() in PAC responder

pac_save_memberships_next() is missing static in its definition (declaration is 
ok)

pac_save_memberships_recv() is missing static

you are not assigning the result to *_pwd in get_pwd_from_pac()

Is the TODO in save_pac_user() valid and important? Do we need to address this 
before pushing the patch set?

Nitpick: please use initialization instead of "g = 0;" in get_gids_from_pac()
Typo: Remote users are members ... (comment in pac_save_memberships_send())

Please remove the sysdb_search_group_by_gid() call in 
pac_save_memberships_next(), it is redundant there and can be replaced by 
checking the return code after calling pac_store_memberships().

You are not freeing pr_ctx in some error cases. The only place pr_ctx is 
free'd is pac_save_memberships_done(), i.e. only when everything went ok.

Please add some talloc_free() calls where necessary. My first guess is that 
pac_get_domains_done() and pac_add_pac_user() are the only two places where it 
is necessary.

In function local_sid_to_id(): I think another check should be in place:
if (rid-remote_min+local_min > local_max) return ERANGE;

Sumit, please contact me if you need help with anything.

Thanks
Jan

[Sumit Bose]

On Mon, Jun 11, 2012 at 01:04:14PM +0200, Jan Zelený wrote:
> Sumit sent me his set of patches last Friday. I've done some merges, filled 
> commit messages and other cosmetic adjustments. I've also done initial review 
> and I'm sending my comments:
>
> get_rid() doesn't need to be in the header file, it can be static in the module

done
> Defining idmap_talloc() and idmap_free() twice, please remove one of them.

done
> The condition if (pac_ctx->idmap_ctx == NULL) in domsid_rid_to_uid() will be 
> always false, won't it? If it was NULL, the entire responder would fail way 
> earlier.

done
> Missing debug_level initialization in main() in PAC responder

done
> pac_save_memberships_next() is missing static in its definition (declaration is 
> ok)

done
> pac_save_memberships_recv() is missing static

done
> you are not assigning the result to *_pwd in get_pwd_from_pac()

done
> Is the TODO in save_pac_user() valid and important? Do we need to address this 
> before pushing the patch set?

It is a sanity check. I will prepare a separate patch for this.
> Nitpick: please use initialization instead of "g = 0;" in get_gids_from_pac()
> Typo: Remote users are members ... (comment in pac_save_memberships_send())

both done
> Please remove the sysdb_search_group_by_gid() call in 
> pac_save_memberships_next(), it is redundant there and can be replaced by 
> checking the return code after calling pac_store_memberships().

done
> You are not freeing pr_ctx in some error cases. The only place pr_ctx is 
> free'd is pac_save_memberships_done(), i.e. only when everything went ok.
>
> Please add some talloc_free() calls where necessary. My first guess is that 
> pac_get_domains_done() and pac_add_pac_user() are the only two places where it 
> is necessary.

I have added a talloc_free() at both places. But since pr_ctx is a child
of the client context which is freed shortly afterwards one might argue
if the talloc_free() is even needed in pac_save_memberships_done(). But
I would agree with you that it is a good idea to remove pr_ctx when it
is not needed anymore.
> In function local_sid_to_id(): I think another check should be in place:
> if (rid-remote_min+local_min > local_max) return ERANGE;

I added a slightly different check which would also discover overruns.
> Sumit, please contact me if you need help with anything.

Thank you very much for the review and for helping with the patches.

bye,
Sumit
> Thanks
> Jan
> From 72fbc8a3358fc0b6394cdf59f9e0e9bb3fd91bf4 Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Fri, 13 May 2011 18:44:15 +0200
> Subject: [PATCH 01/10] PAC responder: add basic infrastructure
>
> This adds only the basic outline of the PAC responder, it won't support
> any operations, it will just start and initialize itself.
>
> Makefile.am                    |   21 ++++
>  configure.ac                   |    1 +
>  src/confdb/confdb.h            |    3 +
>  src/external/pac_responder.m4  |   35 ++++++
>  src/monitor/monitor.c          |    2 +-
>  src/responder/pac/pacsrv.c     |  223 ++++++++++++++++++++++++++++++++++++++++
>  src/responder/pac/pacsrv.h     |   52 +++++++++
>  src/responder/pac/pacsrv_cmd.c |   61 +++++++++++
>  8 files changed, 397 insertions(+), 1 deletions(-)
>  create mode 100644 src/external/pac_responder.m4
>  create mode 100644 src/responder/pac/pacsrv.c
>  create mode 100644 src/responder/pac/pacsrv.h
>  create mode 100644 src/responder/pac/pacsrv_cmd.c
>
> diff --git a/Makefile.am b/Makefile.am
> index a6c2f909846011e81270e3f99e771e3f1c11bedb..88863d0324c2dd719414d34d1cc18f475f94c4a0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -97,6 +97,10 @@ if BUILD_SSH
>  sssdlibexec_PROGRAMS += sssd_ssh
>  endif
>
> +if BUILD_PAC_RESPONDER
> sssdlibexec_PROGRAMS += sssd_pac
>
> +endif
>
> if HAVE_CHECK
>      non_interactive_check_based_tests = \
>          sysdb-tests \
> @@ -235,6 +239,7 @@ AM_CPPFLAGS = \
>      -DSSS_NSS_MCACHE_DIR="$(mcpath)" \
>      -DSSS_NSS_SOCKET_NAME="$(pipepath)/nss" \
>      -DSSS_PAM_SOCKET_NAME="$(pipepath)/pam" \
> -DSSS_PAC_SOCKET_NAME="$(pipepath)/pac" \
> -DSSS_PAM_PRIV_SOCKET_NAME="$(pipepath)/private/pam" \
> -DSSS_SUDO_SOCKET_NAME="$(pipepath)/sudo" \
> -DSSS_AUTOFS_SOCKET_NAME="$(pipepath)/autofs" \
>
> @@ -348,6 +353,7 @@ dist_noinst_HEADERS = \
>      src/responder/nss/nsssrv_netgroup.h \
>      src/responder/nss/nsssrv_services.h \
>      src/responder/nss/nsssrv_mmap_cache.h \
> src/responder/pac/pacsrv.h \
> src/responder/common/negcache.h \
> src/responder/sudo/sudosrv_private.h \
> src/responder/autofs/autofs_private.h \
>
> @@ -580,6 +586,21 @@ sssd_ssh_LDADD = \
>      libsss_util.la
>  endif
>
> +sssd_pac_SOURCES = \
> src/responder/pac/pacsrv.c \
> src/responder/pac/pacsrv_cmd.c \
> $(SSSD_UTIL_OBJ) \
> $(SSSD_RESPONDER_OBJ)
>
> +sssd_pac_CFLAGS = \
> $(AM_CFLAGS) \
> $(NDR_KRB5PAC_CFLAGS)
>
> +sssd_pac_LDADD = \
> $(NDR_KRB5PAC_LIBS) \
> $(TDB_LIBS) \
> $(SSSD_LIBS) \
> libsss_idmap.la \
> libsss_util.la
>
> sssd_be_SOURCES = \
>      src/providers/data_provider_be.c \
>      src/providers/data_provider_fo.c \
> diff --git a/configure.ac b/configure.ac
> index d4647d8a0ad8e7e2555b97b13acee114e9a81a65..0dee070baf35d6ec37e6bf348159d884bdd2450a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -124,6 +124,7 @@ m4_include([src/external/nsupdate.m4])
>  m4_include([src/external/libkeyutils.m4])
>  m4_include([src/external/libnl.m4])
>  m4_include([src/external/systemd.m4])
> +m4_include([src/external/pac_responder.m4])
>  m4_include([src/util/signal.m4])
>
> WITH_UNICODE_LIB
> diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
> index 5893897f9e85efb579d2996643098efb38fe6da9..8a69d960eb38eb95f58505d29f6c7dd5bffdf808 100644
> --- a/src/confdb/confdb.h
> +++ b/src/confdb/confdb.h
> @@ -115,6 +115,9 @@
>  #define CONFDB_SSH_HASH_KNOWN_HOSTS "ssh_hash_known_hosts"
>  #define CONFDB_DEFAULT_SSH_HASH_KNOWN_HOSTS true
>
> +/* PAC */
> +#define CONFDB_PAC_CONF_ENTRY "config/pac"
>
> /* Data Provider */
>  #define CONFDB_DP_CONF_ENTRY "config/dp"
>
> diff --git a/src/external/pac_responder.m4 b/src/external/pac_responder.m4
> new file mode 100644
> index 0000000000000000000000000000000000000000..fbad267e2e1463312ee6c97e35490a9f2bc5301c
> --- /dev/null
> +++ b/src/external/pac_responder.m4
> @@ -0,0 +1,35 @@
> +AC_SUBST(NDR_KRB5PAC_CFLAGS)
> +AC_SUBST(NDR_KRB5PAC_LIBS)
>
> +AC_ARG_ENABLE([experimental-pac-responder],
>          [AS_HELP_STRING([--enable-experimental-pac-responder],
>                          [build experimental pac responder])],
>          [build_pac_responder=$enableval],
>          [build_pac_responder=no])
>
> +if test x$build_all_experimental_features != xno
> +then
> build_pac_responder=yes
>
> +fi
>
> +if test x$build_pac_responder == xyes
> +then
> PKG_CHECK_MODULES(NDR_KRB5PAC, ndr_krb5pac,,
>    AC_MSG_ERROR([Cannot build pac responder without libndr_krb5pac]))
> AC_PATH_PROG(KRB5_CONFIG, krb5-config)
> AC_MSG_CHECKING(for supported MIT krb5 version)
> KRB5_VERSION="`$KRB5_CONFIG --version`"
> case $KRB5_VERSION in
>    Kerberos\ 5\ release\ 1.9* | \
>    Kerberos\ 5\ release\ 1.10*)
>        AC_MSG_RESULT(yes)
>        ;;
>    *)
>        AC_MSG_ERROR([Cannot build authdata plugin with this version of
>                      MIT Kerberos, please use 1.9.x])
> esac
>
> +fi
>
> +AM_CONDITIONAL([BUILD_PAC_RESPONDER], [test x$build_pac_responder = xyes ])
>
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> index 041f576e686f298d6eaa7610b1b2813fa3096744..3df1b10d301ec542f97eb912e1517d6df674aa80 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -759,7 +759,7 @@ static int check_local_domain_unique(struct sss_domain_info *domains)
>  static char *check_services(char **services)
>  {
>      const char *known_services[] = { "nss", "pam", "sudo", "autofs", "ssh",
>                                 NULL };
>                                 "pac", NULL };
> int i;
>    int ii;
>
> diff --git a/src/responder/pac/pacsrv.c b/src/responder/pac/pacsrv.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..fda4079fc0be4cb13946a9493befc904bc07cdd7
> --- /dev/null
> +++ b/src/responder/pac/pacsrv.c
> @@ -0,0 +1,223 @@
> +/*
> SSSD
> PAC Responder
> Copyright (C) Sumit Bose  2011
> 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
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <string.h>
> +#include <sys/time.h>
> +#include <errno.h>
>
> +#include "popt.h"
> +#include "util/util.h"
> +#include "responder/pac/pacsrv.h"
> +#include "db/sysdb.h"
> +#include "confdb/confdb.h"
> +#include "dbus/dbus.h"
> +#include "sbus/sssd_dbus.h"
> +#include "responder/common/responder_packet.h"
> +#include "responder/common/responder.h"
> +#include "providers/data_provider.h"
> +#include "monitor/monitor_interfaces.h"
> +#include "sbus/sbus_client.h"
>
> +#define SSS_PAC_PIPE_NAME "pac"
>
> +struct sbus_method monitor_pac_methods[] = {
> { MON_CLI_METHOD_PING, monitor_common_pong },
> { MON_CLI_METHOD_RES_INIT, monitor_common_res_init },
> { MON_CLI_METHOD_ROTATE, monitor_common_rotate_logs },
> { NULL, NULL }
>
> +};
>
> +struct sbus_interface monitor_pac_interface = {
> MONITOR_INTERFACE,
> MONITOR_PATH,
> SBUS_DEFAULT_VTABLE,
> monitor_pac_methods,
> NULL
>
> +};
>
> +static struct sbus_method pac_dp_methods[] = {
> { NULL, NULL }
>
> +};
>
> +struct sbus_interface pac_dp_interface = {
> DP_INTERFACE,
> DP_PATH,
> SBUS_DEFAULT_VTABLE,
> pac_dp_methods,
> NULL
>
> +};
>
> +/* TODO: check if this can be made generic for all responders */
> +static void pac_dp_reconnect_init(struct sbus_connection *conn,
>                              int status, void *pvt)
>
> +{
> struct be_conn *be_conn = talloc_get_type(pvt, struct be_conn);
> int ret;
> /* Did we reconnect successfully? */
> if (status == SBUS_RECONNECT_SUCCESS) {
>    DEBUG(SSSDBG_OP_FAILURE, ("Reconnected to the Data Provider.\n"));
>    /* Identify ourselves to the data provider */
>    ret = dp_common_send_id(be_conn->conn,
>                            DATA_PROVIDER_VERSION,
>                            "PAC");
>    /* all fine */
>    if (ret == EOK) {
>        handle_requests_after_reconnect(be_conn->rctx);
>        return;
>    }
> }
> /* Failed to reconnect */
> DEBUG(SSSDBG_FATAL_FAILURE, ("Could not reconnect to %s provider.\n",
>          be_conn->domain->name));
> /* FIXME: kill the frontend and let the monitor restart it ? */
> /* nss_shutdown(rctx); */
>
> +}
>
> +static void *idmap_talloc(size_t size, void *pvt)
> +{
> return talloc_size(pvt, size);
>
> +}
>
> +static void idmap_free(void *ptr, void *pvt)
> +{
> talloc_free(ptr);
>
> +}
>
> +int pac_process_init(TALLOC_CTX *mem_ctx,
>                 struct tevent_context *ev,
>                 struct confdb_ctx *cdb)
>
> +{
> struct sss_cmd_table *pac_cmds;
> struct be_conn *iter;
> struct pac_ctx *pac_ctx;
> int ret, max_retries;
> enum idmap_error_code err;
> pac_ctx = talloc_zero(mem_ctx, struct pac_ctx);
> if (!pac_ctx) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error initializing pac_ctx\n"));
>    return ENOMEM;
> }
> pac_cmds = get_pac_cmds();
> ret = sss_process_init(pac_ctx, ev, cdb,
>                       pac_cmds,
>                       SSS_PAC_SOCKET_NAME, NULL,
>                       CONFDB_PAC_CONF_ENTRY,
>                       PAC_SBUS_SERVICE_NAME,
>                       PAC_SBUS_SERVICE_VERSION,
>                       &monitor_pac_interface,
>                       "PAC", &pac_dp_interface,
>                       &pac_ctx->rctx);
> if (ret != EOK) {
>    return ret;
> }
> pac_ctx->rctx->pvt_ctx = pac_ctx;
> /* Enable automatic reconnection to the Data Provider */
> ret = confdb_get_int(pac_ctx->rctx->cdb,
>                     CONFDB_PAC_CONF_ENTRY,
>                     CONFDB_SERVICE_RECON_RETRIES,
>                     3, &max_retries);
> if (ret != EOK) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to set up automatic reconnection\n"));
>    return ret;
> }
> for (iter = pac_ctx->rctx->be_conns; iter; iter = iter->next) {
>    sbus_reconnect_init(iter->conn, max_retries,
>                        pac_dp_reconnect_init, iter);
> }
> err = sss_idmap_init(idmap_talloc, pac_ctx, idmap_free,
>                     &pac_ctx->idmap_ctx);
> if (err != IDMAP_SUCCESS) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("sss_idmap_init failed.\n"));
>    return EFAULT;
> }
> DEBUG(SSSDBG_TRACE_FUNC, ("PAC Initialization complete\n"));
> return EOK;
>
> +}
>
> +int main(int argc, const char *argv[])
> +{
> int opt;
> poptContext pc;
> struct main_context *main_ctx;
> int ret;
> struct poptOption long_options[] = {
>    POPT_AUTOHELP
>    SSSD_MAIN_OPTS
>    POPT_TABLEEND
> };
> pc = poptGetContext(argv[0], argc, argv, long_options, 0);
> while((opt = poptGetNextOpt(pc)) != -1) {
>    switch(opt) {
>    default:
>        fprintf(stderr, "\nInvalid option %s: %s\n\n",
>              poptBadOption(pc, 0), poptStrerror(opt));
>        poptPrintUsage(pc, stderr, 0);
>        return 1;
>    }
> }
> poptFreeContext(pc);
> /* set up things like debug, signals, daemonization, etc... */
> debug_log_file = "sssd_pac";
> ret = server_setup("sssd[pac]", 0, CONFDB_PAC_CONF_ENTRY, &main_ctx);
> if (ret != EOK) return 2;
> ret = die_if_parent_died();
> if (ret != EOK) {
>    /* This is not fatal, don't return */
>    DEBUG(SSSDBG_OP_FAILURE, ("Could not set up to exit when parent process does\n"));
> }
> ret = pac_process_init(main_ctx,
>                       main_ctx->event_ctx,
>                       main_ctx->confdb_ctx);
> if (ret != EOK) return 3;
> /* loop on main */
> server_loop(main_ctx);
> return 0;
>
> +}
>
> diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..0dfe7f9ee3a11fd4b02133086b3dde2b8d55abc4
> --- /dev/null
> +++ b/src/responder/pac/pacsrv.h
> @@ -0,0 +1,52 @@
> +/*
> SSSD
> PAC Responder, header file
> Copyright (C) Sumit Bose  2011
> 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
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
>
> +#ifndef __PACSRV_H__
> +#define __PACSRV_H__
>
> +#include <stdint.h>
> +#include <sys/un.h>
> +#include "config.h"
> +#include "talloc.h"
> +#include "tevent.h"
> +#include "ldb.h"
> +#include "dbus/dbus.h"
> +#include "sbus/sssd_dbus.h"
> +#include "responder/common/responder_packet.h"
> +#include "responder/common/responder.h"
> +#include "lib/idmap/sss_idmap.h"
>
> +#define PAC_SBUS_SERVICE_VERSION 0x0001
> +#define PAC_SBUS_SERVICE_NAME "pac"
>
> +#define PAC_PACKET_MAX_RECV_SIZE 1024
>
> +struct getent_ctx;
>
> +struct pac_ctx {
> struct resp_ctx *rctx;
>
> +};
>
> +int pac_cmd_execute(struct cli_ctx *cctx);
>
> +struct sss_cmd_table *get_pac_cmds(void);
>
> +#endif /* __PACSRV_H__ */
> diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..892ef85688889d33fe2461a9f1aacf20f1ed3252
> --- /dev/null
> +++ b/src/responder/pac/pacsrv_cmd.c
> @@ -0,0 +1,61 @@
> +/*
> SSSD
> PAC Responder
> Copyright (C) Sumit Bose  2012
>             Jan Zeleny <[e-mail]> 2012
> 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
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
>
> +#include "util/util.h"
> +#include "responder/pac/pacsrv.h"
> +#include "confdb/confdb.h"
> +#include "db/sysdb.h"
>
> +struct cli_protocol_version *register_cli_protocol_version(void)
> +{
> static struct cli_protocol_version pac_cli_protocol_version[] = {
>    {1, "2011-04-12", "initial version"},
>    {0, NULL, NULL}
> };
> return pac_cli_protocol_version;
>
> +}
>
> +static struct sss_cmd_table pac_cmds[] = {
> {SSS_GET_VERSION, sss_cmd_get_version},
> {SSS_CLI_NULL, NULL}
>
> +};
>
> +struct sss_cmd_table *get_pac_cmds(void) {
> return pac_cmds;
>
> +}
>
> +int pac_cmd_execute(struct cli_ctx *cctx)
> +{
> enum sss_cli_command cmd;
> int i;
> cmd = sss_packet_get_cmd(cctx->creq->in);
> for (i = 0; pac_cmds[i].cmd != SSS_CLI_NULL; i++) {
>    if (cmd == pac_cmds[i].cmd) {
>        return pac_cmds[i].fn(cctx);
>    }
> }
> return EINVAL;
> +}
>
> 1.7.7.6
> From 0752cbefe5da0197650fed88a8ec44cd10c50ecd Mon Sep 17 00:00:00 2001
> From: Jan Zeleny 
> Date: Mon, 11 Jun 2012 06:03:45 -0400
> Subject: [PATCH 02/10] PAC responder: add some utility functions
>
> Makefile.am                      |    1 +
>  src/responder/pac/pacsrv.h       |   19 +++++++++++
>  src/responder/pac/pacsrv_utils.c |   66 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100644 src/responder/pac/pacsrv_utils.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 88863d0324c2dd719414d34d1cc18f475f94c4a0..8212070f1050147635e07c3dbeb7dc8dea766d6c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -589,6 +589,7 @@ endif
>  sssd_pac_SOURCES = \
>      src/responder/pac/pacsrv.c \
>      src/responder/pac/pacsrv_cmd.c \
> src/responder/pac/pacsrv_utils.c \
> $(SSSD_UTIL_OBJ) \
> $(SSSD_RESPONDER_OBJ)
>
> sssd_pac_CFLAGS = \
> diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
> index 0dfe7f9ee3a11fd4b02133086b3dde2b8d55abc4..1400074fc0630dc4231def395538dd99da41bf3d 100644
> --- a/src/responder/pac/pacsrv.h
> +++ b/src/responder/pac/pacsrv.h
> @@ -40,13 +40,32 @@
>  #define PAC_PACKET_MAX_RECV_SIZE 1024
>
> struct getent_ctx;
> +struct dom_sid;
>
> struct pac_ctx {
>      struct resp_ctx *rctx;
> struct sss_idmap_ctx *idmap_ctx;
> struct dom_sid *my_dom_sid;
> struct local_mapping_ranges *range_map;
>
> +};
>
> +struct range {
> uint32_t min;
> uint32_t max;
>
> +};
>
> +struct local_mapping_ranges {
> struct range local_ids;
> struct range primary_rids;
> struct range secondary_rids;
>
> };
>
> int pac_cmd_execute(struct cli_ctx *cctx);
>
> struct sss_cmd_table *get_pac_cmds(void);
>
> +errno_t get_rid(struct dom_sid *sid, uint32_t *rid);
>
> +errno_t local_sid_to_id(struct local_mapping_ranges *map, struct dom_sid *sid,
>                    uint32_t *id);
>
> #endif /* __PACSRV_H__ */
> diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..9f2932be9a7ec2c2560031f8cdddd3f38af95b2f
> --- /dev/null
> +++ b/src/responder/pac/pacsrv_utils.c
> @@ -0,0 +1,66 @@
> +/*
> SSSD
> PAC Responder - utility finctions
> Copyright (C) Sumit Bose  2012
> 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
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
> +#include <stdbool.h>
> +#include <util/data_blob.h>
> +#include <gen_ndr/security.h>
>
> +#include "util/util.h"
> +#include "responder/pac/pacsrv.h"
>
> +errno_t get_rid(struct dom_sid *sid, uint32_t *rid)
> +{
> if (sid == NULL || sid->num_auths < 1 || rid == NULL) {
>    return EINVAL;
> }
> *rid = sid->sub_auths[sid->num_auths - 1];
> return EOK;
>
> +}
>
> +errno_t local_sid_to_id(struct local_mapping_ranges *map, struct dom_sid *sid,
>                    uint32_t *id)
>
> +{
> int ret;
> uint32_t rid;
> if (map == NULL || sid == NULL || id == NULL) {
>    return EINVAL;
> }
> ret = get_rid(sid, &rid);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("get_rid failed.\n"));
>    return ret;
> }
> if (rid >= map->primary_rids.min && rid <= map->primary_rids.max) {
>    *id = map->local_ids.min + (rid -  map->primary_rids.min);
> } else if (rid >= map->secondary_rids.min &&
>           rid <= map->secondary_rids.max) {
>    *id = map->local_ids.min + (rid -  map->secondary_rids.min);
> } else {
>    return ENOENT;
> }
> return EOK;
>
> +}
>
> -- 
> 1.7.7.6
> From 2dd79ab2c11c81d652b4a16b8ac818a079cce8fb Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Wed, 18 May 2011 13:28:17 +0200
> Subject: [PATCH 03/10] PAC responder: add the core functionality
>
> This adds support for parsing PAC and storing information contained
> within. In particular the user and all his memberships are stored. In
> case it is necessary, getgrgid() requests are sent to provider for group
> resolution.
>
> src/responder/pac/pacsrv.c     |    2 +-
>  src/responder/pac/pacsrv_cmd.c |  895 ++++++++++++++++++++++++++++++++++++++++
>  src/sss_client/sss_cli.h       |    3 +
>  3 files changed, 899 insertions(+), 1 deletions(-)
>
> diff --git a/src/responder/pac/pacsrv.c b/src/responder/pac/pacsrv.c
> index fda4079fc0be4cb13946a9493befc904bc07cdd7..2e1aa881e32c48e64f4ec36a1e668db5409f9924 100644
> --- a/src/responder/pac/pacsrv.c
> +++ b/src/responder/pac/pacsrv.c
> @@ -48,7 +48,7 @@
>  struct sbus_method monitor_pac_methods[] = {
>      { MON_CLI_METHOD_PING, monitor_common_pong },
>      { MON_CLI_METHOD_RES_INIT, monitor_common_res_init },
> { MON_CLI_METHOD_ROTATE, monitor_common_rotate_logs },
> { MON_CLI_METHOD_ROTATE, responder_logrotate },
> { NULL, NULL }
>
> };
>
> diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
> index 892ef85688889d33fe2461a9f1aacf20f1ed3252..e314e520a105a976b88ec12d84b9cc2c14239469 100644
> --- a/src/responder/pac/pacsrv_cmd.c
> +++ b/src/responder/pac/pacsrv_cmd.c
> @@ -19,12 +19,906 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see http://www.gnu.org/licenses/.
>  */
> +#include <stdbool.h>
> +#include <util/data_blob.h>
> +#include <ndr.h>
> +#include <gen_ndr/krb5pac.h>
> +#include <gen_ndr/ndr_krb5pac.h>
>
> #include "util/util.h"
> +#include "util/sss_nss.h"
>  #include "responder/pac/pacsrv.h"
>  #include "confdb/confdb.h"
>  #include "db/sysdb.h"
>
> +#define PAC_USER_OFFSET 200000
> +#define PAC_HOME_PATH "/home/"
> +#define PAC_DEFAULT_SHELL "/bin/bash"
>
> +static errno_t pac_cmd_done(struct cli_ctx *cctx, int cmd_ret)
> +{
> int ret;
> if (cmd_ret == EAGAIN) {
>    /* async processing, just return here */
>    return EOK;
> }
> ret = sss_packet_new(cctx->creq, 0, sss_packet_get_cmd(cctx->creq->in),
>                     &cctx->creq->out);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("sss_packet_new failed [%d][%s].\n",
>                              ret, strerror(ret)));
>    return ret;
> }
> sss_packet_set_error(cctx->creq->out, cmd_ret);
> sss_cmd_done(cctx, NULL);
> return EOK;
>
> +}
>
> +static void *idmap_talloc(size_t size, void *pvt)
> +{
> return talloc_size(pvt, size);
>
> +}
>
> +static void idmap_free(void *ptr, void *pvt)
> +{
> talloc_free(ptr);
>
> +}
>
> +static errno_t add_idmap_domain(struct sss_idmap_ctx *idmap_ctx,
>                            struct sysdb_ctx *sysdb,
>                            const char *domain_name,
>                            const char *dom_sid_str)
>
> +{
> struct sss_idmap_range range;
> enum idmap_error_code err;
> /* TODO: read range form sysdb if
> * https://fedorahosted.org/freeipa/ticket/2185 is fixed */
> range.min = 200000;
> range.max = 400000;
> err = sss_idmap_add_domain(idmap_ctx, domain_name, dom_sid_str, &range);
> if (err != IDMAP_SUCCESS) {
>    DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_add_domain failed.\n"));
>    return EFAULT;
> }
> return EOK;
>
> +}
>
> +static errno_t domsid_rid_to_uid(struct pac_ctx *pac_ctx,
>                             struct sysdb_ctx *sysdb,
>                             const char *domain_name,
>                             struct dom_sid2 *domsid, uint32_t rid,
>                             uid_t *uid)
>
> +{
> enum idmap_error_code err;
> char *sid_str = NULL;
> char *dom_sid_str = NULL;
> uint32_t id;
> int ret;
> if (pac_ctx->idmap_ctx == NULL) {
>    err = sss_idmap_init(idmap_talloc, pac_ctx, idmap_free,
>                         &pac_ctx->idmap_ctx);
>    if (err != IDMAP_SUCCESS) {
>        DEBUG(SSSDBG_FATAL_FAILURE, ("sss_idmap_init failed.\n"));
>        return EFAULT;
>    }
> }
> err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, domsid,
>                               &dom_sid_str);
> if (err != IDMAP_SUCCESS) {
>    DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_smb_sid_to_sid failed.\n"));
>    ret = EFAULT;
>    goto done;
> }
> sid_str = talloc_asprintf(NULL, "%s-%lu", dom_sid_str, (unsigned long) rid);
> if (sid_str == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("dom_sid_and_rid_string failed.\n"));
>    return ENOMEM;
> }
> err = sss_idmap_smb_sid_to_sid(pac_ctx->idmap_ctx, domsid, &sid_str);
> if (err != IDMAP_SUCCESS) {
>    DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_smb_sid_to_sid failed.\n"));
>    ret = EFAULT;
>    goto done;
> }
> err = sss_idmap_sid_to_unix(pac_ctx->idmap_ctx, sid_str, &id);
> if (err == IDMAP_NO_DOMAIN) {
>    ret = add_idmap_domain(pac_ctx->idmap_ctx, sysdb, domain_name,
>                           dom_sid_str);
>    if (ret != EOK) {
>        DEBUG(SSSDBG_OP_FAILURE, ("add_idmap_domain failed.\n"));
>        goto done;
>    }
>    err = sss_idmap_sid_to_unix(pac_ctx->idmap_ctx, sid_str, &id);
>    if (err != IDMAP_SUCCESS) {
>        DEBUG(SSSDBG_FATAL_FAILURE, ("sss_idmap_sid_to_unix failed "
>                                     "even in the second attempt.\n"));
>        ret = ENOENT;
>        goto done;
>    }
> } else if (err != IDMAP_SUCCESS && err != IDMAP_NO_DOMAIN) {
>    DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_sid_to_unix failed.\n"));
>    ret = EFAULT;
>    goto done;
> }
> *uid = (uid_t) id;
> ret = EOK;
>
> +done:
> talloc_free(dom_sid_str);
> talloc_free(sid_str);
> return ret;
>
> +}
>
> +static errno_t get_my_domain_sid(struct pac_ctx *pac_ctx,
>                             struct sss_domain_info *dom,
>                             struct dom_sid **_sid)
>
> +{
> struct sysdb_ctx *sysdb;
> int ret;
> struct ldb_dn *basedn;
> const char *attrs[] = {SYSDB_SUBDOMAIN_ID,
>                       NULL};
> size_t msgs_count;
> const char *sid_str;
> struct ldb_message **msgs;
> TALLOC_CTX *tmp_ctx = NULL;
> struct dom_sid *sid = NULL;
> char *dom_name;
> enum idmap_error_code err;
> if (pac_ctx->my_dom_sid == NULL) {
>    if (dom->parent != NULL) {
>        sysdb = dom->parent->sysdb;
>        dom_name = dom->parent->name;
>    } else {
>        sysdb = dom->sysdb;
>        dom_name = dom->name;
>    }
>    if (sysdb == NULL) {
>        DEBUG(SSSDBG_FATAL_FAILURE, ("Missing sysdb context.\n"));
>        ret = EINVAL;
>        goto done;
>    }
>    tmp_ctx = talloc_new(NULL);
>    if (tmp_ctx == NULL) {
>        DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
>        ret = ENOMEM;
>        goto done;
>    }
>    basedn = sysdb_domain_dn(sysdb, tmp_ctx, dom_name);
>    if (basedn == NULL) {
>        ret = ENOMEM;
>        goto done;
>    }
>    ret = sysdb_search_entry(tmp_ctx, sysdb, basedn, LDB_SCOPE_BASE, NULL,
>                             attrs, &msgs_count, &msgs);
>    if (ret != LDB_SUCCESS) {
>        ret = EIO;
>        goto done;
>    }
>    if (msgs_count != 1) {
>        DEBUG(SSSDBG_OP_FAILURE, ("Base search returned [%d] results, "
>                                 "expected 1.\n", msgs_count));
>        ret = EINVAL;
>        goto done;
>    }
>    sid_str = ldb_msg_find_attr_as_string(msgs[0], SYSDB_SUBDOMAIN_ID, NULL);
>    if (sid_str == NULL) {
>        DEBUG(SSSDBG_OP_FAILURE, ("SID of my domain is not available.\n"));
>        ret = EINVAL;
>        goto done;
>    }
>    err = sss_idmap_sid_to_smb_sid(pac_ctx->idmap_ctx, sid_str, &sid);
>    if (err != IDMAP_SUCCESS) {
>        DEBUG(SSSDBG_OP_FAILURE, ("sss_idmap_sid_to_smb_sid failed.\n"));
>        ret = EFAULT;
>        goto done;
>    }
>    pac_ctx->my_dom_sid = talloc_memdup(pac_ctx, sid,
>                                        sizeof(struct dom_sid));
>    if (pac_ctx->my_dom_sid == NULL) {
>        DEBUG(SSSDBG_OP_FAILURE, ("talloc_memdup failed.\n"));
>        ret = ENOMEM;
>        goto done;
>    }
> }
> *_sid = pac_ctx->my_dom_sid;
> ret = EOK;
>
> +done:
> talloc_free(sid);
> talloc_free(tmp_ctx);
> return ret;
>
> +}
>
> +static bool dom_sid_in_domain(const struct dom_sid *domain_sid,
>                          const struct dom_sid *sid)
>
> +{
> size_t c;
> if (!domain_sid || !sid) {
>    return false;
> }
> if (domain_sid->sid_rev_num != sid->sid_rev_num) {
>    return false;
> }
> for (c = 0; c < 6; c++) {
>    if (domain_sid->id_auth[c] != sid->id_auth[c]) {
>        return false;
>    }
> }
> if (domain_sid->num_auths > sid->num_auths) {
>    return false;
> }
> for (c = 0; c < domain_sid->num_auths-1; c++) {
>    if (domain_sid->sub_auths[c] != sid->sub_auths[c]) {
>        return false;
>    }
> }
> return true;
>
> +}
>
> +static errno_t get_gids_from_pac(TALLOC_CTX *mem_ctx,
>                             struct local_mapping_ranges *range_map,
>                             struct dom_sid *domain_sid,
>                             struct PAC_LOGON_INFO *logon_info,
>                             size_t *_gid_count, gid_t **_gids)
>
> +{
> int ret;
> size_t g;
> size_t s;
> struct netr_SamInfo3 *info3;
> gid_t *gids = NULL;
> info3 = &logon_info->info3;
> g = 0;
> if (info3->sidcount == 0) {
>    DEBUG(SSSDBG_TRACE_ALL, ("No extra groups found.\n"));
>    ret = EOK;
>    goto done;
> }
> gids = talloc_array(mem_ctx, gid_t, info3->sidcount);
> if (gids == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
>    ret = ENOMEM;
>    goto done;
> }
> for(s = 0; s < info3->sidcount; s++) {
>    if (dom_sid_in_domain(domain_sid, info3->sids[s].sid)) {
>        ret = local_sid_to_id(range_map, info3->sids[s].sid, &gids[g]);
>        if (ret != EOK) {
>            DEBUG(SSSDBG_OP_FAILURE, ("get_rid failed.\n"));
>            goto done;
>        }
>        DEBUG(SSSDBG_TRACE_ALL, ("Found extra group "
>                                 "with gid [%d].\n", gids[g]));
>        g++;
>    }
> }
> ret = EOK;
>
> +done:
> if (ret == EOK) {
>    *_gid_count = g;
>    *_gids = gids;
> } else {
>    talloc_free(gids);
> }
> return ret;
>
> +}
>
> +static errno_t get_data_from_pac(TALLOC_CTX *mem_ctx,
>                             uint8_t *pac_blob, size_t pac_len,
>                             struct PAC_LOGON_INFO **_logon_info)
>
> +{
> DATA_BLOB blob;
> struct ndr_pull *ndr_pull;
> struct PAC_DATA *pac_data;
> enum ndr_err_code ndr_err;
> size_t c;
> int ret;
> blob.data = pac_blob;
> blob.length = pac_len;
> ndr_pull = ndr_pull_init_blob(&blob, mem_ctx);
> if (ndr_pull == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("ndr_pull_init_blob failed.\n"));
>    return ENOMEM;
> }
> ndr_pull->flags |= LIBNDR_FLAG_REF_ALLOC; /* FIXME: is this really needed ? */
> pac_data = talloc_zero(mem_ctx, struct PAC_DATA);
> if (pac_data == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero failed.\n"));
>    return ENOMEM;
> }
> ndr_err = ndr_pull_PAC_DATA(ndr_pull, NDR_SCALARS|NDR_BUFFERS, pac_data);
> if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>    DEBUG(SSSDBG_OP_FAILURE, ("ndr_pull_PAC_DATA failed [%d]\n", ndr_err));
>    return EBADMSG;
> }
> for(c = 0; c < pac_data->num_buffers; c++) {
>    if (pac_data->buffers[c].type == PAC_TYPE_LOGON_INFO) {
>        *_logon_info = pac_data->buffers[c].info->logon_info.info;
>        return EOK;
>    }
> }
> ret = EINVAL;
> talloc_free(pac_data);
> return ret;
>
> +}
>
> +static errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
>                            struct pac_ctx *pac_ctx,
>                            struct sss_domain_info *dom,
>                            struct PAC_LOGON_INFO *logon_info,
>                            struct passwd **_pwd)
>
> +{
> struct passwd *pwd = NULL;
> struct netr_SamBaseInfo *base_info;
> int ret;
> pwd = talloc_zero(mem_ctx, struct passwd);
> if (pwd == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero failed.\n"));
>    return ENOMEM;
> }
> base_info = &logon_info->info3.base;
> if (base_info->account_name.size != 0) {
>    pwd->pw_name = talloc_strdup(pwd,
>                                 base_info->account_name.string);
>    if (pwd->pw_name == NULL) {
>        DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
>        ret = ENOMEM;
>        goto done;
>    }
> } else {
>    DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n"));
>    ret = EINVAL;
>    goto done;
> }
> if (base_info->rid > 0) {
>    ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name,
>                            base_info->domain_sid,
>                            base_info->rid, &pwd->pw_uid);
>    if (ret != EOK) {
>        DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n"));
>        goto done;
>    }
> } else {
>    DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n"));
>    ret = EINVAL;
>    goto done;
> }
> pwd->pw_gid = 0; /* We use MPGs for sub-domains */
> if (base_info->full_name.size != 0) {
>    pwd->pw_gecos = talloc_strdup(pwd, base_info->full_name.string);
> } else {
>    DEBUG(SSSDBG_OP_FAILURE, ("Missing full name in PAC "
>                              "using account name for gecos.\n"));
>    pwd->pw_gecos = talloc_strdup(pwd,
>                                  base_info->account_name.string);
> }
> if (pwd->pw_gecos == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
>    ret = ENOMEM;
>    goto done;
> }
> if (dom->subdomain_homedir) {
>    pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir,
>                                          pwd->pw_name, pwd->pw_uid,
>                                          dom->name);
>    if (pwd->pw_dir == NULL) {
>        ret = ENOMEM;
>        goto done;
>    }
> }
> pwd->pw_shell = NULL; /* Using default */
>
> +done:
> if (ret != EOK) {
>    talloc_free(pwd);
> }
> return ret;
>
> +}
>
> +struct pac_req_ctx {
> struct cli_ctx *cctx;
> struct pac_ctx *pac_ctx;
> const char *domain_name;
> const char *user_name;
> struct sss_domain_info *dom;
> struct PAC_LOGON_INFO *logon_info;
> struct dom_sid2 *domain_sid;
> size_t gid_count;
> gid_t *gids;
>
> +};
>
> +static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx);
> +static void pac_get_domains_done(struct tevent_req *req);
> +static errno_t save_pac_user(struct pac_req_ctx *pr_ctx);
> +static void pac_get_group_done(struct tevent_req *subreq);
> +static errno_t pac_save_memberships_next(struct tevent_req *req);
> +static errno_t pac_store_memberships(struct pac_req_ctx *pr_ctx,
>                                 struct sysdb_ctx *group_sysdb,
>                                 struct ldb_dn *user_dn,
>                                 int gid_iter);
>
> +struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx);
> +static void pac_save_memberships_done(struct tevent_req *req);
>
> +static errno_t pac_add_pac_user(struct cli_ctx *cctx)
> +{
> int ret;
> uint8_t *body;
> size_t blen;
> struct pac_req_ctx *pr_ctx;
> struct tevent_req *req;
> sss_packet_get_body(cctx->creq->in, &body, &blen);
> pr_ctx = talloc_zero(cctx, struct pac_req_ctx);
> if (pr_ctx == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
>    return ENOMEM;
> }
> pr_ctx->cctx = cctx;
> pr_ctx->pac_ctx = talloc_get_type(cctx->rctx->pvt_ctx,  struct pac_ctx);
> if (pr_ctx->pac_ctx == NULL) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Cannot find pac responder context.\n"));
>    return EINVAL;
> }
> ret = get_data_from_pac(pr_ctx, body, blen,
>                        &pr_ctx->logon_info);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("get_data_from_pac failed.\n"));
>    goto done;
> }
> pr_ctx->domain_name = pr_ctx->logon_info->info3.base.logon_domain.string;
> if (pr_ctx->domain_name == NULL) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("No domain name in PAC"));
>    ret = EINVAL;
>    goto done;
> }
> pr_ctx->user_name = pr_ctx->logon_info->info3.base.account_name.string;
> if (pr_ctx->user_name == NULL) {
>    ret = EINVAL;
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Missing account name in PAC.\n"));
>    goto done;
> }
> pr_ctx->dom = responder_get_domain(pr_ctx, cctx->rctx, pr_ctx->domain_name);
> if (pr_ctx->dom == NULL) {
>    req = sss_dp_get_domains_send(cctx->rctx, cctx->rctx, true,
>                                  pr_ctx->domain_name);
>    if (req == NULL) {
>        ret = ENOMEM;
>    } else {
>        tevent_req_set_callback(req, pac_get_domains_done, pr_ctx);
>        ret = EAGAIN;
>    }
>    goto done;
> }
> ret = pac_add_user_next(pr_ctx);
>
> +done:
> return pac_cmd_done(cctx, ret);
>
> +}
>
> +static void pac_get_domains_done(struct tevent_req *req)
> +{
> struct pac_req_ctx *pr_ctx = tevent_req_callback_data(req,
>                                                      struct pac_req_ctx);
> struct cli_ctx *cctx = pr_ctx->cctx;
> int ret;
> ret = sss_dp_get_domains_recv(req);
> talloc_free(req);
> if (ret != EOK) {
>    goto done;
> }
> pr_ctx->dom = responder_get_domain(pr_ctx, cctx->rctx, pr_ctx->domain_name);
> if (pr_ctx->dom == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("Corresponding domain [%s] has not been "
>                              "found\n", pr_ctx->domain_name));
>    ret = ENOENT;
>    goto done;
> }
> ret = pac_add_user_next(pr_ctx);
>
> +done:
> pac_cmd_done(cctx, ret);
>
> +}
>
> +static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx)
> +{
> int ret;
> struct tevent_req *req;
> struct dom_sid *my_dom_sid;
> ret = save_pac_user(pr_ctx);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n"));
>    goto done;
> }
> ret = get_my_domain_sid(pr_ctx->pac_ctx, pr_ctx->dom, &my_dom_sid);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("get_my_domain_sid failed.\n"));
>    goto done;
> }
> ret = get_gids_from_pac(pr_ctx, pr_ctx->pac_ctx->range_map, my_dom_sid,
>                        pr_ctx->logon_info, &pr_ctx->gid_count,
>                        &pr_ctx->gids);
> if (ret != EOK) {
>    DEBUG(SSSDBG_OP_FAILURE, ("get_gids_from_pac failed.\n"));
>    goto done;
> }
> req = pac_save_memberships_send(pr_ctx);
> if (req == NULL) {
>    ret = ENOMEM;
>    goto done;
> }
> tevent_req_set_callback(req, pac_save_memberships_done, pr_ctx);
> ret = EAGAIN;
>
> +done:
> return ret;
>
> +}
>
> +static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
> +{
> struct sysdb_ctx *sysdb;
> int ret;
> const char *attrs[] = {SYSDB_NAME, SYSDB_UIDNUM, SYSDB_GIDNUM, NULL};
> struct ldb_message *msg;
> struct passwd *pwd = NULL;
> TALLOC_CTX *tmp_ctx = NULL;
> sysdb = pr_ctx->dom->sysdb;
> if (sysdb == NULL) {
>    ret = EINVAL;
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Fatal: Sysdb CTX not found for this domain!\n"));
>    goto done;
> }
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) {
>    ret = ENOMEM;
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
>    goto done;
> }
> ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs,
>                                &msg);
> if (ret == EOK) {
>    /* TODO: check id uid and gid are equal. */
> } else if (ret == ENOENT) {
>    ret = get_pwd_from_pac(tmp_ctx, pr_ctx->pac_ctx, pr_ctx->dom,
>                           pr_ctx->logon_info, &pwd);
>    if (ret != EOK) {
>        DEBUG(SSSDBG_OP_FAILURE, ("get_pwd_from_pac failed.\n"));
>        goto done;
>    }
>    ret = sysdb_store_user(sysdb, pwd->pw_name, NULL,
>                           pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
>                           pwd->pw_dir,
>                           pwd->pw_shell, NULL, NULL,
>                           pr_ctx->dom->user_timeout, 0);
>    if (ret != EOK) {
>        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n",
>                                  ret, strerror(ret)));
>        goto done;
>    }
> } else {
>    DEBUG(SSSDBG_OP_FAILURE, ("sysdb_search_user_by_name failed.\n"));
>    goto done;
> }
> ret = EOK;
>
> +done:
> talloc_free(tmp_ctx);
> return ret;
>
> +}
>
> +struct pac_save_memberships_state {
> int gid_iter;
> struct ldb_dn *user_dn;
> struct pac_req_ctx *pr_ctx;
> struct sss_domain_info *group_dom;
>
> +};
>
> +struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
> +{
> struct pac_save_memberships_state *state;
> struct sss_domain_info *dom = pr_ctx->dom;
> struct tevent_req *req;
> errno_t ret;
> req = tevent_req_create(pr_ctx, &state, struct pac_save_memberships_state);
> if (req == NULL) {
>    return NULL;
> }
> state->gid_iter = 0;
> state->user_dn = sysdb_user_dn(dom->sysdb, state, dom->name,
>                               pr_ctx->user_name);
> if (state->user_dn == NULL) {
>    ret = ENOMEM;
>    goto done;
> }
> state->pr_ctx = pr_ctx;
> /* Remote users are member of local groups */
> if (pr_ctx->dom->parent != NULL) {
>    state->group_dom = pr_ctx->dom->parent;
> } else {
>    state->group_dom = pr_ctx->dom;
> }
> ret = pac_save_memberships_next(req);
> if (ret == EOK) {
>    tevent_req_done(req);
>    tevent_req_post(req, pr_ctx->cctx->ev);
> }
>
> +done:
> if (ret != EOK && ret != EAGAIN) {
>    tevent_req_error(req, ret);
>    tevent_req_post(req, pr_ctx->cctx->ev);
> }
> return req;
>
> +}
>
> +errno_t pac_save_memberships_next(struct tevent_req *req)
> +{
> errno_t ret;
> uint32_t gid;
> struct tevent_req *subreq;
> struct ldb_message *group;
> struct pac_save_memberships_state *state;
> struct pac_req_ctx *pr_ctx;
> state = tevent_req_data(req, struct pac_save_memberships_state);
> pr_ctx = state->pr_ctx;
> while (state->gid_iter < pr_ctx->gid_count) {
>    gid = pr_ctx->gids[state->gid_iter];
>    ret = sysdb_search_group_by_gid(pr_ctx, state->group_dom->sysdb,
>                                    gid, NULL, &group);
>    if (ret == EOK) {
>        ret = pac_store_memberships(state->pr_ctx, state->group_dom->sysdb,
>                                    state->user_dn, state->gid_iter);
>        if (ret != EOK) {
>            goto done;
>        }
>        state->gid_iter++;
>        continue;
>    } else if (ret != ENOENT) {
>        goto done;
>    }
>    subreq = sss_dp_get_account_send(state, pr_ctx->cctx->rctx,
>                                     state->group_dom, true,
>                                     SSS_DP_GROUP, NULL,
>                                     gid, NULL);
>    if (subreq == NULL) {
>        ret = ENOMEM;
>        goto done;
>    }
>    tevent_req_set_callback(subreq, pac_get_group_done, req);
>    return EAGAIN;
> }
> ret = EOK;
>
> +done:
> return ret;
>
> +}
>
> +static void pac_get_group_done(struct tevent_req *subreq)
> +{
> struct tevent_req *req;
> struct pac_save_memberships_state *state;
> req = tevent_req_callback_data(subreq, struct tevent_req);
> state = tevent_req_data(req, struct pac_save_memberships_state);
> errno_t ret;
> dbus_uint16_t err_maj;
> dbus_uint32_t err_min;
> char *err_msg;
> ret = sss_dp_get_account_recv(req, subreq,
>                              &err_maj, &err_min,
>                              &err_msg);
> talloc_zfree(subreq);
> talloc_zfree(err_msg);
> if (ret != EOK) {
>    goto error;
> }
> ret = pac_store_memberships(state->pr_ctx, state->group_dom->sysdb,
>                            state->user_dn, state->gid_iter);
> if (ret != EOK) {
>    goto error;
> }
> state->gid_iter++;
> ret = pac_save_memberships_next(req);
> if (ret == EOK) {
>    tevent_req_done(req);
> } else if (ret != EAGAIN) {
>    goto error;
> }
> return;
>
> +error:
> tevent_req_error(req, ret);
>
> +}
>
> +static errno_t
> +pac_store_memberships(struct pac_req_ctx *pr_ctx,
>                  struct sysdb_ctx *group_sysdb,
>                  struct ldb_dn *user_dn,
>                  int gid_iter)
>
> +{
> TALLOC_CTX *tmp_ctx;
> const char *group_name;
> struct sysdb_attrs *group_attrs;
> struct ldb_message *group;
> uint32_t gid;
> errno_t ret;
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) {
>    return ENOMEM;
> }
> gid = pr_ctx->gids[gid_iter];
> ret = sysdb_search_group_by_gid(tmp_ctx, group_sysdb,
>                                gid, NULL, &group);
> if (ret != EOK) {
>    goto done;
> }
> group_name = ldb_msg_find_attr_as_string(group, SYSDB_NAME, NULL);
> if (group_name == NULL) {
>    ret = EIO;
>    goto done;
> }
> group_attrs = talloc_zero(tmp_ctx, struct sysdb_attrs);
> if (group_attrs == NULL) {
>    ret = ENOMEM;
>    goto done;
> }
> group_attrs->num = 1;
> group_attrs->a = ldb_msg_find_element(group, SYSDB_MEMBER);
> if (group_attrs->a == NULL) {
>    group_attrs->a = talloc_zero(group_attrs, struct ldb_message_element);
>    if (group_attrs->a == NULL) {
>        ret = ENOMEM;
>        goto done;
>    }
>    group_attrs->a[0].name = talloc_strdup(group_attrs->a, SYSDB_MEMBER);
>    if (group_attrs->a[0].name == NULL) {
>        ret = ENOMEM;
>        goto done;
>    }
> }
> ret = sysdb_attrs_add_string(group_attrs, SYSDB_MEMBER,
>                             ldb_dn_get_linearized(user_dn));
> if (ret != EOK) {
>    goto done;
> }
> ret = sysdb_store_group(group_sysdb, group_name, gid,
>                        group_attrs, pr_ctx->dom->group_timeout, 0);
> if (ret != EOK) {
>    goto done;
> }
>
> +done:
> talloc_free(tmp_ctx);
> return ret;
>
> +}
>
> +errno_t pac_save_memberships_recv(struct tevent_req *subreq)
> +{
> TEVENT_REQ_RETURN_ON_ERROR(subreq);
> return EOK;
>
> +}
>
> +static void pac_save_memberships_done(struct tevent_req *req)
> +{
> struct pac_req_ctx *pr_ctx = tevent_req_callback_data(req, struct pac_req_ctx);
> struct cli_ctx *cctx = pr_ctx->cctx;
> errno_t ret;
> ret = pac_save_memberships_recv(req);
> talloc_zfree(req);
> talloc_free(pr_ctx);
> pac_cmd_done(cctx, ret);
>
> +}
>
> struct cli_protocol_version *register_cli_protocol_version(void)
>  {
>      static struct cli_protocol_version pac_cli_protocol_version[] = {
> @@ -37,6 +931,7 @@ struct cli_protocol_version *register_cli_protocol_version(void)
>
> static struct sss_cmd_table pac_cmds[] = {
>      {SSS_GET_VERSION, sss_cmd_get_version},
> {SSS_PAC_ADD_PAC_USER, pac_add_pac_user},
> {SSS_CLI_NULL, NULL}
>
> };
>
> diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
> index 647d233f650863b28ebee160d1e63ea788b2e04e..30018d876d0f65ea19d60c2e49cde91547964af9 100644
> --- a/src/sss_client/sss_cli.h
> +++ b/src/sss_client/sss_cli.h
> @@ -175,6 +175,9 @@ enum sss_cli_command {
>      SSS_SSH_GET_USER_PUBKEYS = 0x00E1,
>      SSS_SSH_GET_HOST_PUBKEYS = 0x00E2,
>
> +/* PAC responder calls */
> SSS_PAC_ADD_PAC_USER     = 0x00E9,
>
> /* PAM related calls */
>      SSS_PAM_AUTHENTICATE     = 0x00F1, /**< see pam_sm_authenticate(3) for
>                                          * details.
> -- 
> 1.7.7.6
> From 67764b41da937a6718154e37e46fba4fae5612da Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Tue, 24 May 2011 11:57:09 +0200
> Subject: [PATCH 04/10] PAC responder: support in spec file
>
> contrib/sssd.spec.in |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> index 8d7a8d5a831628793372dd1efe56a41302b99866..2ebcb3456b44d8031b39b98bf5d92d837ba79c63 100644
> --- a/contrib/sssd.spec.in
> +++ b/contrib/sssd.spec.in
> @@ -321,6 +321,7 @@ rm -rf $RPM_BUILD_ROOT
>  %{_libexecdir}/%{servicename}/sssd_autofs
>  %{_libexecdir}/%{servicename}/sssd_ssh
>  %{_libexecdir}/%{servicename}/sssd_sudo
> +%{_libexecdir}/%{servicename}/sssd_pac
>  %endif
>
> %{_libdir}/%{name}/libsss_ipa.so
> @@ -367,6 +368,9 @@ rm -rf $RPM_BUILD_ROOT
>  /%{_lib}/libnss_sss.so.2
>  /%{_lib}/security/pam_sss.so
>  %{_libdir}/krb5/plugins/libkrb5/sssd_krb5_locator_plugin.so
> +%if (0%{?enable_experimental} == 1)
> +%{_libdir}/krb5/plugins/authdata/sssd_pac_plugin.so
> +%endif
>  %{_mandir}/man8/pam_sss.8*
>  %{_mandir}/man8/sssd_krb5_locator_plugin.8*
>
> -- 
> 1.7.7.6
> From 510c93543198c096bb5c7898971835e2c11b685e Mon Sep 17 00:00:00 2001
> From: Jan Zeleny 
> Date: Mon, 11 Jun 2012 06:00:22 -0400
> Subject: [PATCH 05/10] PAC responder: test suite
>
> Makefile.am                     |   15 +++++-
>  src/tests/pac_responder-tests.c |  106 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+), 1 deletions(-)
>  create mode 100644 src/tests/pac_responder-tests.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 8212070f1050147635e07c3dbeb7dc8dea766d6c..6348ee2805956afde620904cd65eb8e82863cb09 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -119,7 +119,8 @@ if HAVE_CHECK
>          util-tests \
>          debug-tests \
>          ipa_hbac-tests \
>    sss_idmap-tests
>    sss_idmap-tests \
>    pac_responder-tests
>
> endif
>
> check_PROGRAMS = \
> @@ -985,6 +986,18 @@ sss_idmap_tests_LDADD = \
>      libsss_test_common.la \
>      libsss_idmap.la
>
> +pac_responder_tests_SOURCES = \
> src/tests/pac_responder-tests.c \
> src/responder/pac/pacsrv_utils.c
>
> +pac_responder_tests_CFLAGS = \
> $(AM_CFLAGS) \
> $(NDR_KRB5PAC_CFLAGS) \
> $(CHECK_CFLAGS)
>
> +pac_responder_tests_LDADD = \
> $(CHECK_LIBS) \
> $(TALLOC_LIBS) \
> libsss_debug.la \
> libsss_test_common.la
>
> endif
>
> stress_tests_SOURCES = \
> diff --git a/src/tests/pac_responder-tests.c b/src/tests/pac_responder-tests.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..720793cc6670571da29b07468c54e7fb6c10c3f3
> --- /dev/null
> +++ b/src/tests/pac_responder-tests.c
> @@ -0,0 +1,106 @@
> +/*
> SSSD - Test for PAC reponder functions
> Authors:
>    Sumit Bose <[e-mail]>
> Copyright (C) 2012 Red Hat
> 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
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
>
> +#include <check.h>
>
> +#include <stdbool.h>
> +#include <util/data_blob.h>
> +#include <gen_ndr/security.h>
>
> +#include "tests/common.h"
> +#include "responder/pac/pacsrv.h"
>
> +struct dom_sid test_smb_sid = {1, 5, {0, 0, 0, 0, 0, 5},
>                           {21, 2127521184, 1604012920, 1887927527, 1123,
>                            0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
>
> +const uint32_t test_id = 1200123;
>
> +struct dom_sid test_smb_sid_2nd = {1, 5, {0, 0, 0, 0, 0, 5},
>                           {21, 2127521184, 1604012920, 1887927527, 201456,
>                            0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
>
> +const uint32_t test_id_2nd = 1200456;
>
> +struct local_mapping_ranges test_map = {{1200000, 1399999},
>                                    {1000, 200999},
>                                    {201000, 400999}};
>
> +START_TEST(pac_test_local_sid_to_id)
> +{
> int ret;
> uint32_t id;
> ret = local_sid_to_id(&test_map, &test_smb_sid, &id);
> fail_unless(ret == EOK,
>            "Failed to convert local sid to id.");
> fail_unless(id == test_id, "Wrong id returne, expected [%d], got [%d].",
>                           test_id, id);
>
> +}
> +END_TEST
>
> +START_TEST(pac_test_seondary_local_sid_to_id)
> +{
> int ret;
> uint32_t id;
> ret = local_sid_to_id(&test_map, &test_smb_sid_2nd, &id);
> fail_unless(ret == EOK,
>            "Failed to convert local sid to id.");
> fail_unless(id == test_id_2nd, "Wrong id returne, expected [%d], got [%d].",
>                           test_id_2nd, id);
>
> +}
> +END_TEST
>
> +Suite *idmap_test_suite (void)
> +{
> Suite *s = suite_create ("PAC responder");
> TCase *tc_pac = tcase_create("PAC responder tests");
> /*tcase_add_checked_fixture(tc_init,
>                          leak_check_setup,
>                          leak_check_teardown);*/
> tcase_add_test(tc_pac, pac_test_local_sid_to_id);
> tcase_add_test(tc_pac, pac_test_seondary_local_sid_to_id);
> suite_add_tcase(s, tc_pac);
> return s;
>
> +}
>
> +int main(int argc, const char *argv[])
> +{
> int number_failed;
> tests_set_cwd();
> Suite *s = idmap_test_suite();
> SRunner *sr = srunner_create(s);
> /* If CK_VERBOSITY is set, use that, otherwise it defaults to CK_NORMAL */
> srunner_run_all(sr, CK_ENV);
> number_failed = srunner_ntests_failed (sr);
> srunner_free (sr);
> return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
>
> 1.7.7.6
> From ff0ce62fb23d45bc22ac83cc84caa23d1d865af2 Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Fri, 13 May 2011 18:41:16 +0200
> Subject: [PATCH 06/10] PAC client: add basic support in common client code
>
> src/sss_client/common.c  |   33 +++++++++++++++++++++++++++++++++
>  src/sss_client/sss_cli.h |    5 +++++
>  2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/src/sss_client/common.c b/src/sss_client/common.c
> index 28adb4424dfc72dab79ca00fc905152a7cff3e8a..83c95b2921eac58fa349925430688c609b404291 100644
> --- a/src/sss_client/common.c
> +++ b/src/sss_client/common.c
> @@ -383,6 +383,8 @@ static bool sss_cli_check_version(const char *socket_name)
>          expected_version = SSS_AUTOFS_PROTOCOL_VERSION;
>      } else if (strcmp(socket_name, SSS_SSH_SOCKET_NAME) == 0) {
>          expected_version = SSS_SSH_PROTOCOL_VERSION;
> } else if (strcmp(socket_name, SSS_PAC_SOCKET_NAME) == 0) {
>    expected_version = SSS_PAC_PROTOCOL_VERSION;
> } else {
>        return false;
>    }
>
> @@ -715,6 +717,37 @@ enum nss_status sss_nss_make_request(enum sss_cli_command cmd,
>      }
>  }
>
> +int sss_pac_make_request(enum sss_cli_command cmd,
>                     struct sss_cli_req_data *rd,
>                     uint8_t **repbuf, size_t *replen,
>                     int *errnop)
>
> +{
> enum sss_status ret;
> char *envval;
> /* avoid looping in the nss daemon */
> envval = getenv("_SSS_LOOPS");
> if (envval && strcmp(envval, "NO") == 0) {
>    return NSS_STATUS_NOTFOUND;
> }
> ret = sss_cli_check_socket(errnop, SSS_PAC_SOCKET_NAME);
> if (ret != SSS_STATUS_SUCCESS) {
>    return NSS_STATUS_UNAVAIL;
> }
> ret = sss_cli_make_request_nochecks(cmd, rd, repbuf, replen, errnop);
> switch (ret) {
> case SSS_STATUS_TRYAGAIN:
>    return NSS_STATUS_TRYAGAIN;
> case SSS_STATUS_SUCCESS:
>    return NSS_STATUS_SUCCESS;
> case SSS_STATUS_UNAVAIL:
> default:
>    return NSS_STATUS_UNAVAIL;
> }
>
> +}
>
> errno_t check_server_cred(int sockfd)
>  {
>  #ifdef HAVE_UCRED
> diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
> index 30018d876d0f65ea19d60c2e49cde91547964af9..70e4f1fe31bd37c8a3b8e198d2b732d18d57798c 100644
> --- a/src/sss_client/sss_cli.h
> +++ b/src/sss_client/sss_cli.h
> @@ -47,6 +47,7 @@ typedef int errno_t;
>  #define SSS_SUDO_PROTOCOL_VERSION 0
>  #define SSS_AUTOFS_PROTOCOL_VERSION 1
>  #define SSS_SSH_PROTOCOL_VERSION 0
> +#define SSS_PAC_PROTOCOL_VERSION 1
>
> #ifdef LOGIN_NAME_MAX
>  #define SSS_NAME_MAX LOGIN_NAME_MAX
> @@ -483,6 +484,10 @@ int sss_pam_make_request(enum sss_cli_command cmd,
>                                       struct sss_cli_req_data *rd,
>                                       uint8_t **repbuf, size_t *replen,
>                                       int *errnop);
> +int sss_pac_make_request(enum sss_cli_command cmd,
>                     struct sss_cli_req_data *rd,
>                     uint8_t **repbuf, size_t *replen,
>                     int *errnop);
>
> int sss_sudo_make_request(enum sss_cli_command cmd,
>                            struct sss_cli_req_data *rd,
> -- 
> 1.7.7.6
> From dfc7f885a221ad36ced580380b1a1a737df35f0f Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Tue, 24 May 2011 11:12:28 +0200
> Subject: [PATCH 07/10] PAC client: add krb5 authdata plugin
>
> Makefile.am                        |   21 +++
>  configure.ac                       |    1 +
>  src/conf_macros.m4                 |   14 ++
>  src/sss_client/krb5_authdata_int.h |  185 ++++++++++++++++++++++++
>  src/sss_client/sssd_pac.c          |  279 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 500 insertions(+), 0 deletions(-)
>  create mode 100644 src/sss_client/krb5_authdata_int.h
>  create mode 100644 src/sss_client/sssd_pac.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 6348ee2805956afde620904cd65eb8e82863cb09..c2b83234957d72cfb76eb55e2fa7416465d04bab 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,9 @@ ldblibdir = @ldblibdir@
>  if BUILD_KRB5_LOCATOR_PLUGIN
>  krb5plugindir = @krb5pluginpath@
>  endif
> +if BUILD_PAC_RESPONDER
> +krb5authdata_plugindir = @krb5authdatapluginpath@
> +endif
>  sssdconfdir = $(sysconfdir)/sssd
>  sssddatadir = $(datadir)/sssd
>  sssdapiplugindir = $(sssddatadir)/sssd.api.d
> @@ -153,6 +156,11 @@ krb5plugin_LTLIBRARIES = \
>      sssd_krb5_locator_plugin.la
>  endif
>
> +if BUILD_PAC_RESPONDER
> +krb5authdata_plugin_LTLIBRARIES = \
> sssd_pac_plugin.la
>
> +endif
>
> noinst_LTLIBRARIES = \
>      libsss_crypt.la
>
> @@ -1364,6 +1372,19 @@ sssd_krb5_locator_plugin_la_LDFLAGS = \
>      -module
>  endif
>
> +sssd_pac_plugin_la_SOURCES = \
> src/sss_client/sssd_pac.c \
> src/sss_client/common.c \
> src/sss_client/sss_cli.h \
> src/sss_client/krb5_authdata_int.h
>
> +sssd_pac_plugin_la_CFLAGS = \
> $(AM_CFLAGS) \
> $(KRB5_CFLAGS)
>
> +sssd_pac_plugin_la_LDFLAGS = \
> -lkrb5 \
> -avoid-version \
> -module
>
> if BUILD_PYTHON_BINDINGS
>  pysss_la_SOURCES = \
>      $(SSSD_TOOLS_OBJ) \
> diff --git a/configure.ac b/configure.ac
> index 0dee070baf35d6ec37e6bf348159d884bdd2450a..102ed7b8845104c1a349574031fc91fd0de84198 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -88,6 +88,7 @@ WITH_MANPAGES
>  WITH_XML_CATALOG
>  WITH_KRB5_PLUGIN_PATH
>  WITH_KRB5_RCACHE_DIR
> +WITH_KRB5AUTHDATA_PLUGIN_PATH
>  WITH_PYTHON_BINDINGS
>  WITH_SELINUX
>  WITH_NSCD
> diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
> index 8c13e5dba71a98f25c42acaeccd3b01233f77210..b82b742a207ff30c5618c4e7b112c2ed14ba7a8d 100644
> --- a/src/conf_macros.m4
> +++ b/src/conf_macros.m4
> @@ -246,6 +246,20 @@ AC_DEFUN([WITH_KRB5_RCACHE_DIR],
>      AC_SUBST(krb5rcachedir)
>      AC_DEFINE_UNQUOTED(KRB5_RCACHE_DIR, "$krb5rcachedir", [Directory used for storing Kerberos replay caches])
>    ])
>
> +AC_DEFUN([WITH_KRB5AUTHDATA_PLUGIN_PATH],
> [ AC_ARG_WITH([krb5authdata-plugin-path],
>            [AC_HELP_STRING([--with-krb5authdata-plugin-path=PATH],
>                            [Path to kerberos authdata plugin store [/usr/lib/krb5/plugins/authdata]]
>                           )
>            ]
>           )
> krb5authdatapluginpath="${libdir}/krb5/plugins/authdata"
> if test x"$with_krb5authdata_plugin_path" != x; then
>    krb5authdatapluginpath=$with_krb5authdata_plugin_path
> fi
> AC_SUBST(krb5authdatapluginpath)
> ])
>
> AC_DEFUN([WITH_PYTHON_BINDINGS],
>    [ AC_ARG_WITH([python-bindings],
> diff --git a/src/sss_client/krb5_authdata_int.h b/src/sss_client/krb5_authdata_int.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..5e0cf5e026f8ae96fca642f83cee3f2343fe1320
> --- /dev/null
> +++ b/src/sss_client/krb5_authdata_int.h
> @@ -0,0 +1,185 @@
> +/*
> SSSD - MIT Kerberos authdata plugin
> This file contains definitions and declarations to build authdata plugins
> for MIT Kerberos outside of the MIT Kerberos source tree.
>
> +*/
>
> +#ifndef _KRB5_AUTHDATA_INT_H
> +#define _KRB5_AUTHDATA_INT_H
>
> +krb5_error_code KRB5_CALLCONV
> +krb5_ser_pack_int32(krb5_int32, krb5_octet **, size_t *);
>
> +krb5_error_code KRB5_CALLCONV
> +krb5_ser_unpack_int32(krb5_int32 *, krb5_octet **, size_t *);
>
> +krb5_error_code KRB5_CALLCONV
> +krb5_ser_pack_bytes(krb5_octet *, size_t, krb5_octet **, size_t *);
>
> +#define AD_USAGE_AS_REQ         0x01
> +#define AD_USAGE_TGS_REQ        0x02
> +#define AD_USAGE_AP_REQ         0x04
> +#define AD_USAGE_KDC_ISSUED     0x08
> +#define AD_USAGE_MASK           0x0F
> +#define AD_INFORMATIONAL        0x10
>
> +struct _krb5_authdata_context;
> +typedef struct _krb5_authdata_context *krb5_authdata_context;
>
> +typedef void
> +(*authdata_client_plugin_flags_proc)(krb5_context kcontext,
>                                 void *plugin_context,
>                                 krb5_authdatatype ad_type,
>                                 krb5_flags *flags);
>
> +typedef krb5_error_code
> +(*authdata_client_plugin_init_proc)(krb5_context context,
>                                void **plugin_context);
>
> +typedef void
> +(*authdata_client_plugin_fini_proc)(krb5_context kcontext,
>                                void *plugin_context);
>
> +typedef krb5_error_code
> +(*authdata_client_request_init_proc)(krb5_context kcontext,
>                                 struct _krb5_authdata_context *context,
>                                 void *plugin_context,
>                                 void **request_context);
>
> +typedef void
> +(*authdata_client_request_fini_proc)(krb5_context kcontext,
>                                 struct _krb5_authdata_context *context,
>                                 void *plugin_context,
>                                 void *request_context);
>
> +typedef krb5_error_code
> +(*authdata_client_import_authdata_proc)(krb5_context kcontext,
>                                    struct _krb5_authdata_context *context,
>                                    void *plugin_context,
>                                    void *request_context,
>                                    krb5_authdata **authdata,
>                                    krb5_boolean kdc_issued_flag,
>                                    krb5_const_principal issuer);
>
> +typedef krb5_error_code
> +(*authdata_client_export_authdata_proc)(krb5_context kcontext,
>                                    struct _krb5_authdata_context *context,
>                                    void *plugin_context,
>                                    void *request_context,
>                                    krb5_flags usage,
>                                    krb5_authdata ***authdata);
>
> +typedef krb5_error_code
> +(*authdata_client_get_attribute_types_proc)(krb5_context kcontext,
>                                        struct _krb5_authdata_context *context,
>                                        void *plugin_context,
>                                        void *request_context,
>                                        krb5_data **attrs);
>
> +typedef krb5_error_code
> +(*authdata_client_get_attribute_proc)(krb5_context kcontext,
>                                  struct _krb5_authdata_context *context,
>                                  void *plugin_context,
>                                  void *request_context,
>                                  const krb5_data *attribute,
>                                  krb5_boolean *authenticated,
>                                  krb5_boolean *complete,
>                                  krb5_data *value,
>                                  krb5_data *display_value,
>                                  int *more);
>
> +typedef krb5_error_code
> +(*authdata_client_set_attribute_proc)(krb5_context kcontext,
>                                  struct _krb5_authdata_context *context,
>                                  void *plugin_context,
>                                  void *request_context,
>                                  krb5_boolean complete,
>                                  const krb5_data *attribute,
>                                  const krb5_data *value);
>
> +typedef krb5_error_code
> +(*authdata_client_delete_attribute_proc)(krb5_context kcontext,
>                                     struct _krb5_authdata_context *context,
>                                     void *plugin_context,
>                                     void *request_context,
>                                     const krb5_data *attribute);
>
> +typedef krb5_error_code
> +(*authdata_client_export_internal_proc)(krb5_context kcontext,
>                                    struct _krb5_authdata_context *context,
>                                    void *plugin_context,
>                                    void *request_context,
>                                    krb5_boolean restrict_authenticated,
>                                    void **ptr);
>
> +typedef void
> +(*authdata_client_free_internal_proc)(krb5_context kcontext,
>                                  struct _krb5_authdata_context *context,
>                                  void *plugin_context,
>                                  void *request_context,
>                                  void *ptr);
>
> +typedef krb5_error_code
> +(*authdata_client_verify_proc)(krb5_context kcontext,
>                           struct _krb5_authdata_context *context,
>                           void *plugin_context,
>                           void *request_context,
>                           const krb5_auth_context *auth_context,
>                           const krb5_keyblock *key,
>                           const krb5_ap_req *req);
>
> +typedef krb5_error_code
> +(*authdata_client_size_proc)(krb5_context kcontext,
>                         struct _krb5_authdata_context *context,
>                         void *plugin_context,
>                         void *request_context,
>                         size_t *sizep);
>
> +typedef krb5_error_code
> +(*authdata_client_externalize_proc)(krb5_context kcontext,
>                                struct _krb5_authdata_context *context,
>                                void *plugin_context,
>                                void *request_context,
>                                krb5_octet **buffer,
>                                size_t *lenremain);
>
> +typedef krb5_error_code
> +(*authdata_client_internalize_proc)(krb5_context kcontext,
>                                struct _krb5_authdata_context *context,
>                                void *plugin_context,
>                                void *request_context,
>                                krb5_octet **buffer,
>                                size_t *lenremain);
>
> +typedef krb5_error_code
> +(*authdata_client_copy_proc)(krb5_context kcontext,
>                         struct _krb5_authdata_context *context,
>                         void *plugin_context,
>                         void *request_context,
>                         void *dst_plugin_context,
>                         void *dst_request_context);
>
> +typedef struct krb5plugin_authdata_client_ftable_v0 {
> char *name;
> krb5_authdatatype *ad_type_list;
> authdata_client_plugin_init_proc init;
> authdata_client_plugin_fini_proc fini;
> authdata_client_plugin_flags_proc flags;
> authdata_client_request_init_proc request_init;
> authdata_client_request_fini_proc request_fini;
> authdata_client_get_attribute_types_proc get_attribute_types;
> authdata_client_get_attribute_proc get_attribute;
> authdata_client_set_attribute_proc set_attribute;
> authdata_client_delete_attribute_proc delete_attribute;
> authdata_client_export_authdata_proc export_authdata;
> authdata_client_import_authdata_proc import_authdata;
> authdata_client_export_internal_proc export_internal;
> authdata_client_free_internal_proc free_internal;
> authdata_client_verify_proc verify;
> authdata_client_size_proc size;
> authdata_client_externalize_proc externalize;
> authdata_client_internalize_proc internalize;
> authdata_client_copy_proc copy; /* optional */
>
> +} krb5plugin_authdata_client_ftable_v0;
>
> +#endif /* _KRB5_AUTHDATA_INT_H */
> diff --git a/src/sss_client/sssd_pac.c b/src/sss_client/sssd_pac.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5abb7c05a53992d99c5aac48b68f8f6bbe71d101
> --- /dev/null
> +++ b/src/sss_client/sssd_pac.c
> @@ -0,0 +1,279 @@
> +/*
> Authors:
>    Sumit Bose <[e-mail]>
> Copyright (C) 2011 Red Hat
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU Lesser General Public License as published by
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU Lesser General Public License for more details.
> You should have received a copy of the GNU Lesser General Public License
> along with this program.  If not, see http://www.gnu.org/licenses/.
>
> +*/
>
> +/* A short documentation about authdata plugins can be found in
> http://http://k5wiki.kerberos.org/wiki/Projects/VerifyAuthData */
>
> +#include <krb5/krb5.h>
> +#include <errno.h>
>
> +#include "krb5_authdata_int.h"
> +#include "sss_cli.h"
>
> +struct sssd_context {
> krb5_data data;
>
> +};
>
> +static krb5_error_code
> +sssdpac_init(krb5_context kcontext, void **plugin_context)
> +{
> *plugin_context = NULL;
> return 0;
>
> +}
>
> +static void
> +sssdpac_flags(krb5_context kcontext,
>          void *plugin_context,
>          krb5_authdatatype ad_type,
>          krb5_flags *flags)
>
> +{
> *flags = AD_USAGE_KDC_ISSUED | AD_INFORMATIONAL;
>
> +}
>
> +static void
> +sssdpac_fini(krb5_context kcontext, void *plugin_context)
> +{
> return;
>
> +}
>
> +static krb5_error_code
> +sssdpac_request_init(krb5_context kcontext,
>                 krb5_authdata_context context,
>                 void *plugin_context,
>                 void **request_context)
>
> +{
> struct sssd_context *sssdctx;
> sssdctx = (struct sssd_context *)calloc(1, sizeof(*sssdctx));
> if (sssdctx == NULL)
>    return ENOMEM;
> *request_context = sssdctx;
> return 0;
>
> +}
>
> +static krb5_error_code
> +sssdpac_import_authdata(krb5_context kcontext,
>                    krb5_authdata_context context,
>                    void *plugin_context,
>                    void *request_context,
>                    krb5_authdata **authdata,
>                    krb5_boolean kdc_issued,
>                    krb5_const_principal kdc_issuer)
>
> +{
> struct sss_cli_req_data sss_data;
> int ret;
> uint8_t *repbuf;
> size_t replen;
> int errnop;
> char *data = NULL;
> struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> if (authdata[0] == NULL) {
>    return EINVAL;
> }
> sss_data.len = authdata[0]->length;
> sss_data.data = authdata[0]->contents;
> ret = sss_pac_make_request(SSS_PAC_ADD_PAC_USER, &sss_data,
>                           &repbuf, &replen, &errnop);
> if (ret != 0) {
>    /* Ignore the error */
> }
> if (authdata[0]->length > 0) {
>    data = malloc(sizeof(char) * authdata[0]->length);
>    if (data == NULL) {
>        return ENOMEM;
>    }
>    memcpy(data, authdata[0]->contents, authdata[0]->length);
> }
> if (sssdctx->data.data != NULL) {
>    krb5_free_data_contents(kcontext, &sssdctx->data);
> }
> sssdctx->data.length = authdata[0]->length;
> sssdctx->data.data = data;
> return 0;
>
> +}
>
> +static void
> +sssdpac_request_fini(krb5_context kcontext,
>                 krb5_authdata_context context,
>                 void *plugin_context,
>                 void *request_context)
>
> +{
> struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> if (sssdctx != NULL) {
>    if (sssdctx->data.data != NULL)
>        krb5_free_data_contents(kcontext, &sssdctx->data);
>    free(sssdctx);
> }
>
> +}
>
> +static krb5_error_code
> +sssdpac_size(krb5_context kcontext,
>         krb5_authdata_context context,
>         void *plugin_context,
>         void *request_context,
>         size_t *sizep)
>
> +{
> struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> *sizep += sizeof(krb5_int32);
> *sizep += sssdctx->data.length;
> *sizep += sizeof(krb5_int32);
> return 0;
>
> +}
>
> +static krb5_error_code
> +sssdpac_externalize(krb5_context kcontext,
>                krb5_authdata_context context,
>                void *plugin_context,
>                void *request_context,
>                krb5_octet **buffer,
>                size_t *lenremain)
>
> +{
> krb5_error_code code = 0;
> struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> size_t required = 0;
> krb5_octet *bp;
> size_t remain;
> bp = *buffer;
> remain = *lenremain;
> if (sssdctx->data.data != NULL) {
>    sssdpac_size(kcontext, context, plugin_context,
>               request_context, &required);
>    if (required <= remain) {
>        krb5_ser_pack_int32((krb5_int32)sssdctx->data.length,
>                            &bp, &remain);
>        krb5_ser_pack_bytes((krb5_octet *)sssdctx->data.data,
>                            (size_t)sssdctx->data.length,
>                            &bp, &remain);
>        krb5_ser_pack_int32(0,
>                            &bp, &remain);
>    } else {
>        code = ENOMEM;
>    }
> } else {
>    krb5_ser_pack_int32(0, &bp, &remain); /* length */
>    krb5_ser_pack_int32(0, &bp, &remain); /* verified */
> }
> *buffer = bp;
> *lenremain = remain;
> return code;
>
> +}
>
> +static krb5_error_code
> +sssdpac_internalize(krb5_context kcontext,
>                krb5_authdata_context context,
>                void *plugin_context,
>                void *request_context,
>                krb5_octet **buffer,
>                size_t *lenremain)
>
> +{
> struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> krb5_error_code code;
> krb5_int32 ibuf;
> krb5_octet *bp;
> size_t remain;
> krb5_data data;
> bp = *buffer;
> remain = *lenremain;
> /* length */
> code = krb5_ser_unpack_int32(&ibuf, &bp, &remain);
> if (code != 0)
>    return code;
> if (ibuf != 0) {
>    data.length = ibuf;
>    data.data = malloc(sizeof(char) * ibuf);
>    if (data.data == NULL) {
>        return ENOMEM;
>    }
>    memcpy(data.data, bp, ibuf);
>    bp += ibuf;
>    remain -= ibuf;
> } else {
>    data.length = 0;
>    data.data = NULL;
> }
> /* verified */
> code = krb5_ser_unpack_int32(&ibuf, &bp, &remain);
> if (code != 0) {
>    return code;
> }
> if (sssdctx->data.data != NULL) {
>    krb5_free_data_contents(kcontext, &sssdctx->data);
> }
> sssdctx->data.length = data.length;
> sssdctx->data.data = data.data;
> *buffer = bp;
> *lenremain = remain;
> return 0;
>
> +}
>
> +static krb5_authdatatype sssdpac_ad_types[] = { KRB5_AUTHDATA_WIN2K_PAC, 0 };
>
> +krb5plugin_authdata_client_ftable_v0 authdata_client_0 = {
> ((void *)((uintptr_t)("sssd_sssdpac"))),
> sssdpac_ad_types,
> sssdpac_init,
> sssdpac_fini,
> sssdpac_flags,
> sssdpac_request_init,
> sssdpac_request_fini,
> NULL,
> NULL,
> NULL,
> NULL,
> NULL,
> sssdpac_import_authdata,
> NULL,
> NULL,
> NULL,
> sssdpac_size,
> sssdpac_externalize,
> sssdpac_internalize,
> NULL
> +};
>
> 1.7.7.6

[Jan Zelený]

First I'd like to point out that I could not try full functionality since 
there are some pieces missing on the server side. That said, I have couple 
minor comments for some patches.

Patch #0001:                                                                                         
In pac_responder.m4 you are checking for the right version of kerberos. Would 
it be possible to make the check more generic, like "if the version is 1.9 or 
higher"? Otherwise we would need to update this every time new Kerberos is 
out. The error message should be corrected accordingly (right now it only 
specifies Kerberos 1.9 is necessary)

idmap_talloc() and idmap_free(): I just noticed that we use practically the 
same functions in ldap provider (sdap_idmap.c). How about moving both 
functions to util/ and utilize them both in ldap provider and pac responder?

Patch #0002:
Ack

Patch #0003:
Nitpick: pac_store_memberships() should be more accuratelly called 
pac_store_membership()
> > You are not freeing pr_ctx in some error cases. The only place pr_ctx is
> > free'd is pac_save_memberships_done(), i.e. only when everything went ok.
> >
> > Please add some talloc_free() calls where necessary. My first guess is
> > that pac_get_domains_done() and pac_add_pac_user() are the only two
> > places where it is necessary.
>
> I have added a talloc_free() at both places. But since pr_ctx is a child
> of the client context which is freed shortly afterwards one might argue
> if the talloc_free() is even needed in pac_save_memberships_done(). But
> I would agree with you that it is a good idea to remove pr_ctx when it
> is not needed anymore.

Just for the record, I don't think the cctx context is freed before responder 
starts shutting down. That's why I asked you to free those pr_ctx contexts.

Patch #0004:
Please add build dependencies (conditionally of course) to our example spec 
file.

Patch #0005:
Ack

Patch #0006:
Ack

Patch #0007:
Are there any changes that should be done in spec file for this?

Nitpick: the "if" statement on line 361 of the patch is missing curly braces. 
The same on line 425.

Thanks
Jan

[Sumit Bose]

On Thu, Jun 14, 2012 at 04:00:32PM +0200, Jan Zelený wrote:
> First I'd like to point out that I could not try full functionality since 
> there are some pieces missing on the server side. That said, I have couple 
> minor comments for some patches.

Thank you for the review. Please find new patches attached. I added two
now patches to support the new ID range objects on the IPA server. I
didn't squash them into the other patches to hopefully make review
easier.

bye
Sumit
> Patch #0001:                                                                                         
> In pac_responder.m4 you are checking for the right version of kerberos. Would 
> it be possible to make the check more generic, like "if the version is 1.9 or 
> higher"? Otherwise we would need to update this every time new Kerberos is 
> out. The error message should be corrected accordingly (right now it only 
> specifies Kerberos 1.9 is necessary)

Corrected the message. But I would like to keep the current scheme of
detecting the MIT Kerberos version, because the used plugin interface is
currently not public (maybe in 1.11) and it should be carefully checked
if everything is working with new versions of MIT Kerberos.
> idmap_talloc() and idmap_free(): I just noticed that we use practically the 
> same functions in ldap provider (sdap_idmap.c). How about moving both 
> functions to util/ and utilize them both in ldap provider and pac responder?
>
> Patch #0002:
> Ack
>
> Patch #0003:
> Nitpick: pac_store_memberships() should be more accuratelly called 
> pac_store_membership()

renamed
> > > You are not freeing pr_ctx in some error cases. The only place pr_ctx is
> > > free'd is pac_save_memberships_done(), i.e. only when everything went ok.
> > >
> > > Please add some talloc_free() calls where necessary. My first guess is
> > > that pac_get_domains_done() and pac_add_pac_user() are the only two
> > > places where it is necessary.
> >
> > I have added a talloc_free() at both places. But since pr_ctx is a child
> > of the client context which is freed shortly afterwards one might argue
> > if the talloc_free() is even needed in pac_save_memberships_done(). But
> > I would agree with you that it is a good idea to remove pr_ctx when it
> > is not needed anymore.
>
> Just for the record, I don't think the cctx context is freed before responder 
> starts shutting down. That's why I asked you to free those pr_ctx contexts.
>
> Patch #0004:
> Please add build dependencies (conditionally of course) to our example spec 
> file.

added
> Patch #0005:
> Ack
>
> Patch #0006:
> Ack
>
> Patch #0007:
> Are there any changes that should be done in spec file for this?

no, MIT Kerberos libraries are all that is needed
> Nitpick: the "if" statement on line 361 of the patch is missing curly braces. 
> The same on line 425.

added braces

bye,
Sumit
> Thanks
> Jan

[Simo Sorce]

On Sun, 2012-06-17 at 11:38 +0200, Sumit Bose wrote:
> On Thu, Jun 14, 2012 at 04:00:32PM +0200, Jan Zelený wrote:
> > First I'd like to point out that I could not try full functionality since 
> > there are some pieces missing on the server side. That said, I have couple 
> > minor comments for some patches.
>
> Thank you for the review. Please find new patches attached. I added two
> now patches to support the new ID range objects on the IPA server. I
> didn't squash them into the other patches to hopefully make review
> easier.
>
> bye
> Sumit
> > Patch #0001:                                                                                         
> > In pac_responder.m4 you are checking for the right version of kerberos. Would 
> > it be possible to make the check more generic, like "if the version is 1.9 or 
> > higher"? Otherwise we would need to update this every time new Kerberos is 
> > out. The error message should be corrected accordingly (right now it only 
> > specifies Kerberos 1.9 is necessary)
>
> Corrected the message. But I would like to keep the current scheme of
> detecting the MIT Kerberos version, because the used plugin interface is
> currently not public (maybe in 1.11) and it should be carefully checked
> if everything is working with new versions of MIT Kerberos.
> > idmap_talloc() and idmap_free(): I just noticed that we use practically the 
> > same functions in ldap provider (sdap_idmap.c). How about moving both 
> > functions to util/ and utilize them both in ldap provider and pac responder?
> >
> > Patch #0002:
> > Ack
> >
> > Patch #0003:
> > Nitpick: pac_store_memberships() should be more accuratelly called 
> > pac_store_membership()
>
> renamed
> > > > You are not freeing pr_ctx in some error cases. The only place pr_ctx is
> > > > free'd is pac_save_memberships_done(), i.e. only when everything went ok.
> > > >
> > > > Please add some talloc_free() calls where necessary. My first guess is
> > > > that pac_get_domains_done() and pac_add_pac_user() are the only two
> > > > places where it is necessary.
> > >
> > > I have added a talloc_free() at both places. But since pr_ctx is a child
> > > of the client context which is freed shortly afterwards one might argue
> > > if the talloc_free() is even needed in pac_save_memberships_done(). But
> > > I would agree with you that it is a good idea to remove pr_ctx when it
> > > is not needed anymore.
> >
> > Just for the record, I don't think the cctx context is freed before responder 
> > starts shutting down. That's why I asked you to free those pr_ctx contexts.
> >
> > Patch #0004:
> > Please add build dependencies (conditionally of course) to our example spec 
> > file.
>
> added
> > Patch #0005:
> > Ack
> >
> > Patch #0006:
> > Ack
> >
> > Patch #0007:
> > Are there any changes that should be done in spec file for this?
>
> no, MIT Kerberos libraries are all that is needed
> > Nitpick: the "if" statement on line 361 of the patch is missing curly braces. 
> > The same on line 425.
>
> added braces

Just giving a quick look.

0. Can you add more comments on what the various functions are supposed
to do ?

1. Patch 0002 is *really* big, I would split it in smaller pieces.

2. In patch 0002 leave the gecos empty if there is no full name.

3. In pac_add_pac_user you seem to be trusting the logon_domain w/o any
check. I would rather we verify it is valid wrt the realm of the
principal that owns the PAC. Otherwise technically one trust domain
could try to attack another by giving us a same named user and injecting
a domain name of a different trusted domain.

4. In pac_add_pac_user() you are returning pac_cmd_done() whether that
whole funciton completed or had to fork off async requests, is that
intentional ?

5. SSS_PAC_ADD_PAC_USER     = 0x00E9,
  Make this 0x0101

Simo.

[Sumit Bose]

On Sun, Jun 17, 2012 at 06:47:05PM -0400, Simo Sorce wrote:
> On Sun, 2012-06-17 at 11:38 +0200, Sumit Bose wrote:
> > On Thu, Jun 14, 2012 at 04:00:32PM +0200, Jan Zelený wrote:
> > > First I'd like to point out that I could not try full functionality since 
> > > there are some pieces missing on the server side. That said, I have couple 
> > > minor comments for some patches.
> >
> > Thank you for the review. Please find new patches attached. I added two
> > now patches to support the new ID range objects on the IPA server. I
> > didn't squash them into the other patches to hopefully make review
> > easier.
> >
> > bye
> > Sumit
> > > Patch #0001:                                                                                         
> > > In pac_responder.m4 you are checking for the right version of kerberos. Would 
> > > it be possible to make the check more generic, like "if the version is 1.9 or 
> > > higher"? Otherwise we would need to update this every time new Kerberos is 
> > > out. The error message should be corrected accordingly (right now it only 
> > > specifies Kerberos 1.9 is necessary)
> >
> > Corrected the message. But I would like to keep the current scheme of
> > detecting the MIT Kerberos version, because the used plugin interface is
> > currently not public (maybe in 1.11) and it should be carefully checked
> > if everything is working with new versions of MIT Kerberos.
> > > idmap_talloc() and idmap_free(): I just noticed that we use practically the 
> > > same functions in ldap provider (sdap_idmap.c). How about moving both 
> > > functions to util/ and utilize them both in ldap provider and pac responder?
> > >
> > > Patch #0002:
> > > Ack
> > >
> > > Patch #0003:
> > > Nitpick: pac_store_memberships() should be more accuratelly called 
> > > pac_store_membership()
> >
> > renamed
> > > > > You are not freeing pr_ctx in some error cases. The only place pr_ctx is
> > > > > free'd is pac_save_memberships_done(), i.e. only when everything went ok.
> > > > >
> > > > > Please add some talloc_free() calls where necessary. My first guess is
> > > > > that pac_get_domains_done() and pac_add_pac_user() are the only two
> > > > > places where it is necessary.
> > > >
> > > > I have added a talloc_free() at both places. But since pr_ctx is a child
> > > > of the client context which is freed shortly afterwards one might argue
> > > > if the talloc_free() is even needed in pac_save_memberships_done(). But
> > > > I would agree with you that it is a good idea to remove pr_ctx when it
> > > > is not needed anymore.
> > >
> > > Just for the record, I don't think the cctx context is freed before responder 
> > > starts shutting down. That's why I asked you to free those pr_ctx contexts.
> > >
> > > Patch #0004:
> > > Please add build dependencies (conditionally of course) to our example spec 
> > > file.
> >
> > added
> > > Patch #0005:
> > > Ack
> > >
> > > Patch #0006:
> > > Ack
> > >
> > > Patch #0007:
> > > Are there any changes that should be done in spec file for this?
> >
> > no, MIT Kerberos libraries are all that is needed
> > > Nitpick: the "if" statement on line 361 of the patch is missing curly braces. 
> > > The same on line 425.
> >
> > added braces
>
> Just giving a quick look.

thank you.

I forgot to attach the very first patch, so patch number no differ by 1.
> Can you add more comments on what the various functions are supposed
>
> to do ?

comments added
> Patch 0002 is *really* big, I would split it in smaller pieces.

I moved all the utility calls into 'PAC responder: add some utility
functions' and also into the pacsrv_utils.c file.
> In patch 0002 leave the gecos empty if there is no full name.

done
> In pac_add_pac_user you seem to be trusting the logon_domain w/o any
>
> check. I would rather we verify it is valid wrt the realm of the
> principal that owns the PAC. Otherwise technically one trust domain
> could try to attack another by giving us a same named user and injecting
> a domain name of a different trusted domain.

we talked about this on irc and agreed that this check should be done by
the KDC before resigning the PAC. I opened ticket
https://fedorahosted.org/freeipa/ticket/2849 to track this.
> In pac_add_pac_user() you are returning pac_cmd_done() whether that
>
> whole funciton completed or had to fork off async requests, is that
> intentional ?

yes, as done in other responders as well pac_cmd_done() returns to the
main loop when called with EAGAIN as second parameter.
> SSS_PAC_ADD_PAC_USER     = 0x00E9,
>
> Make this 0x0101

done
> Simo.

new versions attached.

bye,
Sumit

[Jan Zelený]

Dne pondělí 18 června 2012 16:34:23, Sumit Bose napsal(a):
> On Sun, Jun 17, 2012 at 06:47:05PM -0400, Simo Sorce wrote:
> > On Sun, 2012-06-17 at 11:38 +0200, Sumit Bose wrote:
> > > On Thu, Jun 14, 2012 at 04:00:32PM +0200, Jan Zelený wrote:
> > > > First I'd like to point out that I could not try full functionality
> > > > since
> > > > there are some pieces missing on the server side. That said, I have
> > > > couple
> > > > minor comments for some patches.
> > >
> > > Thank you for the review. Please find new patches attached. I added two
> > > now patches to support the new ID range objects on the IPA server. I
> > > didn't squash them into the other patches to hopefully make review
> > > easier.
> > >
> > > bye
> > > Sumit
> > > > Patch #0001:
> > > > In pac_responder.m4 you are checking for the right version of
> > > > kerberos. Would it be possible to make the check more generic, like
> > > > "if the version is 1.9 or higher"? Otherwise we would need to update
> > > > this every time new Kerberos is out. The error message should be
> > > > corrected accordingly (right now it only specifies Kerberos 1.9 is
> > > > necessary)
> > >
> > > Corrected the message. But I would like to keep the current scheme of
> > > detecting the MIT Kerberos version, because the used plugin interface is
> > > currently not public (maybe in 1.11) and it should be carefully checked
> > > if everything is working with new versions of MIT Kerberos.
> > > > idmap_talloc() and idmap_free(): I just noticed that we use
> > > > practically the
> > > > same functions in ldap provider (sdap_idmap.c). How about moving both
> > > > functions to util/ and utilize them both in ldap provider and pac
> > > > responder?
> > > >
> > > > Patch #0002:
> > > > Ack
> > > >
> > > > Patch #0003:
> > > > Nitpick: pac_store_memberships() should be more accuratelly called
> > > > pac_store_membership()
> > >
> > > renamed
> > > > > > You are not freeing pr_ctx in some error cases. The only place
> > > > > > pr_ctx is
> > > > > > free'd is pac_save_memberships_done(), i.e. only when everything
> > > > > > went ok.
> > > > > >
> > > > > > Please add some talloc_free() calls where necessary. My first
> > > > > > guess is
> > > > > > that pac_get_domains_done() and pac_add_pac_user() are the only
> > > > > > two
> > > > > > places where it is necessary.
> > > > >
> > > > > I have added a talloc_free() at both places. But since pr_ctx is a
> > > > > child
> > > > > of the client context which is freed shortly afterwards one might
> > > > > argue
> > > > > if the talloc_free() is even needed in pac_save_memberships_done().
> > > > > But
> > > > > I would agree with you that it is a good idea to remove pr_ctx when
> > > > > it
> > > > > is not needed anymore.
> > > >
> > > > Just for the record, I don't think the cctx context is freed before
> > > > responder starts shutting down. That's why I asked you to free those
> > > > pr_ctx contexts.
> > > >
> > > > Patch #0004:
> > > > Please add build dependencies (conditionally of course) to our example
> > > > spec
> > > > file.
> > >
> > > added
> > > > Patch #0005:
> > > > Ack
> > > >
> > > > Patch #0006:
> > > > Ack
> > > >
> > > > Patch #0007:
> > > > Are there any changes that should be done in spec file for this?
> > >
> > > no, MIT Kerberos libraries are all that is needed
> > > > Nitpick: the "if" statement on line 361 of the patch is missing curly
> > > > braces. The same on line 425.
> > >
> > > added braces
> >
> > Just giving a quick look.
>
> thank you.
>
> I forgot to attach the very first patch, so patch number no differ by 1.
> > Can you add more comments on what the various functions are supposed
> >
> > to do ?
>
> comments added
> > Patch 0002 is *really* big, I would split it in smaller pieces.
>
> I moved all the utility calls into 'PAC responder: add some utility
> functions' and also into the pacsrv_utils.c file.
> > In patch 0002 leave the gecos empty if there is no full name.
>
> done
> > In pac_add_pac_user you seem to be trusting the logon_domain w/o any
> >
> > check. I would rather we verify it is valid wrt the realm of the
> > principal that owns the PAC. Otherwise technically one trust domain
> > could try to attack another by giving us a same named user and injecting
> > a domain name of a different trusted domain.
>
> we talked about this on irc and agreed that this check should be done by
> the KDC before resigning the PAC. I opened ticket
> https://fedorahosted.org/freeipa/ticket/2849 to track this.
> > In pac_add_pac_user() you are returning pac_cmd_done() whether that
> >
> > whole funciton completed or had to fork off async requests, is that
> > intentional ?
>
> yes, as done in other responders as well pac_cmd_done() returns to the
> main loop when called with EAGAIN as second parameter.
> > SSS_PAC_ADD_PAC_USER     = 0x00E9,
> >
> > Make this 0x0101
>
> done
> > Simo.
>
> new versions attached.

All issues have been addressed. If noone else has any objections, ACK.

Jan

[Stephen Gallagher]

On Thu, 2012-06-21 at 16:55 +0200, Jan Zelený wrote:
> > new versions attached.
>
> All issues have been addressed. If noone else has any objections, ACK.

Pushed to master