On 05/10/2016 04:00 PM, Jakub Hrozek wrote:
On Thu, Apr 21, 2016 at 02:54:21PM +0200, Pavel Březina wrote:
We can fail in sasl_bind_send() with ERR_AUTH_FAILED for basically
unspecified reason but we do not failover to next server. This patch should
fix it.

As said on the meeting, I didn't reproduce it and I'm not sure if it will
fix the customer issue unless they confirm it, but it seems to be a valid
patch anyway.

 From c9e3bf2726d9edba3cddc91c4ae98a87a5c1652a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 6 Apr 2016 13:20:59 +0200
Subject: [PATCH 1/2] Inform about (un)successful connection

---
  src/providers/ldap/sdap_async_connection.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/src/providers/ldap/sdap_async_connection.c 
b/src/providers/ldap/sdap_async_connection.c
index 
f9074afb0c1340c7c2a50d4df0021eee4ae0d076..435cb1bb6864b516d386fafa360d062f780b5d04
 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -2035,6 +2035,9 @@ int sdap_cli_connect_recv(struct tevent_req *req,
      if (tevent_req_is_error(req, &tstate, &err)) {
          /* mark the server as bad if connection failed */
          if (state->srv) {
+            DEBUG(SSSDBG_OP_FAILURE, "Unable to establish connection "
+                  "[%d]: %s\n", err, sss_strerror(err));
+

In file included from /sssd/src/util/util.h:54:0,
                  from /sssd/src/providers/ldap/sdap_async_connection.c:26:
/sssd/src/providers/ldap/sdap_async_connection.c: In function 
'sdap_cli_connect_recv':
/sssd/src/providers/ldap/sdap_async_connection.c:2038:38: warning: format '%d' 
expects argument of type 'int', but argument 6 has type 'uint64_t {aka long 
unsigned int}' [-Wformat=]
              DEBUG(SSSDBG_OP_FAILURE, "Unable to establish connection "
                                       ^
/sssd/src/util/debug.h:108:22: note: in definition of macro 'DEBUG'
                       format, ##__VA_ARGS__); \

Fixed.

                       ^

              be_fo_set_port_status(state->be, state->service->name,
                                    state->srv, PORT_NOT_WORKING);
          } else {
@@ -2048,6 +2051,8 @@ int sdap_cli_connect_recv(struct tevent_req *req,
          }
          return EIO;
      } else if (state->srv) {
+        DEBUG(SSSDBG_TRACE_FUNC, "Connection established.\n");
+
          be_fo_set_port_status(state->be, state->service->name,
                                state->srv, PORT_WORKING);
      }
--
2.1.0


 From 8d6f024ac77ddb8dc87d7fd693cfe5846f3a88d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 6 Apr 2016 13:21:21 +0200
Subject: [PATCH 2/2] Failover to next server if authentication fails

I was only able to test this patch 'synthetically', by setting ret to the
ERR_AUTH_FAILED value. But then it seemed to do the right thing, so ACK.

Here as well.

From dc336c16dda67c84908fc707b3bf549ea1c01ab3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 6 Apr 2016 13:20:59 +0200
Subject: [PATCH 1/2] Inform about (un)successful connection

---
 src/providers/ldap/sdap_async_connection.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index f9074afb0c1340c7c2a50d4df0021eee4ae0d076..5045a1d19b1112fdfed1167367a5a298f1601005 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -2035,6 +2035,9 @@ int sdap_cli_connect_recv(struct tevent_req *req,
     if (tevent_req_is_error(req, &tstate, &err)) {
         /* mark the server as bad if connection failed */
         if (state->srv) {
+            DEBUG(SSSDBG_OP_FAILURE, "Unable to establish connection "
+                  "[%"PRIu64"]: %s\n", err, sss_strerror(err));
+
             be_fo_set_port_status(state->be, state->service->name,
                                   state->srv, PORT_NOT_WORKING);
         } else {
@@ -2048,6 +2051,8 @@ int sdap_cli_connect_recv(struct tevent_req *req,
         }
         return EIO;
     } else if (state->srv) {
+        DEBUG(SSSDBG_TRACE_FUNC, "Connection established.\n");
+
         be_fo_set_port_status(state->be, state->service->name,
                               state->srv, PORT_WORKING);
     }
-- 
2.1.0

From 25f507d6ec3951c248767f12ead23f6e49eba457 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 6 Apr 2016 13:21:21 +0200
Subject: [PATCH 2/2] Failover to next server if authentication fails

---
 src/providers/ldap/sdap_id_op.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/providers/ldap/sdap_id_op.c b/src/providers/ldap/sdap_id_op.c
index 6ad7e8188dc6211907cac88e90e4feff5080695c..3a3de3643711fc2e604b876f401a88b486f941d5 100644
--- a/src/providers/ldap/sdap_id_op.c
+++ b/src/providers/ldap/sdap_id_op.c
@@ -603,6 +603,7 @@ static void sdap_id_op_connect_done(struct tevent_req *subreq)
             case EIO:
             case EFAULT:
             case ETIMEDOUT:
+            case ERR_AUTH_FAILED:
                 break;
 
             default:
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to