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

knock: init at 0.8 #356740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

knock: init at 0.8 #356740

wants to merge 1 commit into from

Conversation

theobori
Copy link
Member

@theobori theobori commented Nov 17, 2024

A port-knocking implementation.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@theobori theobori added the needs_reviewer (old Marvin label, do not use) label Nov 25, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch does quite a bit more than just moving the output destination. You can probably accomplish the same result with substituteInPlace in the postPatch phase.

pkgs/by-name/kn/knock/package.nix Outdated Show resolved Hide resolved
@xanderio xanderio added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 3, 2024
Copy link
Member

@Aleksanaa Aleksanaa left a comment

Choose a reason for hiding this comment

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

This may not be accepted as the project lacks activity and it is said to have an unrevealed vulnerability (jvinet/knock#91)

Comment on lines +30 to +35
substituteInPlace Makefile.am \
--replace-fail "sbin_PROGRAMS = knockd" "bin_PROGRAMS += knockd" \
--replace-fail "dist_sbin_SCRIPTS = src/knock_helper_ipt.sh" "" \
--replace-fail "sysconf_DATA = knockd.conf" "" \
--replace-fail "dist_doc_DATA = README.md TODO ChangeLog COPYING" "" \
--replace-fail "src/knock_helper_ipt.sh" ""
Copy link
Member

Choose a reason for hiding this comment

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

Please use patch instead, this looks painful

Copy link
Member

Choose a reason for hiding this comment

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

(The review above does not say that you should move the complicated parts to substituteInPlace)

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about just patching the sbin_PROGAMS part ^^', in this case this should indeed be a patch.

'';

meta = {
description = "A port-knocking implementation";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "A port-knocking implementation";
description = "Port-knocking implementation";
longDescription = ''
This is a port-knocking server/client. Port-knocking is
a method where a server can sniff one of its interfaces
for a special "knock" sequence of port-hits. When
detected, it will run a specified event bound to that port
knock sequence. These port-hits need not be on open
ports, since we use libpcap to sniff the raw interface traffic.
'';

license = lib.licenses.gpl2Plus;
maintainers = with lib.maintainers; [ theobori ];
mainProgram = "knock";
platforms = lib.platforms.unix;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
platforms = lib.platforms.unix;
platforms = lib.platforms.linux;

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 4, 2024
@Aleksanaa Aleksanaa added awaiting_changes (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes awaiting_changes (old Marvin label, do not use)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants