Commit 5f4aaffc authored by Sebastien Decugis's avatar Sebastien Decugis
Browse files

Fix broken 'almostcasecmp' logic

parent 5b6434cd
......@@ -731,8 +731,8 @@ int rgw_clients_create_origin(struct rgw_radius_msg_meta *msg, struct rgw_client
*/
/* first, check if the nas_id is the fqdn of the peer or a known alias */
if (!fd_os_almostcasecmp(nas_id + 1, nas_id_len,
cli->fqdn, cli->fqdn_len)) {
if (!fd_os_almostcasesrch(nas_id + 1, nas_id_len,
cli->fqdn, cli->fqdn_len, NULL)) {
TRACE_DEBUG(FULL, "NAS-Identifier contains the fqdn of the client");
found = 1;
} else {
......
......@@ -93,7 +93,7 @@ static int str_match(struct ard_criteria * c, uint8_t *s, size_t l, int is0term,
*match = 0;
if (c->is_regex == 0) {
if ( ! fd_os_almostcasecmp(c->s, c->sl, s, l) )
if ( ! fd_os_almostcasesrch(c->s, c->sl, s, l, NULL) )
*match = 1;
} else {
int err;
......
......@@ -149,7 +149,7 @@ int redir_fwd_cb(void * cbdata, struct msg ** msg)
/* The list is kept ordered by id so that it is faster to compare to candidates later */
for (li = task.rh.next; li != &task.rh; li = li->next) {
struct redir_host * nhost = li->o;
if ( fd_os_almostcasecmp(id, len, nhost->id, nhost->len) <= 0 )
if ( fd_os_cmp(id, len, nhost->id, nhost->len) <= 0 )
break;
}
fd_list_insert_before(li, &h->chain);
......
......@@ -170,7 +170,7 @@ static int apply_rule(struct redir_entry * e, struct msg * msg, struct fd_list *
/* Special case: ALL_HOST rules: we decrease the score of the Origin-Host if present */
if (e->type == ALL_HOST) {
cmp = fd_os_almostcasecmp(cand->diamid, cand->diamidlen, e->data.host.s, e->data.host.l);
cmp = fd_os_almostcasesrch(cand->diamid, cand->diamidlen, e->data.host.s, e->data.host.l, NULL);
if (!cmp) {
TRACE_DEBUG(FULL, "Redirect msg %p: peer '%.*s' += %d (previous ALL_HOST Redirect originated from this peer)", msg, cand->diamidlen, cand->diamid, FD_SCORE_SENT_REDIRECT);
cand->score += FD_SCORE_SENT_REDIRECT;
......@@ -197,19 +197,20 @@ static int apply_rule(struct redir_entry * e, struct msg * msg, struct fd_list *
/* the candidates list is not guaranteed to be ordered at this time, so we cannot avoid the two imbricated loops */
struct rtd_candidate * cand = (struct rtd_candidate *) lic;
/* Is this candidate in the "Redirect-Host" list ? */
/* Is this candidate in the "Redirect-Host" list ? We must search caseinsentive here. */
for (lirh = e->target_peers_list.next; lirh != &e->target_peers_list; lirh = lirh->next) {
struct redir_host * host = lirh->o;
int cont;
cmp = fd_os_cmp( cand->diamid, cand->diamidlen, host->id, host->len );
if (cmp < 0)
break; /* targets are ordered */
cmp = fd_os_almostcasesrch( cand->diamid, cand->diamidlen, host->id, host->len, &cont );
if (cmp == 0) {
TRACE_DEBUG(FULL, "Redirect msg %p: peer '%.*s' += %d (rule t:%d @%p)", msg, cand->diamidlen, cand->diamid, redirects_usages[e->type].score, e->type, e);
cand->score += redirects_usages[e->type].score;
break;
}
if (!cont)
break;
}
}
......
......@@ -620,14 +620,21 @@ int fd_os_cmp_int(os0_t os1, size_t os1sz, os0_t os2, size_t os2sz);
#define fd_os_cmp(_o1, _l1, _o2, _l2) fd_os_cmp_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2)
/* A roughly case-insensitive variant, which actually only compares ASCII chars (0-127) in a case-insentitive maneer
-- this should be OK with (old) DNS names. Not sure how it works with IDN...
-- it also clearly does not support locales where a lowercase letter uses more space than upper case, such as ß -> ss
It is slower than fd_os_cmp...
This function should give the same order as fd_os_cmp, except when it finds 2 strings to be equal.
-- it does not support locales where a lowercase letter uses more space than upper case, such as ß -> ss
It is slower than fd_os_cmp.
Note that the result is NOT the same as strcasecmp !!!
*/
int fd_os_almostcasecmp_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz);
#define fd_os_almostcasecmp(_o1, _l1, _o2, _l2) fd_os_almostcasecmp_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2)
This function gives the same order as fd_os_cmp, except when it finds 2 strings to be equal.
However this is not always sufficient:
for example fd_os_cmp gives: "Ac" < "aB" < "aa"
if you attempt to fd_os_almostcasesrch "Aa" you will actually have to go past "aB" which is > "Aa".
Therefore you can use the maybefurther parameter.
This parameter is 1 on return if os1 may have been stored further that os2 (assuming os2 values are ordered by fd_os_cmp)
and 0 if we are sure that it is not the case.
When looping through a list of fd_os_cmp classified values, this parameter must be used to stop looping, in addition to the comp result.
*/
int fd_os_almostcasesrch_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz, int * maybefurther);
#define fd_os_almostcasesrch(_o1, _l1, _o2, _l2, _mb) fd_os_almostcasesrch_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2, _mb)
/* Analyze a DiameterURI and return its components.
Return EINVAL if the URI is not valid.
......
......@@ -303,8 +303,8 @@ static int save_remote_CE_info(struct msg * msg, struct fd_peer * peer, struct f
}
/* We check that the value matches what we know, otherwise disconnect the peer */
if (fd_os_almostcasecmp(hdr->avp_value->os.data, hdr->avp_value->os.len,
peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen)) {
if (fd_os_almostcasesrch(hdr->avp_value->os.data, hdr->avp_value->os.len,
peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen, NULL)) {
TRACE_DEBUG(INFO, "Received a message with Origin-Host set to '%.*s' while expecting '%s'\n",
hdr->avp_value->os.len, hdr->avp_value->os.data, peer->p_hdr.info.pi_diamid);
error->pei_errcode = "ER_DIAMETER_AVP_NOT_ALLOWED";
......@@ -812,7 +812,7 @@ int fd_p_ce_process_receiver(struct fd_peer * peer)
/* Validate the realm if needed */
if (peer->p_hdr.info.config.pic_realm) {
size_t len = strlen(peer->p_hdr.info.config.pic_realm);
if (fd_os_almostcasecmp(peer->p_hdr.info.config.pic_realm, len, peer->p_hdr.info.runtime.pir_realm, peer->p_hdr.info.runtime.pir_realmlen)) {
if (fd_os_almostcasesrch(peer->p_hdr.info.config.pic_realm, len, peer->p_hdr.info.runtime.pir_realm, peer->p_hdr.info.runtime.pir_realmlen, NULL)) {
TRACE_DEBUG(INFO, "Rejected CER from peer '%s', realm mismatch with configured value (returning DIAMETER_UNKNOWN_PEER).\n", peer->p_hdr.info.pi_diamid);
pei.pei_errcode = "DIAMETER_UNKNOWN_PEER"; /* maybe AVP_NOT_ALLOWED would be better fit? */
goto error_abort;
......
......@@ -93,7 +93,7 @@ int fd_peer_alloc(struct fd_peer ** ptr)
int fd_peer_add ( struct peer_info * info, char * orig_dbg, void (*cb)(struct peer_info *, void *), void * cb_data )
{
struct fd_peer *p = NULL;
struct fd_list * li;
struct fd_list * li, *li_inf;
int ret = 0;
TRACE_ENTRY("%p %p %p %p", info, orig_dbg, cb, cb_data);
......@@ -142,16 +142,22 @@ int fd_peer_add ( struct peer_info * info, char * orig_dbg, void (*cb)(struct pe
/* Ok, now check if we don't already have an entry with the same Diameter Id, and insert this one */
CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_peers_rw) );
li_inf = &fd_g_peers;
for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
struct fd_peer * next = (struct fd_peer *)li;
int cmp = fd_os_almostcasecmp( p->p_hdr.info.pi_diamid, p->p_hdr.info.pi_diamidlen,
next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
if (cmp > 0)
continue;
if (cmp == 0)
int cont;
int cmp = fd_os_almostcasesrch( p->p_hdr.info.pi_diamid, p->p_hdr.info.pi_diamidlen,
next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen,
&cont );
if (cmp < 0)
li_inf = li;
if (cmp == 0) {
ret = EEXIST;
break;
break;
}
if (!cont)
break;
}
/* We can insert the new peer object */
......@@ -161,7 +167,7 @@ int fd_peer_add ( struct peer_info * info, char * orig_dbg, void (*cb)(struct pe
CHECK_FCT_DO( ret = fd_p_expi_update( p ), break );
/* Insert the new element in the list */
fd_list_insert_before( li, &p->p_hdr.chain );
fd_list_insert_after( li_inf, &p->p_hdr.chain );
} while (0);
CHECK_POSIX( pthread_rwlock_unlock(&fd_g_peers_rw) );
......@@ -184,19 +190,28 @@ int fd_peer_getbyid( DiamId_t diamid, size_t diamidlen, int igncase, struct peer
/* Search in the list */
CHECK_POSIX( pthread_rwlock_rdlock(&fd_g_peers_rw) );
for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
struct fd_peer * next = (struct fd_peer *)li;
int cmp;
if (igncase)
cmp = fd_os_almostcasecmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
else
cmp = fd_os_cmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
if (cmp > 0)
continue;
if (cmp == 0)
*peer = &next->p_hdr;
break;
}
if (igncase)
for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
struct fd_peer * next = (struct fd_peer *)li;
int cmp, cont;
cmp = fd_os_almostcasesrch( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen, &cont );
if (cmp == 0) {
*peer = &next->p_hdr;
break;
}
if (!cont)
break;
}
else
for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
struct fd_peer * next = (struct fd_peer *)li;
int cmp = fd_os_cmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
if (cmp > 0)
continue;
if (cmp == 0)
*peer = &next->p_hdr;
break;
}
CHECK_POSIX( pthread_rwlock_unlock(&fd_g_peers_rw) );
return 0;
......@@ -428,7 +443,7 @@ int fd_peer_handle_newCER( struct msg ** cer, struct cnxctx ** cnx )
struct msg * msg;
struct avp *avp_oh;
struct avp_hdr * avp_hdr;
struct fd_list * li;
struct fd_list * li, *li_inf;
int found = 0;
int ret = 0;
struct fd_peer * peer;
......@@ -469,15 +484,21 @@ int fd_peer_handle_newCER( struct msg ** cer, struct cnxctx ** cnx )
*/
CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_peers_rw) );
li_inf = &fd_g_peers;
for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
int cmp;
int cmp, cont;
peer = (struct fd_peer *)li->o;
cmp = fd_os_almostcasecmp( avp_hdr->avp_value->os.data, avp_hdr->avp_value->os.len, peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen );
if (cmp > 0)
cmp = fd_os_almostcasesrch( avp_hdr->avp_value->os.data, avp_hdr->avp_value->os.len, peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen, &cont );
if (cmp < 0) {
li_inf = li;
continue;
if (cmp == 0)
}
if (cmp == 0) {
found = 1;
break;
break;
}
if (!cont)
break;
}
if (!found) {
......@@ -499,7 +520,7 @@ int fd_peer_handle_newCER( struct msg ** cer, struct cnxctx ** cnx )
-- RFC3539 states that this must not be inferior to BRINGDOWN_INTERVAL = 5 minutes */
/* Insert the new peer in the list (the PSM will take care of setting the expiry after validation) */
fd_list_insert_before( li, &peer->p_hdr.chain );
fd_list_insert_after( li_inf, &peer->p_hdr.chain );
/* Start the PSM, which will receive the event bellow */
CHECK_FCT_DO( ret = fd_psm_begin(peer), goto out );
......
......@@ -273,11 +273,11 @@ static int score_destination_avp(void * cbdata, struct msg * msg, struct fd_list
#endif /* 0 */
/* In the AVPs, the value comes from the network, so let's be case permissive */
if (dh && !fd_os_almostcasecmp(dh->os.data, dh->os.len, c->diamid, c->diamidlen) ) {
if (dh && !fd_os_almostcasesrch(dh->os.data, dh->os.len, c->diamid, c->diamidlen, NULL) ) {
/* The candidate is the Destination-Host */
c->score += FD_SCORE_FINALDEST;
} else {
if (dr && !fd_os_almostcasecmp(dr->os.data, dr->os.len, c->realm, c->realmlen) ) {
if (dr && !fd_os_almostcasesrch(dr->os.data, dr->os.len, c->realm, c->realmlen, NULL) ) {
/* The candidate's realm matchs the Destination-Realm */
c->score += FD_SCORE_REALM;
}
......@@ -591,7 +591,7 @@ static int msg_rt_in(struct msg ** pmsg)
} );
ASSERT( ahdr->avp_value );
/* Compare the Destination-Host AVP of the message with our identity */
if (!fd_os_almostcasecmp(ahdr->avp_value->os.data, ahdr->avp_value->os.len, fd_g_config->cnf_diamid, fd_g_config->cnf_diamid_len)) {
if (!fd_os_almostcasesrch(ahdr->avp_value->os.data, ahdr->avp_value->os.len, fd_g_config->cnf_diamid, fd_g_config->cnf_diamid_len, NULL)) {
is_dest_host = YES;
} else {
is_dest_host = NO;
......@@ -612,7 +612,7 @@ static int msg_rt_in(struct msg ** pmsg)
ASSERT( ahdr->avp_value );
dr_val = ahdr->avp_value;
/* Compare the Destination-Realm AVP of the message with our identity */
if (!fd_os_almostcasecmp(dr_val->os.data, dr_val->os.len, fd_g_config->cnf_diamrlm, fd_g_config->cnf_diamrlm_len)) {
if (!fd_os_almostcasesrch(dr_val->os.data, dr_val->os.len, fd_g_config->cnf_diamrlm, fd_g_config->cnf_diamrlm_len, NULL)) {
is_dest_realm = YES;
} else {
is_dest_realm = NO;
......
......@@ -68,13 +68,22 @@ static inline uint8_t asciitolower(uint8_t a)
return a;
}
/* a little less sensitive to case, slower. */
int fd_os_almostcasecmp_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz)
/* less sensitive to case, slower. */
int fd_os_almostcasesrch_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz, int *maybefurther)
{
int i;
int res = 0;
ASSERT( os1 && os2);
if (maybefurther)
*maybefurther = 0;
if (os1sz < os2sz)
return -1;
if (maybefurther)
*maybefurther = 1;
if (os1sz > os2sz)
return 1;
......@@ -82,10 +91,13 @@ int fd_os_almostcasecmp_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t o
if (os1[i] == os2[i])
continue;
if (!res)
res = os1[i] < os2[i] ? -1 : 1;
if (asciitolower(os1[i]) == asciitolower(os2[i]))
continue;
return os1[i] < os2[i] ? -1 : 1;
return res;
}
return 0;
......
......@@ -159,8 +159,8 @@ void fd_rtd_candidate_del(struct rt_data * rtd, uint8_t * id, size_t idsz)
for (li = rtd->candidates.next; li != &rtd->candidates; li = li->next) {
struct rtd_candidate * c = (struct rtd_candidate *) li;
int cmp = fd_os_almostcasecmp(id, idsz, c->diamid, c->diamidlen);
int cont;
int cmp = fd_os_almostcasesrch(id, idsz, c->diamid, c->diamidlen, &cont);
if (!cmp) {
/* Found it! Remove it */
......@@ -171,7 +171,7 @@ void fd_rtd_candidate_del(struct rt_data * rtd, uint8_t * id, size_t idsz)
break;
}
if (cmp > 0)
if (cont)
continue;
/* The list is guaranteed to be ordered only if not extracted */
......
......@@ -98,7 +98,7 @@ int main(int argc, char *argv[])
CHECK( 0, fd_os_validate_DiameterIdentity(&res, &len, 1) );
CHECK( 0, strcasecmp(res, TEST_IDN_UTF8) );
CHECK( 0, fd_os_cmp(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8)) );
CHECK( 0, fd_os_almostcasecmp(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8)) );
CHECK( 0, fd_os_almostcasesrch(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8), NULL) );
CHECK( 0, fd_os_validate_DiameterIdentity(&res, &len, 0) );
CHECK( 0, strcasecmp(res, TEST_IDN_UTF8) );
CHECK( CONSTSTRLEN(TEST_IDN_UTF8), len );
......@@ -127,6 +127,75 @@ int main(int argc, char *argv[])
}
{
/* test fd_os_cmp and fd_os_almostcasesrch and that they are compatible */
char *t1 = "a";
char *t2 = "b";
char *t3 = "C";
char *t4 = "d";
char *t5 = "aa";
char *t6 = "aB";
char *t7 = "Ac";
char *t8 = "aD";
char *t9 = "AAA";
char *t5b = "Aa";
char *t6b = "ab";
/* First, create a list with all the elements in order given by fd_os_cmp */
char *t[] = { t1, t2, t3, t4, t5, t6,t7, t8, t9 };
int i;
struct fd_list *li, l = FD_LIST_INITIALIZER(l);
for (i = 0; i < sizeof(t) / sizeof(t[0]); i++) {
/* insert t[i] */
struct fd_list *n = malloc(sizeof(struct fd_list));
CHECK( 1, n ? 1 : 0 );
fd_list_init(n, t[i]);
for (li = l.next; li != &l; li = li->next) {
if ( fd_os_cmp(t[i], strlen(t[i]), li->o, strlen(li->o)) < 0 )
break;
}
fd_list_insert_before(li, n);
}
/* in principle the result is: [ "C", "a", "b", "d", "Ac", "aB", "aD", "aa", "AAA" ] */
/* Since there is no equal value in the list (even case-insensitive), check that the order is valid also for the caseinsensitive variant */
for (li = l.next; li != l.prev; li = li->next) {
CHECK( 1, fd_os_almostcasesrch(li->o, strlen(li->o), li->next->o, strlen(li->next->o), NULL) < 0 ? 1 : 0 );
}
/* Now check that we can case-insentively find t5b and t6b to be equal to t5 and t6 resp. (this is how we use it in the daemon) */
for (li = l.next; li != &l; li = li->next) {
int cont, cmp;
cmp = fd_os_almostcasesrch(t5b, strlen(t5b), li->o, strlen(li->o), &cont);
TRACE_DEBUG(FULL, "Comp '%s' : %d, %d", (char *)li->o, cmp, cont);
if (cmp == 0)
break;
if (!cont)
break;
}
CHECK( li->o, t5 );
for (li = l.next; li != &l; li = li->next) {
int cont, cmp;
cmp = fd_os_almostcasesrch(t6b, strlen(t6b), li->o, strlen(li->o), &cont);
TRACE_DEBUG(FULL, "Comp '%s' : %d, %d", (char *)li->o, cmp, cont);
if (cmp == 0)
break;
if (!cont)
break;
}
CHECK( li->o, t6 );
/* done */
while (!FD_IS_LIST_EMPTY(&l)) {
li = l.next;
fd_list_unlink(li);
free(li);
}
}
/* That's all for the tests yet */
PASSTEST();
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment