Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 18 Sep 2015 04:01:56 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: "struct db_password" allocation

On Fri, Sep 18, 2015 at 12:13:42AM +0200, magnum wrote:
> On 2015-09-17 23:59, Solar Designer wrote:
> >On Thu, Sep 17, 2015 at 11:04:54PM +0200, magnum wrote:
> >>It's main use (or at least the reason Jim implemented it) is to get
> >>BSSID printed/logged when cracking WPA-PSK. In that case, the ESSID is
> >>the "login" and sometimes there are many same ESSID so we need to see
> >>the BSSID as well (but we do not want it used by Single mode so it can't
> >>be included in login). There might be other uses too. I'll look into
> >>that double string idea!
> >
> >Oh, I didn't realize this is controlled with ShowUIDinCracks, and is
> >disabled by default.  If so, we should simply make the allocation for
> >the uid pointer conditional upon that setting.  And no need for the
> >double string then.
> 
> I don't quite get those allocs, could you fix that please?

Patch attached.

This also avoids allocating the "source" pointer when possible, which it
is when running with --save-memory=1 (or higher) and the format provides
source().  In that case, the binary ends up being allocated with the
very next call to mem_alloc_tiny(), which most likely places it right
into where the "source" pointer would have been.  Previously, we already
tried to put the binary into the unused "source" pointer, but only if
it'd fit in the pointer size - now we do an equivalent of this for
larger binaries as well (but only at --save-memory=1 or higher).  When
this "source" reuse happens, the binary pointer ends up pointing to
right after itself, so chances are very good that the actual binary is
in the same cache line with the binary pointer.

I've also adjusted the documentation to advertise --save-memory=1 as
potentially speeding things up.

I'll get these changes into core (except for the "uid" stuff) after some
more testing in jumbo.

--save-memory=1 on the 29M testcase:

real    0m41.366s
user    2m54.951s
sys     0m13.279s

Alexander

diff -urp bleeding-jumbo-opt6/doc/OPTIONS bleeding-jumbo-opt/doc/OPTIONS
--- bleeding-jumbo-opt6/doc/OPTIONS	2015-08-13 17:53:46 +0000
+++ bleeding-jumbo-opt/doc/OPTIONS	2015-09-18 00:39:36 +0000
@@ -190,12 +190,20 @@ john.pot-like file (to start from, and t
 --save-memory=LEVEL		enable memory saving, at LEVEL 1..3
 
 You might need this option if you don't have enough memory or don't
-want John to affect other processes too much.  Level 1 tells John to
-not waste memory on login names; it is only supported when a cracking
-mode other than "single crack" is explicitly requested.  The only
-impact is that you won't see the login names while cracking.  Higher
-memory saving levels have a performance impact; you should probably
-avoid using them unless John doesn't work or gets into swap otherwise.
+want John to affect other processes too much or don't need it to load
+and print login names along with cracked passwords.  Level 1 tells John
+not to waste memory on login names; it is only supported when a cracking
+mode other than "single crack" is explicitly requested.  It has no
+negative performance impact - in fact, it sometimes speeds things up.
+Please note that without the --save-memory=1 option (or higher), John
+will waste some memory on potential login names even if the password
+hash files don't happen to contain any login names.  (The complete lack
+of login names isn't known to John when it starts parsing the files, so
+it has to record the fact that each individual entry doesn't have a
+login name unless you specify this option.)  Levels 2 and 3 reduce use
+of performance optimizations involving large lookup tables, and thus
+have a negative performance impact.  You should probably avoid using
+them unless John doesn't work or gets into swap otherwise.
 
 --node=MIN[-MAX]/TOTAL		this node's number range out of TOTAL count
 
diff -urp bleeding-jumbo-opt6/src/loader.c bleeding-jumbo-opt/src/loader.c
--- bleeding-jumbo-opt6/src/loader.c	2015-09-17 04:39:44 +0000
+++ bleeding-jumbo-opt/src/loader.c	2015-09-17 23:59:13 +0000
@@ -147,19 +147,34 @@ static void read_file(struct db_main *db
 	if (fclose(file)) pexit("fclose");
 }
 
-void ldr_init_database(struct db_main *db, struct db_options *options)
+void ldr_init_database(struct db_main *db, struct db_options *db_options)
 {
 	db->loaded = 0;
 
-	db->options = mem_alloc_copy(options,
+	db->pw_size = sizeof(struct db_password);
+	db->salt_size = sizeof(struct db_salt);
+	if (!(db_options->flags & DB_WORDS)) {
+		db->pw_size -= sizeof(struct list_main *);
+		if (db_options->flags & DB_LOGIN) {
+			if (!options.show_uid_on_crack)
+				db->pw_size -= sizeof(char *);
+		} else
+			db->pw_size -= sizeof(char *) * 2;
+		db->salt_size -= sizeof(struct db_keys *);
+	}
+
+	db->options = mem_alloc_copy(db_options,
 	    sizeof(struct db_options), MEM_ALIGN_WORD);
 
+	if (db->options->flags & DB_WORDS)
+		db->options->flags |= DB_LOGIN;
+
 	db->salts = NULL;
 
 	db->password_hash = NULL;
 	db->password_hash_func = NULL;
 
-	if (options->flags & DB_CRACKED) {
+	if (db_options->flags & DB_CRACKED) {
 		db->salt_hash = NULL;
 
 		db->cracked_hash = mem_alloc(
@@ -173,10 +188,6 @@ void ldr_init_database(struct db_main *d
 			SALT_HASH_SIZE * sizeof(struct db_salt *));
 
 		db->cracked_hash = NULL;
-
-		if (options->flags & DB_WORDS)
-			options->flags |= DB_LOGIN;
-
 	}
 
 	list_init(&db->plaintexts);
@@ -793,7 +804,7 @@ static void ldr_load_pw_line(struct db_m
 	struct db_salt *current_salt, *last_salt;
 	struct db_password *current_pw, *last_pw;
 	struct list_main *words;
-	size_t pw_size, salt_size;
+	size_t pw_size;
 	int i;
 
 #ifdef HAVE_FUZZ
@@ -816,20 +827,6 @@ static void ldr_load_pw_line(struct db_m
 
 	words = NULL;
 
-	if (db->options->flags & DB_WORDS) {
-		pw_size = sizeof(struct db_password);
-		salt_size = sizeof(struct db_salt);
-	} else {
-		if (db->options->flags & DB_LOGIN)
-			pw_size = sizeof(struct db_password) -
-				sizeof(struct list_main *);
-		else
-			pw_size = sizeof(struct db_password) -
-				(sizeof(char *) + sizeof(struct list_main *));
-		salt_size = sizeof(struct db_salt) -
-			sizeof(struct db_keys *);
-	}
-
 	if (!db->password_hash) {
 		ldr_init_password_hash(db);
 		if (cfg_get_bool(SECTION_OPTIONS, NULL,
@@ -915,7 +912,7 @@ static void ldr_load_pw_line(struct db_m
 		if (!current_salt) {
 			last_salt = db->salt_hash[salt_hash];
 			current_salt = db->salt_hash[salt_hash] =
-				mem_alloc_tiny(salt_size, MEM_ALIGN_WORD);
+				mem_alloc_tiny(db->salt_size, MEM_ALIGN_WORD);
 			current_salt->next = last_salt;
 
 			current_salt->salt = mem_alloc_copy(salt,
@@ -943,6 +940,13 @@ static void ldr_load_pw_line(struct db_m
 		current_salt->count++;
 		db->password_count++;
 
+/* If we're not allocating memory for the "login" field, we may as well not
+ * allocate it for the "source" field if the format doesn't need it. */
+		pw_size = db->pw_size;
+		if (!(db->options->flags & DB_LOGIN) &&
+		    format->methods.source != fmt_default_source)
+			pw_size -= sizeof(char *);
+
 		last_pw = current_salt->list;
 		current_pw = current_salt->list = mem_alloc_tiny(
 			pw_size, MEM_ALIGN_WORD);
@@ -952,9 +956,11 @@ static void ldr_load_pw_line(struct db_m
 		db->password_hash[pw_hash] = current_pw;
 		current_pw->next_hash = last_pw;
 
-/* If we're not going to use the source field for its usual purpose, see if we
- * can pack the binary value in it. */
-		if (format->methods.source != fmt_default_source &&
+/* If we're not going to use the source field for its usual purpose yet we had
+ * to allocate memory for it (because we need at least one field after it), see
+ * if we can pack the binary value in it. */
+		if ((db->options->flags & DB_LOGIN) &&
+		    format->methods.source != fmt_default_source &&
 		    sizeof(current_pw->source) >= format->params.binary_size)
 			current_pw->binary = memcpy(&current_pw->source,
 				binary, format->params.binary_size);
@@ -976,23 +982,22 @@ static void ldr_load_pw_line(struct db_m
 			if (login != no_username && index == 0)
 				login = ldr_conv(login);
 
-			current_pw->uid = "";
+			if (options.show_uid_on_crack)
+				current_pw->uid = str_alloc_copy(uid);
+
 			if (count >= 2 && count <= 9) {
 				current_pw->login = mem_alloc_tiny(
 					strlen(login) + 3, MEM_ALIGN_NONE);
 				sprintf(current_pw->login, "%s:%d",
 					login, index + 1);
-				current_pw->uid = str_alloc_copy(uid);
 			} else
 			if (login == no_username)
 				current_pw->login = login;
 			else
 			if (words && *login)
 				current_pw->login = words->head->data;
-			else {
+			else
 				current_pw->login = str_alloc_copy(login);
-				current_pw->uid = str_alloc_copy(uid);
-			}
 		}
 	}
 }
diff -urp bleeding-jumbo-opt6/src/loader.h bleeding-jumbo-opt/src/loader.h
--- bleeding-jumbo-opt6/src/loader.h	2015-09-15 19:12:04 +0000
+++ bleeding-jumbo-opt/src/loader.h	2015-09-17 23:05:13 +0000
@@ -213,6 +213,10 @@ struct db_main {
 /* Are hashed passwords loaded into this database? */
 	int loaded;
 
+/* Base allocation sizes for "struct db_password" and "struct db_salt" as
+ * possibly adjusted by ldr_init_database() given options->flags and such. */
+	size_t pw_size, salt_size;
+
 /* Options */
 	struct db_options *options;
 

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