Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oubliette Password Manager support #5680

Merged
merged 11 commits into from
Mar 4, 2025

Conversation

solardiz
Copy link
Member

@solardiz solardiz commented Mar 1, 2025

I didn't know I can create a GitHub PR from a third-party repo, but apparently I can? So doing this to record in here this unfinished work by @davidedg that I just came across "by accident". This is not ready in several ways, including that there's no *2john tool to extract those "hashes" yet, and that the attempts at SIMD here are just stubs (better drop them from the first real PR). Also, this can probably be one format rather than two, and the source code could be formatted more similarly to ours.

@solardiz
Copy link
Member Author

solardiz commented Mar 1, 2025

The password manager is pretty ancient, no wonder it doesn't appear to use a real KDF (if the code here is correct).

https://sourceforge.net/projects/oubliette/

Full-featured password manager for Windows95/98/2K/XP, with strong encryption. Store passwords for internet accounts, PIN numbers, credit cards. Securely encrypts data with Blowfish or IDEA. Easy navigation, search, import/export, clipboard protection.

License
Mozilla Public License 1.1 (MPL 1.1)

@solardiz
Copy link
Member Author

solardiz commented Mar 1, 2025

CI: Fails build --without-openssl, fails our whitespace-errors check.

@davidedg
Copy link
Contributor

davidedg commented Mar 1, 2025

including that there's no *2john tool to extract those "hashes" yet

Hi, this is the hash extractor: oubliette2john.py

Yes, the plugin is not finished, I do not have much time to continue the development.
It is working with both simple and complex passwords (I tested up to ~120 characters with non-printable ascii), but only with CPU (no experience to write an OpenCL implementation).

I based this work on the Keepass 1.x Oubliette import tool

If you need sample Oubliette password files, or any other info, I'm happy to provide.

Regards.
Davide.

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

Thank you very much @davidedg!

Yes, the plugin is not finished, I do not have much time to continue the development.

