PermaLinkAn Unlikely But Interesting Problem In A DSAPI Filter
03:35:01 PM

About a year ago, Stephen Walch of Proposion and I teamed up to write a DSAPI filter for one of my clients. They are an Application Service Provider, operating a Domino-based service for their customers, and one of their customers wanted to use their internally-developed network login system to bypass the service's login and password. Their internal system generated digitally signed XML tokens, which contained user data including their own internally assigned unique id for each user. Our filter verified the signature, checked the timestamp on the token, did a lookup on the id, generated a session cookie, and then let the user in.


Recently we got a report from my client that a small number of users were being given the wrong identity by our filter. In other words, they got into the system, but they saw someone else's data. @Username was reporting that they were, in fact, someone else. After having my client's admin make some Notes.INI changes to turn on some tracing features that I had put into the code, I participated in a conference call with their customer, and watched on remote console while they ran through some test scenarios with the users who were having problems. I could tell from the tracing information that this was no illusion. The filter really was doing exactly what they said it was. The tokens had the correct user information and were correctly signed, and the code was extracting the correct user id, but the final piece of tracing was recording the wrong user name!


The tracing told me that everything was going correctly, right up to the point where the lookup of the user id was being done, and I got ready to look at the code... but there was something else suspicious in the traces, so I decided to look in the $Users view in the Domino Directory first. The traces showed that the user ids that my client's customer was using in production had the format LetterDigitDigitDigitDigit. Does that sound familiar to you? It did to me.


LetterDigitDigitDigitDigit is exactly the format generated by the @Soundex function in Notes. The possibility that we were running into collisions between assigned user ids and calculated soundex codes popped into my mind, and I recalled that the first sorted column in the $Users view does contain entries with the soundex codes computed from each user's name. I immediately suspected that the problem was that one user's id was matching another user's soundex code in that view. A quick look at the Domino Directory verified that for each of the problem user ids, there were in fact two entries in $Users view. One entry corresponded to the shortname for the given user id, and the other entry corresponded with the soundex code for some other user id. A quick look at the code verified that it was in fact using $Users to lookup names.


Here's the code for the relevant part of the DSAPI filter:


        // call NAMELookup to locate the user_id in $Users

    if (nError = NAMELookup( NULL,        /* name server */    
        NAME_LOOKUP_EXHAUSTIVE|NAME_LOOKUP_UPDATE,    /* NAME_LOOKUP_ALL, NAME_LOOKUP_NOSEARCHING, NAME_LOOKUP_EXHAUSTIVE, NAME_LOOKUP_UPDATE */
        1,                                 /* Number of name spaces */
        "$Users",                        /* Name of name space (view name) */
        1,                                 /* # of names */
        (char*)id,                        /* Ptr to the names themselves */
        2,                                 /* # of items desired */
        "FullName",                        /* Ptr to the item names */
        &hLookup                         /*  Handle of returned buffer */
        ))
        return false;

    char* pLookup = (char*)OSLockObject(hLookup);
    try {
        char* pName = NULL;
        WORD   NumMatches;

        // loop through the set of matches returned by NAMELookup

        pName = (char*)NAMELocateNextName(pLookup, pName, &NumMatches);

        char* pMatch = NULL; 
        for (int j = 0; j < NumMatches; j++)
        {
            pMatch = (char*)NAMELocateNextMatch(pLookup, pName, pMatch);
    
            // if it's a true match, return this one
            nError = NAMEGetTextItem(pMatch, 0, 0, fullName, MAXUSERNAME);
            if (NOERROR == nError && strlen(fullName) > 0)
                return true;
        }
    } __finally {
        OSUnlockObject(hLookup);
        OSMemFree(hLookup);
    }

