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

wordlists: init at unstable-2020-11-23 #104712

Closed
wants to merge 7 commits into from

Conversation

Pamplemousse
Copy link
Member

Signed-off-by: Pamplemousse xav.maso@gmail.com

Motivation for this change

Wordlists are a must-have for pentesting.
I thought it would be nice to package some, for ease of use.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Pamplemousse
Copy link
Member Author

I am quite unsure if this fits nixpkgs, or if I should put that elsewhere (NUR).
Thoughts and feedback appreciated :)

@iblech
Copy link
Contributor

iblech commented Nov 24, 2020

Oh, nice! I wanted to have a couple wordlists available in the past, but was too lazy to package them. We also ship john the ripper, it might even make sense to mark these wordlists as dependencies of john, hence I do believe that these wordlists are useful additions to nixpkgs. Thank you for your work, very much appreciated.

pkgs/tools/security/wordlists/dirbuster.nix Outdated Show resolved Hide resolved
pkgs/tools/security/wordlists/wfuzz.nix Outdated Show resolved Hide resolved
pkgs/tools/security/wordlists/rockyou.nix Outdated Show resolved Hide resolved
pkgs/tools/security/wordlists/seclists.nix Outdated Show resolved Hide resolved
@Pamplemousse
Copy link
Member Author

@nixinator This can be done at "merge" time through GH's UI...

Pamplemousse and others added 5 commits May 5, 2021 08:22
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Don't pass `pkgs` to expressions, but the relevant specific package.

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Use `symlinkJoin` instead of managing the links manually.

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
@Pamplemousse
Copy link
Member Author

@SuperSandro2000 @jonringer any chance this can be merged?

This PR has been reviewed by many people already;
I have fixed things as soon as possible every time;
But the PR gets forgotten every other month;
And now, it seems to mostly attract nitpicking when I ask for "review"...

Copy link
Contributor

@fogti fogti left a comment

Choose a reason for hiding this comment

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

otherwise fine

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Copy link
Contributor

@fogti fogti left a comment

Choose a reason for hiding this comment

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

please try to incorporate my newly suggestions; I rebased them. https://github.com/zseri/nixpkgs/tree/wordlists / Pamplemousse#1

@fogti
Copy link
Contributor

fogti commented May 7, 2021

currently, makeScope is used (probably to make extending the package set easier), but extending the package set of wordlists is still not easy, because it involves overrideAttrs on withLists to be comfortably usable. Additionally, I'm unsure if we should provide an attribute withLists' which takes the list of packages directly instead of a function.

Copy link
Contributor

@fogti fogti left a comment

Choose a reason for hiding this comment

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

anyway, I would really appreciate if this PR could be merged rather sooner than later, it doesn't matter (to me) if my suggestions get merged into this PR, or if I try to get them in via the usual nixpkgs PR workflow after this has been merged. I think the interface is good enough, it works, details and interna can be improved later on / in the long run.

@Pamplemousse
Copy link
Member Author

@zseri, thanks, it took your comment on the other PR for me to realise I did not push the commit I took from what you did...

@fogti
Copy link
Contributor

fogti commented May 18, 2021

thanks.

@SuperSandro2000 please re-review. this imo is now ready to be merged.

@fogti fogti requested a review from jonringer May 18, 2021 20:40
@stale
Copy link

stale bot commented Nov 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@iblech
Copy link
Contributor

iblech commented Nov 16, 2021

Looks good also to me!

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@buckley310
Copy link
Contributor

Was putting the wordlists in a sub folder $out/share/wordlists/ considered? I don't feel too strongly either way, but it would be a little more similar to the security distributions (kali) that way.

@Pamplemousse
Copy link
Member Author

I am tired of this PR, it:

  • is almost a year old
  • has attracted two main rounds of reviews, and corrections were made as soon as possible
  • has been approved several times
  • regularly goes into limbo for several months
  • attracts new kind of nitpicking every now and then

IMHO, this was an improvement on having nothing at all, and good enough to be iterated from if needed.
Closing given that this is unlikely to be merged ever.

@jonringer
Copy link
Contributor

@Pamplemousse sorry, I think it would be beneficial to have them available in nixpkgs. But I agree, it can be painful to upstream these, especially if you're treading new water.

I would recommend creating a personal repo (or NUR repo) where this can be available.

For me personally, I don't really know how to "ensure this is the correct way forward"... not to mention it would take me to ~14 hours a day to tackle all of my github notifications a day :(

@Janik-Haag Janik-Haag mentioned this pull request Oct 10, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants