Skip to contentSkip to conversation

nfsidmap: a keyringer - 2nd try

PATCH In review Benjamin Coddington 2 participants 4 messages Mar 25, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover nfsidmap: a keyringer - 2nd try

I took a second stab at this since it looks like fixing the keyrings is probably the way to go. This time through, the nfsidmap creates the keyrings ad-hoc as they fill up instead of taking new command-line parameters. It also explicitly sets the permissions on the new keyrings, so they should be able to be cleaned up in later kernels - though I haven't tested that myself.

Ben

Benjamin Coddington (2):
nfsidmap: Match names with kernel default keyring
nfsidmap: Create id_resolver child keyrings

 utils/nfsidmap/nfsidmap.c |   76 ++++++++++++++++++++++++++++++++++++---------
 1 files changed, 61 insertions(+), 15 deletions(-)
#1 lore raw

PATCH 1/2 nfsidmap: Match names with kernel default keyring

---
 utils/nfsidmap/nfsidmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
utils/nfsidmap/nfsidmap.c +1 -1
index 2518ed6..ae84633 100644
@@ -24,7 +24,7 @@ char *usage="Usage: %s [-v] [-c || [-u|-g|-r key] || [-t timeout] key desc]";
 
 #define PROCKEYS "/proc/keys"
 #ifndef DEFAULT_KEYRING
-#define DEFAULT_KEYRING "id_resolver"
+#define DEFAULT_KEYRING ".id_resolver"
 #endif
 
 #ifndef PATH_IDMAPDCONF
Signature · 1 line
1.7.1
#2 lore raw

PATCH 2/2 nfsidmap: Create id_resolver child keyrings

In reply to Benjamin Coddington

Create and fill child keyrings of MAX_KEYS number of keys with id_resolver keys to expand the idmapper's key capacity.

---
 utils/nfsidmap/nfsidmap.c |   74 ++++++++++++++++++++++++++++++++++++--------
 1 files changed, 60 insertions(+), 14 deletions(-)
utils/nfsidmap/nfsidmap.c +60 -14
index ae84633..c66c19f 100644
@@ -27,6 +27,10 @@ char *usage="Usage: %s [-v] [-c || [-u|-g|-r key] || [-t timeout] key desc]";
 #define DEFAULT_KEYRING ".id_resolver"
 #endif
 
+#ifndef MAX_KEYS
+#define MAX_KEYS 500
+#endif
+
 #ifndef PATH_IDMAPDCONF
 #define PATH_IDMAPDCONF "/etc/idmapd.conf"
 #endif
@@ -39,7 +43,7 @@ static int keyring_clear(char *keyring);
 /*
  * Find either a user or group id based on the name@domain string
  */
-int id_lookup(char *name_at_domain, key_serial_t key, int type)
+int id_lookup(char *name_at_domain, key_serial_t key, int type, key_serial_t dest_keyring)
 {
 	char id[MAX_ID_LEN];
 	uid_t uid = 0;
@@ -58,7 +62,7 @@ int id_lookup(char *name_at_domain, key_serial_t key, int type)
 			(type == USER ? "nfs4_owner_to_uid" : "nfs4_group_owner_to_gid"));
 
 	if (rc == 0) {
-		rc = keyctl_instantiate(key, id, strlen(id) + 1, 0);
+		rc = keyctl_instantiate(key, id, strlen(id) + 1, dest_keyring);
 		if (rc < 0) {
 			switch(rc) {
 			case -EDQUOT:
@@ -67,9 +71,9 @@ int id_lookup(char *name_at_domain, key_serial_t key, int type)
 				/*
 			 	 * The keyring is full. Clear the keyring and try again
 			 	 */
-				rc = keyring_clear(DEFAULT_KEYRING);
+				rc = keyctl_clear(dest_keyring);
 				if (rc == 0)
-					rc = keyctl_instantiate(key, id, strlen(id) + 1, 0);
+					rc = keyctl_instantiate(key, id, strlen(id) + 1, dest_keyring);
 				break;
 			default:
 				break;
@@ -85,7 +89,7 @@ int id_lookup(char *name_at_domain, key_serial_t key, int type)
 /*
  * Find the name@domain string from either a user or group id
  */
-int name_lookup(char *id, key_serial_t key, int type)
+int name_lookup(char *id, key_serial_t key, int type, key_serial_t dest_keyring)
 {
 	char name[IDMAP_NAMESZ];
 	char domain[NFS4_MAX_DOMAIN_LEN];
@@ -113,7 +117,7 @@ int name_lookup(char *id, key_serial_t key, int type)
 			(type == USER ? "nfs4_uid_to_name" : "nfs4_gid_to_name"));
 
 	if (rc == 0) {
-		rc = keyctl_instantiate(key, &name, strlen(name), 0);
+		rc = keyctl_instantiate(key, &name, strlen(name), dest_keyring);
 		if (rc < 0)
 			xlog_err("name_lookup: keyctl_instantiate failed: %m");
 	}
@@ -142,6 +146,8 @@ static int keyring_clear(char *keyring)
 			continue;
 		if (strstr(buf, keyring) == NULL)
 			continue;
+		if (strstr(buf, "perm") == NULL)
+			continue;
 		if (verbose) {
 			*(strchr(buf, '\n')) = '\0';
 			xlog_warn("clearing '%s'", buf);
@@ -157,9 +163,13 @@ static int keyring_clear(char *keyring)
 			return 1;
 		}
 		fclose(fp);
+		// if this is a child, revoke it so it gets cleaned up
+		if (!strstr(keyring, DEFAULT_KEYRING":"))
+			keyctl_revoke(key);
 		return 0;
 	}
-	xlog_err("'%s' keyring was not found.", keyring);
+	if (strstr(keyring, DEFAULT_KEYRING":"))
+		xlog_err("'%s' keyring was not found.", keyring);
 	fclose(fp);
 	return 1;
 }
@@ -232,9 +242,12 @@ int main(int argc, char **argv)
 	char *type;
 	int rc = 1, opt;
 	int timeout = 600;
-	key_serial_t key;
+	int childrings = 0;
+	key_serial_t key, parent_keyring, child_keyring;
 	char *progname, *keystr = NULL;
-	int clearing = 0, keymask = 0;
+	char child_name[BUFSIZ];
+	int clearing = 0, keymask = 0, i;
+	long child_size;
 
 	/* Set the basename */
 	if ((progname = strrchr(argv[0], '/')) != NULL)
@@ -284,9 +297,16 @@ int main(int argc, char **argv)
 		rc = key_revoke(keystr, keymask);
 		return rc;		
 	}
