On Mon, Jan 11, 2016 at 01:39:33PM -0500, Simo Sorce wrote:
The following 2 patches change the connection setup code to be more
flexible.
They are the groundwork to add a new secrets[1] responder that uses a
REST API over a unix socket and therefore requires a different protocol
handler.
The first patch move some protocol and state data into opaque pointers
so that responders can arbitrarily set them.
Ticket:
https://fedorahosted.org/sssd/ticket/2918
The second patch adds the ability to use socket activation for
responders. Although this is not currently wired in for use in
currently.
It is related to:
https://fedorahosted.org/sssd/ticket/2243
These patches are being worked on on my secsrv branch here:
https://fedorapeople.org/cgit/simo/public_git/sssd.git/log/?h=secsrv
[1]
https://fedorahosted.org/sssd/wiki/DesignDocs/SecretsService
--
Simo Sorce * Red Hat, Inc * New York
From e82a6c4fc5bd0c6dc264961c5b0fa4f97372e984 Mon Sep 17 00:00:00
2001
From: Simo Sorce <simo(a)redhat.com>
Date: Fri, 8 Jan 2016 17:51:06 -0500
Subject: [PATCH] Responders: Make the client context more generic
This is useufl to allow reusing the responder code with other protocols.
Store protocol data and responder state data behind opaque pointers and
use tallog_get_type to check they are of the right type.
This also allows to store per responder state_ctx so that, for example,
the autofs responder does not have to carry useless variables used only
by the nss responder.
Resolves:
https://fedorahosted.org/sssd/ticket/2918
There are some new Coverity warnings with this code:
Error: NEGATIVE_RETURNS (CWE-394):
sssd-1.13.90/src/responder/common/responder_common.c:756: var_tested_neg: Assigning:
"rctx->lfd" = a negative value.
sssd-1.13.90/src/responder/common/responder_common.c:790: negative_returns:
"rctx->lfd" is passed to a parameter that cannot be negative.
sssd-1.13.90/src/responder/common/responder_common.c:745:5: neg_sink_parm_call: Passing
"rctx->lfd" to "close", which cannot accept a negative number.
# 788| #endif
# 789|
# 790|-> ret = set_unix_socket(rctx, conn_setup);
# 791| if (ret != EOK) {
# 792| DEBUG(SSSDBG_FATAL_FAILURE, "Fatal error initializing
sockets\n");
Error: NEGATIVE_RETURNS (CWE-394):
sssd-1.13.90/src/responder/common/responder_common.c:757: var_tested_neg: Assigning:
"rctx->priv_lfd" = a negative value.
sssd-1.13.90/src/responder/common/responder_common.c:790: negative_returns:
"rctx->priv_lfd" is passed to a parameter that cannot be negative.
sssd-1.13.90/src/responder/common/responder_common.c:746:5: neg_sink_parm_call: Passing
"rctx->priv_lfd" to "close", which cannot accept a negative
number.
# 788| #endif
# 789|
# 790|-> ret = set_unix_socket(rctx, conn_setup);
# 791| if (ret != EOK) {
# 792| DEBUG(SSSDBG_FATAL_FAILURE, "Fatal error initializing
sockets\n");
But in general it looks good to me. I need to run more tests, though..
From 796ad20628994e671b9af903ed396eb8103d18c4 Mon Sep 17 00:00:00
2001
From: Simo Sorce <simo(a)redhat.com>
Date: Thu, 7 Jan 2016 10:17:38 -0500
Subject: [PATCH 2/2] Responders: Add support for socket activation
Add helper that uses systemd socket activation if available to accept a
pre-listining socket at startup.
Related:
https://fedorahosted.org/sssd/ticket/2913
[...]
+ ret = sd_listen_fds(1);
+ if (ret < 0) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Unexpected error probing for active sockets. "
+ "Will proceed with no sockets. [Error %d (%s)]\n",
+ -ret, sss_strerror(-ret));
+ } else if (ret > numfds) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Too many activated sockets have been found, "
+ "expected %d, found %d\n", numfds, ret);
+ ret = E2BIG;
+ goto done;
+ }
+
+ if (ret == numfds) {
Is this exact comparison correct? If we had a deamon that has both public
and private sockets and the client only wrote to the public socket,
wouldn't sd_listen_fds() only return 1?
> + rctx->lfd = SD_LISTEN_FDS_START;
> + ret = sss_fd_nonblocking(rctx->lfd);
> + if (ret != EOK) goto done;
> + if (numfds == 2) {
> + rctx->priv_lfd = SD_LISTEN_FDS_START + 1;
> + ret = sss_fd_nonblocking(rctx->priv_lfd);
> + if (ret != EOK) goto done;
> + }
> + }
> +#endif
> +
> + ret = set_unix_socket(rctx, conn_setup);
> + if (ret != EOK) {
> + DEBUG(SSSDBG_FATAL_FAILURE, "Fatal error initializing
sockets\n");
> + goto done;
> + }
> +
> +done:
> + return ret;
> +}
> +