I think it would be unreasonable to expect you to implement SIMD and OpenCL, but maybe you do have time to do at least some of the following?

  1. Add your copyright + license statements. If you used code (not just learned the file format, etc.. but literally took code) from somewhere else (you mention "I based this work on the Keepass 1.x Oubliette import tool"), which I guess you did not but am asking just in case, then the copyright + license statements should also reflect that (their copyright, and the license should be compatible with theirs). Other than that, our preferred license is 0-clause BSD as seen in many other source files in here (see excerpt below).
  2. Fix build --without-openssl (use our own implementations of the crypto primitives in that case, or exclude the format from such builds - see e.g. oracle_fmt_plug.c for how the latter is done). However, when OpenSSL is available, then continue to use it (as it may provide faster scalar SHA-1 on some CPUs due to usage of SHA-NI, which we don't have own code for).
  3. Fix whitespace-errors. To reproduce them on your system, run git diff-index --check --cached b1b622f691d40196815939e4736a5da71befd206 (the same command our CI job runs).
  4. Drop the unimplemented SIMD stuff completely (the way it is, it's also not bringing us any closer to a proper implementation, which would be significantly different).
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted.
 *
 * There's ABSOLUTELY NO WARRANTY, express or implied.

If you need sample Oubliette password files, or any other info, I'm happy to provide.

Yes, we need sample files - via a PR to the https://github.com/openwall/john-samples repo, please.

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

@exploide Please take a look at oubliette2john.py added here. Thank you!

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

3. Fix whitespace-errors.

When you do this, perhaps also use this opportunity to reformat the source code to indent with 8-char tabs like we normally use.

Copy link
Contributor

@exploide exploide left a comment

Choose a reason for hiding this comment

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

Python script looks good, easy and comprehensible. Just left two minor nits.

return 0

except IOError as e:
sys.stderr.write(f"{filename}: {str(e)}\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

str() function is unnecessary here and will be called implicitly anyway. {e} is enough for simplification.


def usage():
sys.stderr.write("Usage: oubliette2john.py <oubliette files>\n")
sys.exit(-6)
Copy link
Contributor

Choose a reason for hiding this comment

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

exit codes are unsigned integers. Please use positive numbers here and above.

@davidedg
Copy link
Contributor

davidedg commented Mar 2, 2025

Yes, we need sample files - via a PR to the https://github.com/openwall/john-samples repo, please.

https://github.com/davidedg/john-samples/pull/1/commits

1. Add your copyright + license statements. _If_ you used _code_ 

No, I just used their existing code (rather than the original Oubliette code) to understand how the hashing worked, because it was much easier.
I added the license statement.

2. Fix build `--without-openssl` 
4. Drop the unimplemented SIMD stuff completely

I restored that dev environment and tried configuring without openssl, I'll try to get some time to work on these, but can't promise it'll be soon.

3. Fix `whitespace-errors`.

Tried, seems I'm unable to reproduce:

dev@dev: /src$ git clone https://github.com/davidedg/john.git
Cloning into 'john'...
remote: Enumerating objects: 99316, done.
remote: Counting objects: 100% (440/440), done.
remote: Compressing objects: 100% (136/136), done.
remote: Total 99316 (delta 345), reused 307 (delta 304), pack-reused 98876 (from 2)
Receiving objects: 100% (99316/99316), 130.14 MiB | 19.11 MiB/s, done.
Resolving deltas: 100% (77766/77766), done.
dev@dev: /src$ cd john/
dev@dev: /src/john$ git diff-index --check --cached b1b622f
dev@dev: /src/john$

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

Tried, seems I'm unable to reproduce:

Looks like you forgot git checkout oubliette-plugin, so you checked our default branch.

@davidedg
Copy link
Contributor

davidedg commented Mar 2, 2025

Looks like you forgot git checkout oubliette-plugin, so you checked our default branch.

good catch, that should be fixed now.

When you do this, perhaps also use this opportunity to reformat the source code to indent with 8-char tabs like we normally use.

Using VSCode, if I pick any other file and try detect the indentation from content, it picks 4-char tabs.
Any reference file I can look at? Or, always using VSCode, what settings should I set for the conversion?

@davidedg
Copy link
Contributor

davidedg commented Mar 2, 2025

Python script looks good, easy and comprehensible. Just left two minor nits.

integrated your suggestions, thx!

4. Drop the unimplemented SIMD stuff completely

done!

@davidedg
Copy link
Contributor

davidedg commented Mar 2, 2025

2. Fix build --without-openssl (use our own implementations of the crypto primitives in that case, or exclude the format from such builds - see e.g. oracle_fmt_plug.c for how the latter is done).

For now I excluded the format on no-openssl builds.

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

I forgot to tell you that we normally prefix commit titles with subsystem names, so e.g. Oubliette: Excluded these formats in --without-openssl builds. Also for fix-up commits such as correcting whitespace errors with a not-yet-merged PR, we generally amend the original commits and force-push instead of adding more commits - although I guess VSCode or whatever you use may make this kind of workflow difficult for you? Anyway, now that you already went with adding many more commits, we should probably plan on temporarily enabling squash merging for this repo and using squash-and-merge on this PR (so it'll become just one commit).

This is once again failing whitespace-errors, I guess you didn't re-run this check after making further changes:

src/oubliette_blowfish_fmt_plug.c:4: trailing whitespace.
src/oubliette_idea_fmt_plug.c:4: trailing whitespace.

Using VSCode, if I pick any other file and try detect the indentation from content, it picks 4-char tabs.

Oh. Maybe it just doesn't know what tab width we intended.

Any reference file I can look at? Or, always using VSCode, what settings should I set for the conversion?

I'm not familiar with VSCode. I guess there may be a configuration setting for tab width.

@solardiz
Copy link
Member Author

solardiz commented Mar 2, 2025

@davidedg Please also add a doc/NEWS entry about your contribution. Thank you!

Copy link
Member

@claudioandre-br claudioandre-br left a comment

Choose a reason for hiding this comment

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

The following can be done by the maintainer just to speed up the process:

  • I'm afraid this will have to be done before the merge:
$ git pull --rebase [email protected]:openwall/john.git
$ git reset --soft HEAD~10
$ git commit -m "Oubliette: Added Oubliette Password Manager Hash creator" # Or something like this

$ indent -kr -i4 -ts4 -nlp -nbbo -ncs -l119 -lc119 -bad -il0 src/oubliette_*.c
$ astyle --style=kr -t4 -U -H -p -xC119 -c -k3 -z2           src/oubliette_*.c

$ git mv src/oubliette2john.py run/
$ git commit -a --amend
  • Move the docs/NEWS note to the proper place (the end of the list, below b55f339)
  • Set the file's execution flag .

To compare (or use as a reference):
0001-Oubliette-Added-Oubliette-Password-Manager-Hash-crea.PATCH

@solardiz
Copy link
Member Author

solardiz commented Mar 3, 2025

I'm afraid this will have to be done before the merge

... or right after merge, as a separate commit.

@davidedg
Copy link
Contributor

davidedg commented Mar 3, 2025

Guys, these are 3 simple files... how about:

  • you cancel this PR
  • I delete my oubliette branch and create a new one based on the current main branch
  • I incorporate all the changes so far (including formatting, btw thx @claudioandre-br I was not aware of those tools!)
  • a new PR can be raised, which will contain a single commit?

Let me know.

@solardiz
Copy link
Member Author

solardiz commented Mar 3, 2025

Guys, these are 3 simple files... how about:

Thank you for offering this, but now that this PR isn't in too bad a shape (even CI is happy), I'd rather just squash-and-merge it (one commit) and then add a commit of my own to clean things up. I'm sure there will be other opportunities for you to contribute to our project more when your time permits.

@solardiz solardiz merged commit aeedc6b into openwall:bleeding-jumbo Mar 4, 2025
35 of 36 checks passed
@solardiz
Copy link
Member Author

solardiz commented Mar 4, 2025

I just did the squash-and-merge thing, but unfortunately GitHub changed the way they do it since last time I used this feature. Previously, they would set the original PR's author as commit author (and GitHub as the committer, which was weird), but now they set me (the one merging the PR) as the commit author (and still GitHub as the committer, which is still weird). Luckily, the suggested commit message included Co-authored-by: Davide Del Grande <[email protected]>, which I preserved, so Davide is seen as a contributor to this commit as far as GitHub history is concerned.

@solardiz
Copy link
Member Author

solardiz commented Mar 4, 2025

GitHub changed the way they do it since last time I used this feature. Previously, they would set the original PR's author as commit author (and GitHub as the committer, which was weird), but now they set me (the one merging the PR) as the commit author

Oh! I think I realize what may have changed - this time, I was also the one to create the PR, even if from Davide's repo. So maybe nothing changed on GitHub's end, after all.

@solardiz
Copy link
Member Author

solardiz commented Mar 4, 2025

I've made some basic cleanups and corrections via subsequent commits.

However, testing this against the test vectors from openwall/john-samples#31 I am only able to directly crack the simple password 12345678. For cracking the complex password, I have to first process the wordlist through iconv -f utf8 -t iso-8859-1. I guess it got inadvertently converted the other way somewhere on the way to git commit? Should we replace it with the result of this iconv with a subsequent commit?

Oh, alternatively I am able to get it cracked by adding -target-enc=iso-8859-1.

@magnumripper please suggest how to fix this encoding issue best, to minimize user confusion and users' wasted time on running with wrong encoding settings. Right now, by default we print Using default input encoding: UTF-8, but with the input wordlist actually in UTF-8 we fail to crack this password. So it feels like a bug.

@solardiz
Copy link
Member Author

solardiz commented Mar 4, 2025

I've just created #5681 for the character encoding issue - let's discuss that one in there.

@davidedg
Copy link
Contributor

davidedg commented Mar 4, 2025

which I preserved, so Davide is seen as a contributor to this commit as far as GitHub history is concerned

That's no worries, my name is still in the file's comments.
Many thanks for helping integrating my patch in the main branch!

Oh, alternatively I am able to get it cracked by adding -target-enc=iso-8859-1.

That's how I was testing it.
I remember having some hard time while testing the complex password stuff, having to change back and forth from one format/approach to another, until I read this
https://github.com/openwall/john/blob/bleeding-jumbo/doc/ENCODINGS, which I remember shed some light.

Not sure if I handled everything the way is supposed to be in the source (especially for the FMT* stuff...)

@davidedg davidedg deleted the oubliette-plugin branch March 4, 2025 10:43
@magnumripper
Copy link
Member

I've just created #5681 for the character encoding issue - let's discuss that one in there.

Did so. Also, I reviewed the format flags (that affects encoding) in this PR and they should be correct.

@solardiz
Copy link
Member Author

solardiz commented Mar 5, 2025

FWIW, the Blowfish kind of this format is somewhat slow because it involves Blowfish key setup, which is known-slow. I guess it's reasonable to expect this to be about 65 times (as 1+2*32) faster than bcrypt cost 5 (as we normally benchmark bcrypt here). In my tests, it's actually slower than that, at 40% to 65% the speed we get at bcrypt times 65, which isn't bad considering that for bcrypt on CPU we interleave 2 or 3 instances and here we use OpenSSL's code that is single-instance.

Just like for bcrypt, there isn't much opportunity to use SIMD on CPU because CPUs' SIMD gather instructions still tend to be too inefficient to provide much or any speedup over scalar code. Of course, SIMD can be used for SHA-1, but given that most time is spent in Blowfish, the overall speedup from that would be minimal (it'd make more of a difference for the IDEA kind).

https://www.openwall.com/presentations/Passwords14-Energy-Efficient-Cracking/slide-07.html

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.

5 participants