+
 	if (clearing) {
 		xlog_syslog(0);
-		rc = keyring_clear(DEFAULT_KEYRING);
+		i = 1;
+		for(i = 1; i < MAX_KEYS; i++) {
+			snprintf(child_name, sizeof(child_name), DEFAULT_KEYRING "_child_%d", i);
+			keyring_clear(child_name);
+		}
+
+		rc = keyring_clear(DEFAULT_KEYRING ":");
 		return rc;		
 	}
 
@@ -315,14 +335,40 @@ int main(int argc, char **argv)
 			key, type, value, timeout);
 	}
 
+	parent_keyring = request_key("keyring", DEFAULT_KEYRING, NULL, KEY_SPEC_THREAD_KEYRING);
+
+	for (i = 1; i < MAX_KEYS; i++) {
+		snprintf(child_name, sizeof(child_name), DEFAULT_KEYRING "_child_%d", i);
+
+		child_keyring = keyctl_search(parent_keyring, "keyring", child_name, 0);
+		if (child_keyring < 0) {
+			child_keyring = add_key("keyring", child_name, NULL, 0, parent_keyring);
+			xlog_warn("adding new child %s: %m", child_name);
+
+			if (child_keyring < 0)
+				xlog_err("Failed to add child keyring: %m");
+
+			keyctl_setperm(child_keyring, KEY_POS_ALL|KEY_USR_ALL);
+			break;
+		}
+
+		child_size = keyctl_read(child_keyring, NULL, 0);
+		if (child_size <= MAX_KEYS * 4)
+			break;
+	}
+
 	if (strcmp(type, "uid") == 0)
-		rc = id_lookup(value, key, USER);
+		rc = id_lookup(value, key, USER, child_keyring);
 	else if (strcmp(type, "gid") == 0)
-		rc = id_lookup(value, key, GROUP);
+		rc = id_lookup(value, key, GROUP, child_keyring);
 	else if (strcmp(type, "user") == 0)
-		rc = name_lookup(value, key, USER);
+		rc = name_lookup(value, key, USER, child_keyring);
 	else if (strcmp(type, "group") == 0)
-		rc = name_lookup(value, key, GROUP);
+		rc = name_lookup(value, key, GROUP, child_keyring);
+
+	/* if we hung this off a child, unlink from the parent */
+	if (child_keyring)
+		keyctl_unlink(key, parent_keyring);
 
 	/* Set timeout to 10 (600 seconds) minutes */
 	if (rc == 0)
Signature · 1 line
1.7.1
#3 lore raw
In reply to Benjamin Coddington
Quoted 6 lines · Benjamin Coddington

I took a second stab at this since it looks like fixing the keyrings is probably the way to go. This time through, the nfsidmap creates the keyrings ad-hoc as they fill up instead of taking new command-line parameters. It also explicitly sets the permissions on the new keyrings, so they should be able to be cleaned up in later kernels - though I haven't tested that myself.

I'm not going to take these for upstream since I don't think they are needed... If that turns out not to be the case, we can add it later...

steved.

Quoted 15 lines

Ben

Benjamin Coddington (2):
nfsidmap: Match names with kernel default keyring
nfsidmap: Create id_resolver child keyrings

 utils/nfsidmap/nfsidmap.c |   76 ++++++++++++++++++++++++++++++++++++---------
 1 files changed, 61 insertions(+), 15 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html