Re: [Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist

2015-07-01 Thread thierry bordaz

On 06/30/2015 04:50 PM, Ludwig Krispenz wrote:

new patch attached

On 06/30/2015 03:37 PM, thierry bordaz wrote:

On 06/30/2015 12:07 PM, Ludwig Krispenz wrote:
added verification for issue reported in ticket 5088 and sanity 
checks requested in review for patch 0014




Hello,

The fix looks good except those sanity settings:

  * In ipa_topo_post_del, tsegm needs to be NULL initialized
  * In ipa_topo_check_segment_is_valid or ipa_topo_pre_add, I think
*errtxt should be initialized to NULL

thanks
thierry




ACK

thanks
thierry
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist

2015-07-01 Thread Tomas Babej


On 07/01/2015 12:11 PM, thierry bordaz wrote:
 On 06/30/2015 04:50 PM, Ludwig Krispenz wrote:
 new patch attached

 On 06/30/2015 03:37 PM, thierry bordaz wrote:
 On 06/30/2015 12:07 PM, Ludwig Krispenz wrote:
 added verification for issue reported in ticket 5088 and sanity
 checks requested in review for patch 0014


 Hello,

 The fix looks good except those sanity settings:

   * In ipa_topo_post_del, tsegm needs to be NULL initialized
   * In ipa_topo_check_segment_is_valid or ipa_topo_pre_add, I think
 *errtxt should be initialized to NULL 

 thanks
 thierry


 ACK
 
 thanks
 thierry
 
 

Pushed to master: 5b76df4e7335c723f3fb14ef809e4d71e53509c9

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist

2015-06-30 Thread Ludwig Krispenz

new patch attached

On 06/30/2015 03:37 PM, thierry bordaz wrote:

On 06/30/2015 12:07 PM, Ludwig Krispenz wrote:
added verification for issue reported in ticket 5088 and sanity 
checks requested in review for patch 0014




Hello,

The fix looks good except those sanity settings:

  * In ipa_topo_post_del, tsegm needs to be NULL initialized
  * In ipa_topo_check_segment_is_valid or ipa_topo_pre_add, I think
*errtxt should be initialized to NULL

thanks
thierry



From 042d11abe5b4ec9040bdf4c0709b74f46f5c38f5 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz lkris...@redhat.com
Date: Tue, 30 Jun 2015 16:02:16 +0200
Subject: [PATCH] v2 improve processing of invalid data.

reject attempts to add segments to suffixes, which do not exist or are not configured.
check completenes and validity of segment attributes

cf ticket 5088: https://fedorahosted.org/freeipa/ticket/5088
---
 daemons/ipa-slapi-plugins/topology/topology_post.c | 11 --
 daemons/ipa-slapi-plugins/topology/topology_pre.c  | 40 +-
 daemons/ipa-slapi-plugins/topology/topology_util.c |  6 ++--
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/topology/topology_post.c b/daemons/ipa-slapi-plugins/topology/topology_post.c
index c01505bdedc1b09488b21561315c721dc3847969..4eb3c2fd10643658804943112c1466091f9f624e 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_post.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_post.c
@@ -66,9 +66,14 @@ ipa_topo_post_add(Slapi_PBlock *pb)
 /* initialize the shared topology data for a replica */
 break;
 case TOPO_SEGMENT_ENTRY: {
-TopoReplicaSegment *tsegm;
+TopoReplicaSegment *tsegm = NULL;
 TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry);
 char *status;
+if (tconf == NULL) {
+slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
+ipa_topo_post_add - config area for segment not found\n);
+break;
+}
 /* TBD check that one node is the current server and
  * that the other node is also managed by the
  * shared config.
@@ -225,9 +230,9 @@ ipa_topo_post_del(Slapi_PBlock *pb)
 case TOPO_SEGMENT_ENTRY: {
 /* check if corresponding agreement exists and delete */
 TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(del_entry);
-TopoReplicaSegment *tsegm;
+TopoReplicaSegment *tsegm = NULL;
 char *status;
-tsegm = ipa_topo_util_find_segment(tconf, del_entry);
+if (tconf) tsegm = ipa_topo_util_find_segment(tconf, del_entry);
 if (tsegm == NULL) {
 slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
 segment to be deleted does not exist\n);
diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index c324cf69ed8396431a3171aefa5a02fc9e1dd1d9..0b9f8900940eda4429bb26fe6fc8118e2a71932b 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -284,7 +284,7 @@ ipa_topo_connection_exists(struct node_fanout *fanout, char* from, char *to)
 }
 
 int
-ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
+ipa_topo_check_segment_is_valid(Slapi_PBlock *pb, char **errtxt)
 {
 int rc = 0;
 Slapi_Entry *add_entry;
@@ -307,20 +307,38 @@ ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
 char *leftnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentLeftNode);
 char *rightnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentRightNode);
 char *dir = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentDirection);
-if (strcasecmp(dir,SEGMENT_DIR_BOTH)  strcasecmp(dir,SEGMENT_DIR_LEFT_ORIGIN) 
+if (leftnode == NULL || rightnode == NULL || dir == NULL) {
+*errtxt = slapi_ch_smprintf(Segment definition is incomplete
+   . Add rejected.\n);
+rc = 1;
+} else if (strcasecmp(dir,SEGMENT_DIR_BOTH)  strcasecmp(dir,SEGMENT_DIR_LEFT_ORIGIN) 
 strcasecmp(dir,SEGMENT_DIR_RIGHT_ORIGIN)) {
+*errtxt = slapi_ch_smprintf(Segment has unsupported direction
+   . Add rejected.\n);
 slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
 segment has unknown direction: %s\n, dir);
 rc = 1;
 } else if (0 == strcasecmp(leftnode,rightnode)) {
+*errtxt = slapi_ch_smprintf(Segment is self referential
+   . Add rejected.\n);
 slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
 segment is self referential\n);
 rc = 1;
 } else {
-TopoReplicaSegment *tsegm;
+