ldap/servers/plugins/acl/acleffectiverights.c | 29 +++++-- ldap/servers/plugins/chainingdb/cb_utils.c | 15 +++ ldap/servers/plugins/deref/deref.c | 11 ++ ldap/servers/slapd/back-ldbm/ldbm_search.c | 101 ++++++++++++++++++-------- ldap/servers/slapd/opshared.c | 12 ++- ldap/servers/slapd/proxyauth.c | 13 ++- 6 files changed, 135 insertions(+), 46 deletions(-)
New commits: commit e84c63f7c183577868194b66b3e7c0cd4f54af09 Author: Thierry bordaz (tbordaz) tbordaz@redhat.com Date: Tue Mar 12 10:41:14 2013 +0100
Ticket 600 - Server should return unavailableCriticalExtension when processing a badly formed critical control
Bug Description:
When the server receives a critical control and fails to process it, it should return the result LDAP_UNAVAILABLE_CRITICAL_EXTENSION (RFC 4511 4.1.11). The server may fails to process it because of various reasons (unable to decode the control, unwilling to perform, invalid values...)
Fix Description: The fix consists, when an error condition is detected while processing a control, to check the 'critical' flag to determine which result to return
https://fedorahosted.org/389/ticket/600
Reviewed by: Mark Reynolds (thanks mark for all these reviews)
Platforms tested: Fedora 17
Flag Day: no
Doc impact: no
QE impact: some acceptance tests will need changes (at least filter fltr_97 and fltr_99
diff --git a/ldap/servers/plugins/acl/acleffectiverights.c b/ldap/servers/plugins/acl/acleffectiverights.c index 89ce79b..e4537af 100644 --- a/ldap/servers/plugins/acl/acleffectiverights.c +++ b/ldap/servers/plugins/acl/acleffectiverights.c @@ -40,6 +40,8 @@ #endif
+#include <ldap.h> + #include "acl.h"
/* safer than doing strcat unprotected */ @@ -218,7 +220,10 @@ _ger_parse_control ( { aclutil_str_append ( errbuf, "get-effective-rights: missing subject" ); slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "%s\n", *errbuf ); - return LDAP_INVALID_SYNTAX; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_INVALID_SYNTAX; }
if ( strncasecmp ( "dn:", subjectber->bv_val, 3 ) == 0 ) @@ -239,7 +244,10 @@ _ger_parse_control ( { aclutil_str_append ( errbuf, "get-effective-rights: ber_init failed for the subject" ); slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "%s\n", *errbuf ); - return LDAP_OPERATIONS_ERROR; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_OPERATIONS_ERROR; } /* "a" means to allocate storage as needed for octet string */ if ( ber_scanf (ber, "a", &orig) == LBER_ERROR ) @@ -247,7 +255,10 @@ _ger_parse_control ( aclutil_str_append ( errbuf, "get-effective-rights: invalid ber tag in the subject" ); slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "%s\n", *errbuf ); ber_free ( ber, 1 ); - return LDAP_INVALID_SYNTAX; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_INVALID_SYNTAX; } ber_free ( ber, 1 ); } @@ -263,7 +274,10 @@ _ger_parse_control ( aclutil_str_append ( errbuf, "get-effective-rights: subject is not dnAuthzId" ); slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "%s\n", *errbuf ); slapi_ch_free_string(&orig); - return LDAP_INVALID_SYNTAX; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_INVALID_SYNTAX; }
/* memmove is safe for overlapping copy */ @@ -273,7 +287,10 @@ _ger_parse_control ( aclutil_str_append (errbuf, orig); slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "%s\n", *errbuf); slapi_ch_free_string(&orig); - return LDAP_INVALID_SYNTAX; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_INVALID_SYNTAX; } slapi_ch_free_string(&orig); *subjectndn = normed; @@ -1023,7 +1040,7 @@ acl_get_effective_rights ( char *gerstr = NULL; size_t gerstrsize = 0; size_t gerstrcap = 0; - int iscritical = 1; + int iscritical = 0; /* critical may be missing or false http://tools.ietf.org/html/draft-ietf-ldapext-acl-model-08 */ int rc = LDAP_SUCCESS;
*errbuf = '\0'; diff --git a/ldap/servers/plugins/chainingdb/cb_utils.c b/ldap/servers/plugins/chainingdb/cb_utils.c index f28f5d3..f35943b 100644 --- a/ldap/servers/plugins/chainingdb/cb_utils.c +++ b/ldap/servers/plugins/chainingdb/cb_utils.c @@ -160,21 +160,30 @@ int cb_forward_operation(Slapi_PBlock * pb ) { if ((ber = ber_init(ctl_value)) == NULL) { slapi_log_error(SLAPI_LOG_PLUGIN, CB_PLUGIN_SUBSYSTEM, "cb_forward_operation: ber_init: Memory allocation failed"); - return LDAP_NO_MEMORY; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_NO_MEMORY; } rc = ber_scanf(ber,"i",&hops); if (LBER_ERROR == rc) { slapi_log_error(SLAPI_LOG_PLUGIN, CB_PLUGIN_SUBSYSTEM, "Loop detection control badly encoded."); ber_free(ber,1); - return LDAP_LOOP_DETECT; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_LOOP_DETECT; } if (hops <= 0) { slapi_log_error(SLAPI_LOG_PLUGIN, CB_PLUGIN_SUBSYSTEM, "Max hop count exceeded. Loop detected.\n"); ber_free(ber,1); - return LDAP_LOOP_DETECT; + if (iscritical) + return LDAP_UNAVAILABLE_CRITICAL_EXTENSION; /* RFC 4511 4.1.11 */ + else + return LDAP_LOOP_DETECT; } ber_free(ber,1); } diff --git a/ldap/servers/plugins/deref/deref.c b/ldap/servers/plugins/deref/deref.c index aec48ed..2601170 100644 --- a/ldap/servers/plugins/deref/deref.c +++ b/ldap/servers/plugins/deref/deref.c @@ -395,6 +395,9 @@ deref_parse_ctrl_value(DerefSpecList *speclist, const struct berval *ctrlbv, int len = -1; /* reset */ if ((LBER_ERROR == ber_scanf(ber, "{a{v}}", &derefattr, &attrs)) || !derefattr || !attrs || !attrs[0]){ + if (critical) + *ldapcode = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + else *ldapcode = LDAP_PROTOCOL_ERROR; if (!derefattr) { *ldaperrtext = "Missing dereference attribute name"; @@ -442,6 +445,7 @@ deref_pre_search(Slapi_PBlock *pb) const char *incompatible = NULL; DerefSpecList *speclist = NULL; int ii; + int iscritical = 0;
slapi_log_error(SLAPI_LOG_TRACE, DEREF_PLUGIN_SUBSYSTEM, "--> deref_pre_search\n"); @@ -463,18 +467,22 @@ deref_pre_search(Slapi_PBlock *pb) "No control value specified for dereference control\n"); ldapcode = LDAP_PROTOCOL_ERROR; ldaperrtext = "The dereference control must have a value"; + iscritical = ctrl->ldctl_iscritical; } else if (!ctrl->ldctl_value.bv_val) { slapi_log_error(SLAPI_LOG_FATAL, DEREF_PLUGIN_SUBSYSTEM, "No control value specified for dereference control\n"); ldapcode = LDAP_PROTOCOL_ERROR; ldaperrtext = "The dereference control must have a value"; + iscritical = ctrl->ldctl_iscritical; } else if (!ctrl->ldctl_value.bv_val[0] || !ctrl->ldctl_value.bv_len) { slapi_log_error(SLAPI_LOG_FATAL, DEREF_PLUGIN_SUBSYSTEM, "Empty control value specified for dereference control\n"); ldapcode = LDAP_PROTOCOL_ERROR; ldaperrtext = "The dereference control must have a non-empty value"; + iscritical = ctrl->ldctl_iscritical; } else { derefctrl = ctrl; + iscritical = ctrl->ldctl_iscritical; } } else if (deref_incompatible_ctrl(ctrl->ldctl_oid)) { incompatible = ctrl->ldctl_oid; @@ -509,6 +517,9 @@ deref_pre_search(Slapi_PBlock *pb) }
if (ldapcode != LDAP_SUCCESS) { + if (iscritical) { + ldapcode = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + } slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &ldapcode); slapi_send_ldap_result(pb, ldapcode, NULL, (char *)ldaperrtext, 0, NULL); delete_DerefSpecList(&speclist); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index 5f085f9..70a2ee3 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -43,6 +43,8 @@ /* search.c - ldbm backend search function */ /* view with ts=4 */
+#include <ldap.h> + #include "back-ldbm.h" #include "vlv_srch.h"
@@ -391,18 +393,36 @@ ldbm_back_search( Slapi_PBlock *pb ) if ( NULL != controls ) { /* Are we being asked to sort the results ? */ - sort = slapi_control_present( controls, LDAP_CONTROL_SORTREQUEST, &sort_spec, &is_sorting_critical_orig ); + sort = slapi_control_present(controls, LDAP_CONTROL_SORTREQUEST, &sort_spec, &is_sorting_critical_orig); if(sort) { rc = parse_sort_spec(sort_spec, &sort_control); if (rc) { /* Badly formed SORT control */ - return ldbm_back_search_cleanup(pb, li, sort_control, - LDAP_PROTOCOL_ERROR, "Sort Control", + if (is_sorting_critical_orig) { + /* RFC 4511 4.1.11 the server must not process the operation + * and return LDAP_UNAVAILABLE_CRITICAL_EXTENSION + */ + return ldbm_back_search_cleanup(pb, li, sort_control, + LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "Sort Control", SLAPI_FAIL_GENERAL, NULL, NULL); + } else { + PRUint64 conn_id; + int op_id; + + /* Just ignore the control */ + sort = 0; + slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id); + slapi_pblock_get(pb, SLAPI_OPERATION_ID, &op_id); + + LDAPDebug(LDAP_DEBUG_ANY, + "Warning: Sort control ignored for conn=%d op=%d\n", + conn_id, op_id, 0); + } + } else { + /* set this operation includes the server side sorting */ + operation->o_flags |= OP_FLAG_SERVER_SIDE_SORTING; } - /* set this operation includes the server side sorting */ - operation->o_flags |= OP_FLAG_SERVER_SIDE_SORTING; } is_sorting_critical = is_sorting_critical_orig;
@@ -414,36 +434,55 @@ ldbm_back_search( Slapi_PBlock *pb ) rc = vlv_parse_request_control( be, vlv_spec, &vlv_request_control ); if (rc != LDAP_SUCCESS) { /* Badly formed VLV control */ - return ldbm_back_search_cleanup(pb, li, sort_control, - rc, "VLV Control", SLAPI_FAIL_GENERAL, + if (is_vlv_critical) { + /* RFC 4511 4.1.11 the server must not process the operation + * and return LDAP_UNAVAILABLE_CRITICAL_EXTENSION + */ + return ldbm_back_search_cleanup(pb, li, sort_control, + LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "VLV Control", SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); - } - { - /* Access Control Check to see if the client is allowed to use the VLV Control. */ - Slapi_Entry *feature; - char dn[128]; - char *dummyAttr = "dummy#attr"; - char *dummyAttrs[2] = { NULL, NULL }; - - dummyAttrs[0] = dummyAttr; - - /* This dn is normalized. */ - PR_snprintf(dn,sizeof(dn),"dn: oid=%s,cn=features,cn=config",LDAP_CONTROL_VLVREQUEST); - feature= slapi_str2entry(dn,0); - rc = plugin_call_acl_plugin (pb, feature, dummyAttrs, NULL, SLAPI_ACL_READ, ACLPLUGIN_ACCESS_DEFAULT, NULL); - slapi_entry_free(feature); - if (rc != LDAP_SUCCESS) { - /* Client isn't allowed to do this. */ - return ldbm_back_search_cleanup(pb, li, sort_control, - rc, "VLV Control", SLAPI_FAIL_GENERAL, + } else { + PRUint64 conn_id; + int op_id; + + /* Just ignore the control */ + virtual_list_view = 0; + slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id); + slapi_pblock_get(pb, SLAPI_OPERATION_ID, &op_id); + + LDAPDebug(LDAP_DEBUG_ANY, + "Warning: VLV control ignored for conn=%d op=%d\n", + conn_id, op_id, 0); + } + + } else { + { + /* Access Control Check to see if the client is allowed to use the VLV Control. */ + Slapi_Entry *feature; + char dn[128]; + char *dummyAttr = "dummy#attr"; + char *dummyAttrs[2] = {NULL, NULL}; + + dummyAttrs[0] = dummyAttr; + + /* This dn is normalized. */ + PR_snprintf(dn, sizeof (dn), "dn: oid=%s,cn=features,cn=config", LDAP_CONTROL_VLVREQUEST); + feature = slapi_str2entry(dn, 0); + rc = plugin_call_acl_plugin(pb, feature, dummyAttrs, NULL, SLAPI_ACL_READ, ACLPLUGIN_ACCESS_DEFAULT, NULL); + slapi_entry_free(feature); + if (rc != LDAP_SUCCESS) { + /* Client isn't allowed to do this. */ + return ldbm_back_search_cleanup(pb, li, sort_control, + rc, "VLV Control", SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + } } + /* + * Sorting must always be critical for VLV; Force it be so. + */ + is_sorting_critical = 1; + virtual_list_view = 1; } - /* - * Sorting must always be critical for VLV; Force it be so. - */ - is_sorting_critical= 1; - virtual_list_view= 1; } else { diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index 747b634..42b0935 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -431,7 +431,10 @@ op_shared_search (Slapi_PBlock *pb, int send_result) if ((ctl_value->bv_len == 0) || (ctl_value->bv_val == NULL)) { rc = -1; - send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0, NULL); + if (iscritical) + send_ldap_result(pb, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, NULL, NULL, 0, NULL); + else + send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0, NULL); goto free_and_return_nolock; } else @@ -440,7 +443,10 @@ op_shared_search (Slapi_PBlock *pb, int send_result) if (be_name == NULL) { rc = -1; - send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0, NULL); + if (iscritical) + send_ldap_result(pb, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, NULL, NULL, 0, NULL); + else + send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0, NULL); goto free_and_return_nolock; } } @@ -496,7 +502,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result) } else { /* parse paged-results-control failed */ if (iscritical) { /* return an error since it's critical */ - send_ldap_result(pb, rc, NULL, + send_ldap_result(pb, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, NULL, "Simple Paged Results Search failed", 0, NULL); goto free_and_return; diff --git a/ldap/servers/slapd/proxyauth.c b/ldap/servers/slapd/proxyauth.c index 562ac93..817adce 100644 --- a/ldap/servers/slapd/proxyauth.c +++ b/ldap/servers/slapd/proxyauth.c @@ -40,6 +40,8 @@ # include <config.h> #endif
+#include <ldap.h> + #include "slap.h"
#define BEGIN do { @@ -219,14 +221,19 @@ proxyauth_get_dn( Slapi_PBlock *pb, char **proxydnp, char **errtextp ) rv = parse_LDAPProxyAuth(spec_ber, version, errtextp, &spec); if (LDAP_SUCCESS != rv) { if ( critical ) { - lderr = rv; - } + lderr = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + } else { + lderr = rv; + } break; }
dn = slapi_ch_strdup(spec->auth_dn); if (slapi_dn_isroot(dn) ) { - lderr = LDAP_UNWILLING_TO_PERFORM; + if (critical) + lderr = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + else + lderr = LDAP_UNWILLING_TO_PERFORM; *errtextp = "Proxy dn should not be rootdn"; break;
389-commits@lists.fedoraproject.org
-
thierry bordaz