Re: [Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist
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
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
[Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist
added verification for issue reported in ticket 5088 and sanity checks requested in review for patch 0014 From 03e55b155bfe517c9be35c9c6c3bd44401716442 Mon Sep 17 00:00:00 2001 From: Ludwig Krispenz lkris...@redhat.com Date: Tue, 30 Jun 2015 11:05:32 +0200 Subject: [PATCH] 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 | 9 +++-- daemons/ipa-slapi-plugins/topology/topology_pre.c | 40 +- daemons/ipa-slapi-plugins/topology/topology_util.c | 6 ++-- 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/daemons/ipa-slapi-plugins/topology/topology_post.c b/daemons/ipa-slapi-plugins/topology/topology_post.c index 60011c7e96306fc1f496d4cda0082201f063808e..27bda3c13691b1da23b60c7b2f73f8c38cde6e35 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. @@ -227,7 +232,7 @@ ipa_topo_post_del(Slapi_PBlock *pb) TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(del_entry); TopoReplicaSegment *tsegm; 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..6883c1af6ab11dfb393d93d6af89fdbc9d65be05 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; +TopoReplicaSegment *tsegm = NULL; TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry); -tsegm = ipa_topo_util_find_segment(tconf, add_entry); +if (tconf == NULL ) { +*errtxt = slapi_ch_smprintf(Segment configuration suffix not found + . Add rejected.\n); +slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM, +topology not configured for segment\n); +rc =
Re: [Freeipa-devel] [PATCH 0017] dirsrv crash on segment add if suffix does not exist
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; +