Changeset 445 for trunk/src/mappers/mapper.h

Timestamp:

08/14/10 16:19:36 (22 months ago)

Author:

ludovic.rousseau

Message:

Patch for #239 and #240 (handle more than one cert/pattern matching)

Thanks to Wolf Geldmacher for the patch.
http://www.opensc-project.org/pipermail/opensc-devel/2010-June/014405.html

" Here's a patch to solve the issues I've encountered using pam_pkcs11.

In regards to #239 (pam_pkcs11 only looks at first certificate on
token):

The fix for this turns out to be somewhat problematic, and I'm not at
all sure, whether my implementation of the fix is a valid one.

The basic problem (as I understood it from analyzing the code) is that
finder functions of the mappers return a char*, allowing for a single
value (NULL) to signalize failure and return the key if no mapping (i.e.
no value associated with the key) was found (cf. comment for
mapfile_find in src/mappers/mapper.c). Thus a caller (i.e. find_user in
src/pam_pkcs11/mapper_mgr.c) cannot distinguish between a mapping or a
key being returned and thus will prematurely terminate on the first
certificate that passes the other validity tests.

The fix provided changes the finder function interface by requiring an
additional out parameter that is set to 1, if a real mapping value was
returned and remains unchanged otherwise. This fix breaks existing
loadable mappers.

I considered overloading of the value returned (e.g. having a
byte/substring as first character of the value returned to be able to
distinguish between a value and a key being returned) which would
preserve the interface to the mappers, but refrained from implementing
it that way as I believe this to be unclean and prone to difficult to
track errors.

Another solution I considered was the addition of another entry to the
structure encapsulating the mappers (e.g. a finder2 method), but as this
is no better in breaking the interface for loadable mappers and
duplicates code I forfeited this solution, too.

If somebody could look into the problem and come up with a solution that
preserves the interface to external mappers while allowing the
distinction between keys and values, I'd be more than happy to implement
it.

It might also may make sense to add a new configuration parameter for
the new behaviour of find_user, allowing existing applications to
continue to work with keys being returned instead of values (Feedback
anyone? The comment for find_user actually states that a mapping value
is returned).

In regards to #240 (Allow pattern matching in pam_pkcs11):

I restricted this to only work for mapfiles and the implementation
turned out to be quite simple - it's essentially an 11 line change in
src/mappers/mapper.c - and is triggered by the specification of a fully
anchored (i.e. *must* have initial "^{" and *must* end in "$") pattern as
key in a mapfile.}

This now allows syntax like
^{.*/serialNumber=xxx-xxx-xxx-xxx$ -> username
in all mapfiles.}

The patch attached contains the changes for both issues.

Cheers,
Wolf "

File:

• trunk/src/mappers/mapper.h (modified) (4 diffs)

trunk/src/mappers/mapper.h

@@ -50 +50 @@
     char **(*entries)(X509 *x509, void *context);
     /** cert. login finder */
-    char *(*finder)(X509 *x509, void *context);
+    char *(*finder)(X509 *x509, void *context, int *match);
     /** cert-to-login matcher*/
     int (*matcher)(X509 *x509, const char *login, void *context);
@@ -126 +126 @@
 *@param key String to be mapped
 *@param ignorecase Flag to indicate upper/lowercase ignore in string compare
+*@param match Set to 1 for mapped string return, unmodified for key return
 *@return key on no match, else a clone_str()'d of found mapping
 */
-MAPPER_EXTERN char *mapfile_find(const char *file,char *key,int ignorecase);
+MAPPER_EXTERN char *mapfile_find(const char *file,char *key,int ignorecase,int *match);
 
 /**
@@ -185 +186 @@
 */
 #define _DEFAULT_MAPPER_FIND_USER                                       \
-static char * mapper_find_user(X509 *x509,void *context) {              \
+static char * mapper_find_user(X509 *x509,void *context,int *match) {           \
         if ( !x509 ) return NULL;                                       \
+        *match = 1;                                                     \
         return "nobody";                                                \
 }
@@ -202 +204 @@
 #define _DEFAULT_MAPPER_MATCH_USER                                      \
 static int mapper_match_user(X509 *x509, const char *login, void *context) { \
-        char *username= mapper_find_user(x509,context);                         \
+        int match = 0;                                                  \
+        char *username= mapper_find_user(x509,context,&match);          \
         if (!x509) return -1;                                           \
         if (!login) return -1;                                          \