The code is doing only a cursory check on the validity of the entries returned by NAMELookup. Although it expects only a single name to be returned (because the names are guaranteed to be uniquely assigned), it still does deal with cases where multiple matches are returned. It retrieves the FullName for each match, and and since we "knew" that the user ids are unique we just check to make sure that the FullName didn't come back blank. If not, we go on to the next entry. In the general case, this wouldn't have been a good way of doing this, but if my client's customer had used any other convention for usernames besides LetterDigitDigitDigitDigit, this would have been perfectly sufficient. With the possibility of collisions with soundex codes, we definitely needed to do a better check. We recoded as follows:


    // call NAMELookup to locate the user_id in $Users

    if (nError = NAMELookup( NULL,        /* name server */    
        NAME_LOOKUP_EXHAUSTIVE|NAME_LOOKUP_UPDATE,    /* NAME_LOOKUP_ALL, NAME_LOOKUP_NOSEARCHING, NAME_LOOKUP_EXHAUSTIVE, NAME_LOOKUP_UPDATE */
        1,                                 /* Number of name spaces */
        "$Users",                        /* Name of name space (view name) */
        1,                                 /* # of names */
        (char*)id,                        /* Ptr to the names themselves */
        2,                                 /* # of items desired */
        "FullName\0ShortName",            /* Ptr to the item names */
        &hLookup                         /*  Handle of returned buffer */
        ))
        return false;

    char* pLookup = (char*)OSLockObject(hLookup);
    try {
        char* pName = NULL;
        WORD   NumMatches;

        // loop through the set of matches returned by NAMELookup

        pName = (char*)NAMELocateNextName(pLookup, pName, &NumMatches);

        char* pMatch = NULL; 
        for (int j = 0; j < NumMatches; j++)
        {
            pMatch = (char*)NAMELocateNextMatch(pLookup, pName, pMatch);
    
            // if it's a true match on the ShortName field, return this one
        
            nError = NAMEGetTextItem(pMatch, 1, 0, shortName, MAXUSERNAME);
            if (NOERROR == nError && strlen(shortName) > 0)
            {
                if (strcmp(shortName,id) == 0)
                {
                    // we're good, so get the fullName and return it
                    nError = NAMEGetTextItem(pMatch, 0, 0, fullName, MAXUSERNAME);
                    if (NOERROR == nError && strlen(fullName) > 0)
                        return true;
                }
            }
        }
    } __finally {
        OSUnlockObject(hLookup);
        OSMemFree(hLookup);
    }

The new code retrieves the ShortName as well as the FullName and verifies that the ShortName actually does match the input user id. It still does assume that the ShortName is unique, but this does guard against the collisions with the soundex code.


BTW: In the process of putting in the fix for this, I noticed another minor (apparently inconsequential, though given the fragile of the Notes API, somewhat surprisingly so) problem in the original code. It's fixed in the new version, but it will take a sharp eye to see it. Let me know if you figure it out ;-)

This page has been accessed 414 times. .
Comments :v

1. Julian Robichaux03/22/2005 07:58:11 AM
Homepage: http://www.nsftools.com


In the "we're good, so get the fullName and return it" section, you're not releasing the memory before you return.

Is that it? Did I get it?


- Julian




2. Richard Schwartz03/22/2005 08:22:57 AM
Homepage: http://smokey.rhs.com/web/blog/PowerOfTheSchwartz.nsf


Nope. The finally clause on the try construct should take care of that.

-rich




3. vesoftware11/05/2013 10:23:08 PM
Homepage: http://vesoftware.blogspot.com/2013/10/itupokercom-agen-poker-online-indonesia.html


Agen Bola Promo 100% SBOBET IBCBET Casino Poker Tangkas Online
http://vesoftware.blogspot.com/2013/10/agen-bola-promo-100-sbobet-ibcbet.html
ITUPOKER.COM AGEN POKER ONLINE INDONESIA TERPERCAYA
http://vesoftware.blogspot.com/2013/10/itupokercom-agen-poker-online-indonesia.html
alfaonline.com : Toko belanja online murah, Promo heboh jual barang hanya Rp 1,-
http://vesoftware.blogspot.com/2013/10/alfaonlinecom-toko-belanja-online-murah.html
BELAJAR SEO
http://balapseo.blogspot.com/




4. cialis_online09/08/2016 11:02:44 AM
Homepage: http://c4tadalafil.com/


Hello!
http://c4tadalafil.com/ , ,




Enter Comments^


Email addresses provided are not made available on this site.





You can use UUB Code in your posts.

[b]bold[/b]  [i]italic[/i]  [u]underline[/u]  [s]strikethrough[/s]

URL's will be automatically converted to Links


:-x :cry: :laugh: :-( :cool: :huh: :-) :angry: :-D ;-) :-p :grin: :rolleyes: :-\ :emb: :lips: :-o
bold italic underline Strikethrough





Remember me    

Monthly Archive
Responses Elsewhere



About The Schwartz

rss.jpg


All opinions expressed here are my own, and do not represent positions of my employer.