Skip to content

PuTTY .ppk v3 support#6003

Open
elipsion wants to merge 3 commits into
openwall:bleeding-jumbofrom
elipsion:bleeding-jumbo
Open

PuTTY .ppk v3 support#6003
elipsion wants to merge 3 commits into
openwall:bleeding-jumbofrom
elipsion:bleeding-jumbo

Conversation

@elipsion

Copy link
Copy Markdown
  • Extends putty2john with support for reading Argon fields
  • Extends putty_fmt_plug to consume Argon related data
  • Formatting with indent and astyle

@solardiz

Copy link
Copy Markdown
Member

Thank you!

We need sample input file(s) to test this with, and to be able to re-test it later. Can you please contribute some to https://github.com/openwall/john-samples/tree/main/PuTTY?

The format should start reporting tunable costs - algorithm and its parameters. Looks like up to 4 total in this case.

For hex parsing, we should probably use shared code from common.[ch]. Some other *2john programs depend on these files. In Makefile.in, we have:

../run/hccap2john@EXE_EXT@: hccap2john.o common.o jumbo.o
        $(LD) $(LDFLAGS) @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ common.o hccap2john.o jumbo.o @OPENMP_CFLAGS@ -o $@

../run/putty2john@EXE_EXT@: putty2john.o jumbo.o
        $(LD) $(LDFLAGS) @PTHREAD_CFLAGS@ @PTHREAD_LIBS@ putty2john.o jumbo.o @OPENMP_CFLAGS@ -o $@

so this is where the extra dependency will go.

Please also add a doc/NEWS item to the end of the post-1.9.0 list, currently around line 490.

* switch to common.h for hex decoding
* add tunable costs
* update doc/NEWS
@elipsion

Copy link
Copy Markdown
Author

Thanks for the feedback! Addressed in this PR and john-samples#47

@solardiz solardiz left a comment

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.

Thank you! This is moving in the right direction, but I think there are a few more things to fix here. Also, please roll all of the review feedback changes into your first commit (and force-push), so that we can preserve your commits as they are when merging this yet not have the PR-temporary review/change history stay in the project (such micro-history is of no long-term relevance).

Comment thread doc/NEWS
The formats needed no changes. [magnum; 2025]

- PuTTY format and putty2john: Add support for PPKv3 with Argon2 KDF. This
introduces tunable cost for the PuTTY format. [elipsion; 2026]

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.

Please preserve (re-add) the blank line after this (should be two blank lines separating this from older release news), and I suggest dropping the mention of tunable costs (too minor, as I recall we don't include that in news for other formats).

Comment thread src/putty2john.c
goto error;
if ((b = read_body(fp)) == NULL)
goto error;
argon2_memory = atoi(b);

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.

To avoid potential UB in this and other atoi calls, I suggest pre-checking with our isdec function.

Comment thread src/putty_fmt_plug.c
#define FORMAT_TAG "$putty$"
#define FORMAT_TAG_LEN (sizeof(FORMAT_TAG)-1)
#define ALGORITHM_NAME "SHA1/AES 32/" ARCH_BITS_STR
#define ALGORITHM_NAME "SHA1/AES/Argon2 32/" ARCH_BITS_STR

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.

The "32/" part becomes wrong for Argon2, which can use more of the word width... or actually also SIMD. Need to check how we report this in other formats where Argon2 is also optional and do it consistently.

Should also list SHA256.

Comment thread src/putty_fmt_plug.c
{NULL}
{ "$putty$1*16*1*0*69396df4513221459e8302f2b84b56d1f078cce1*51*0000000b7373682d6564323535313900000020abed4c34945b8e98fad03669eba5911b5890e7070d5212547128c2b586c9cba5*48*878992fc0f3bd20a88d182bb9f765ceb259e1076da2c7d4a0987b95bc692c690886f2020b5959399550cb9224cc71f1a*ssh-ed25519*aes256-cbc*ed25519-key-20170722", "openwall" },
{ "$putty$1*16*1*0*d931af6335088577da918d60a77f3c097d76620a*104*0000001365636473612d736861322d6e69737470323536000000086e6973747032353600000041046bb900eb809a5be6ec1bda5aac286ac9a2e0c7e0bfab317623ccf9b8b47baaedc0a2498287df6cb3a07165461b40ac1dba2f492be96ec841bfcbf93df9d31a43*48*ba7ba53ca50e05e15ba4ea19f2c6891298af84bf7280ea4bdcb7fa0611a9816a5966f972cd4a1eee37a42ac69489601c*ecdsa-sha2-nistp256*aes256-cbc*ecdsa-key-20170722", "openwall" },
/* PuTTYgen 0.83-1 from June, 2026 *//* It uses weaker-than-normal parameters to not overwhelm the test suite */

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.

Great, but we also need to be able to request realistic benchmark if possible. What are the normal parameters?

We may also want to have 2+ test vectors per KDF type, so that things such as salt switching are tested. Those extra tests may in fact use weaker settings.

Comment thread src/putty_fmt_plug.c
"t",
"m",
"p",
"type [0:none 1:Argon2d 2:Argon2i 3:Argon2id]"

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.

"type" should be the first tunable cost, not the last.

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