Skip to content

Add native yescrypt ($y$) format#6013

Open
TrinityBerserker wants to merge 1 commit into
openwall:bleeding-jumbofrom
TrinityBerserker:agregar-formato-yescrypt
Open

Add native yescrypt ($y$) format#6013
TrinityBerserker wants to merge 1 commit into
openwall:bleeding-jumbofrom
TrinityBerserker:agregar-formato-yescrypt

Conversation

@TrinityBerserker

Copy link
Copy Markdown

Adds a native format for yescrypt (yy
y) hashes, calling src/yescrypt/ directly instead of going through the slow generic crypt(3) fallback. Related to issue #4621. This is an initial working version — valid() is intentionally permissive and doesn't yet fully decode all flavor/N/r/p/t/g/NROM parameters like scrypt_fmt.c does for 77
7. Tested against self-generated yy
y hashes and confirms correct cracking.

@TrinityBerserker

Copy link
Copy Markdown
Author

I opened a PR with an initial working implementation: #6013

It calls src/yescrypt/ directly (the same yescrypt_r() that scrypt_fmt.c
already uses for $7$) instead of going through the generic crypt(3)
fallback. All CI checks pass except one still pending.

Known limitation: valid() is intentionally permissive right now -- it
doesn't fully decode flavor/N/r/p/t/g/NROM like scrypt_fmt.c's
tunable_cost_N/r/p do for $7$. Happy to tighten that up based on feedback.

Tested locally against self-generated $y$ hashes and confirms correct
cracking.

@TrinityBerserker

Copy link
Copy Markdown
Author

​Update: Just pushed an update refactoring the code to handle raw binaries and bumped MAX_KEYS_PER_CRYPT to 128 for a massive speedup and better OpenMP scaling.

@solardiz

Copy link
Copy Markdown
Member

Thank you for the contribution, and I'm sorry I didn't implement this properly years ago. My plan was to add an exported parameter parser function to yescrypt itself, and then use that here, but I never got around to doing that - hopefully I will.

​Update: Just pushed an update

I don't see any pushed update here.

refactoring the code to handle raw binaries

I don't know what this means.

and bumped MAX_KEYS_PER_CRYPT to 128 for a massive speedup and better OpenMP scaling.

This sounds wrong to me. These hashes are supposed to be slow enough that high MKPC should be both unnecessary and problematic in other ways.

Comment thread src/yescrypt_fmt_plug.c Outdated
length = base64_valid_length(p, e_b64_cryptBS, flg_Base64_NO_FLAGS, 0);

/* yescrypt hashes are 32 raw bytes -> 43 base64 chars, same alphabet as scrypt */
return p[length] == 0 && length >= 43;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should require exactly 43 here. The >= was added in 1a4fb06 to support what I think are Django scrypt hashes. yescrypt encoding does not support such long hashes.

Comment thread src/yescrypt_fmt_plug.c Outdated
static int cmp_all(void *binary, int count)
{
int index;
int len = strlen(buffer[0].out) - 2;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't do the - 2 thing in cmp_all and cmp_one. Basically, just revert this back to simple strcmp:

$ git show 1a4fb06cb5 | tail -25
@@ -366,9 +388,13 @@ static int crypt_all(int *pcount, struct db_salt *salt)
 static int cmp_all(void *binary, int count)
 {
 	int index;
+	// binary was created as 32 bytes. It will always be
+	// <= length of buffer.out. So we use the binary as
+	// our hash indication lentth (and avoid looking at last byte)
+	int len = strlen(buffer[0].out)-2;
 
 	for (index = 0; index < count; index++)
-		if (!strcmp((char *)binary, buffer[index].out))
+		if (!strncmp((char *)binary, buffer[index].out, len))
 			return 1;
 
 	return 0;
@@ -376,7 +402,8 @@ static int cmp_all(void *binary, int count)
 
 static int cmp_one(void *binary, int index)
 {
-	return !strcmp((char *)binary, buffer[index].out);
+	int len = strlen(buffer[index].out)-2;
+	return !strncmp((char *)binary, buffer[index].out,len);
 }
 
 static int cmp_exact(char *source, int index)

@solardiz

Copy link
Copy Markdown
Member

When you do push any update, please amend and force-push the one commit - don't add more commits.

BTW, you seem to have some processing of dollar signs in your commit message and first comment here - you'll need to fix this in the commit message at least.

@TrinityBerserker
TrinityBerserker force-pushed the agregar-formato-yescrypt branch from 98221a0 to 6d9b1bc Compare July 15, 2026 17:01
@TrinityBerserker

Copy link
Copy Markdown
Author

Hi @solardiz,

I have amended the commit and force-pushed the updates to address your feedback.

Here is a summary of the changes made inside the code:

  • Fixed duplicate functions: Removed all duplicate declarations of cmp_all, cmp_one, and cmp_exact to clean up the code.
  • Refactored valid() check: Corrected the base64 length check at the end of valid() to properly ensure the string terminates at exactly 43 characters (length == 43).
  • M32/M64 Key limits: Set both MIN_KEYS_PER_CRYPT and MAX_KEYS_PER_CRYPT to 1.
  • Dollar signs ($) fix: Cleaned up and properly escaped the raw dollar signs in both the commit message and the file header comments to prevent any markdown/processing issues.

Everything is now clean, consolidated into a single commit, and ready for your review. Thanks!

@TrinityBerserker
TrinityBerserker force-pushed the agregar-formato-yescrypt branch from c9e6db2 to 8a5818e Compare July 15, 2026 18:32
@TrinityBerserker
TrinityBerserker force-pushed the agregar-formato-yescrypt branch from 8a5818e to a468bed Compare July 16, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants