ufw

Improvement: Add comment to rule

Bug #448503 reported by costales
174
This bug affects 35 people
Affects Status Importance Assigned to Milestone
ufw
Fix Released
Wishlist
Unassigned

Bug Description

I have several IP's, and it would be fine to know what means some IP. Like: IP of my office box, IP of my home network, PORT for Transmission, etc.
So could you please add comment option when is added a rule?
Thanks

Changed in ufw:
importance: Undecided → Wishlist
Revision history for this message
jefferz (jas-uci) wrote :

i vote for this, too. it would be wonderful. it's the one major thing firestarter had that ufw does not have, and it would be quite helpful to add it in. After a few rules get added, i forget what is what, and it sure would be nice to have another column for the reason. I.e.,

$ sudo ufw status
[sudo] password for jeff:
Status: active

To Action From Comment
-- ------ ---- -----------------
Anywhere ALLOW 77.49.196.38 thomas at home
Anywhere ALLOW 123.240.238.117 my 2nd box
22 ALLOW 123.240.238.118 john's workstation

where last rule might have been added by something like:

$ sudo ufw allow from 123.240.238.118 to any port 22 comment "john's workstation"

or some such..?

thanks!

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

The comment field is current used for application integration. It would be possible to add a comment, but it is non-trivial. This might be added in a future release.

Changed in ufw:
status: New → Triaged
Revision history for this message
Anders Jackson (anders-jackson) wrote :

One sollution might be to add an option like "-n" when one wants numerical values on ports and ip-adresses.
I no '-n' option, it could look up that feild in hosts, networks, protocols and services and use the result from there. Then I guess the need for comments might be less urgent.

So instead of
Anywhere ALLOW 77.49.196.34
Anywhere ALLOW 123.240.238.117
22 ALLOW 192.168.0.120

we get

Anywhere ALLOW thomas.example.org
Anywhere ALLOW my2ndbox.example.com
ssh ALLOW johnsws.local

Which is more important when we get more IPv6 addresses.

Revision history for this message
Milan Knizek (knizek) wrote :

@3: The above would help only for std ports, while e.g. rpc.bind (NFS) listens on random ports, so in order to use firewall port blocking, admin has to define some specific port (and this varies by installation or admin's will).

Comments would be really welcome.

Revision history for this message
Byron Collins (byron-collins) wrote :

comments would be very useful when trying to remember why you created a rule in the first place

Revision history for this message
Kevin Stenerson (stenersonkevin) wrote :

I'd like to make an additional comment on how useful this feature would be. There are certain servers I work with that must use a lot of non-common ports, a for some applications, and also restrict connections based on IP/etc. It would be very convenient if rules could be labeled or even tagged so that it is quicker to know whether the firewall is a problem with a service or not.
Additionaly, IPv6 support has greatly increased the number of rules that must be managed, and it can sometimes be easy to forget exactly what each one is for.

Revision history for this message
Chris Lundquist (rampantdurandal) wrote :

+1

FWIW Iptables uses C style /* comment here */ at the end of the rule when listing.

Revision history for this message
denix (denics) wrote :

Let's put in the other way round: unfortunately I cannot use ufw because I cannot add comment and I have a very complex configuration

Revision history for this message
James (james-ellis-gmail) wrote :

This would be a nice feature to have, espec when looking through historical rules.

Revision history for this message
costales (costales) wrote :

I implemented it in Gufw 13.10 :) Best regards!

Revision history for this message
Guillaume Gelin (r4mnes) wrote :

Hope this helps.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks for the patch! A couple of questions:
- What version is this against?
- What testing was performed? See README 'Testing' and 'Unit tests' for details
- Can you prepare a merge request against lp:ufw instead? This isn't strictly required, but is easier
- No tests were included as part of the patch. Can you resubmit with added unit tests (at least) for this functionality?

Also, ufw is covered by Canonical's Contributor Agreement. For your patch to be accepted, you'll need to review and sign the contributor agreement. Please see http://www.canonical.com/contributors for details.

Thanks again! :)

Revision history for this message
Guillaume Gelin (r4mnes) wrote :

It is made from the trunk (revision 836).

The patch is breaking get_status() unit test, tough it was expected since it slightly modify its behaviour. I didn't investigate further since I wasn't planning to make merge request until now. Syntax check is passing.

I'll do the new unit tests with the merge request. I just signed the contributor agreement mentioning you as my contact, I'm waiting for the answer.

Revision history for this message
Guillaume Gelin (r4mnes) wrote :

Work is in progress on lp:~rmns/ufw/message repository.

Updated patch with latest changes enclosed.

Revision history for this message
Guillaume Gelin (r4mnes) wrote :

I'm sorry, I don't have the time right now to write unit tests for those modifications. Since the latest commit passes every test, do you want me to do the merge request even without the new unit tests ?

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Please perform the merge request. When I have time to write unit tests for it, I can take a look.

Revision history for this message
Guillaume Gelin (r4mnes) wrote :
Revision history for this message
hoorid (horridguy123123) wrote :

This is REALLY needed

Revision history for this message
Maxim Litvinov (metalim) wrote :

Hi!

Any news on this one?
I'd like to keep ufw, but my collegue is forcing me to switch to FireHOL on our servers, because of the comments and our list of IP rules.

Or maybe there's possibility to have IP lists in external files with comments?

Revision history for this message
Anders Jackson (anders-jackson) wrote :

About #4, I was thinking about instead of printing IP-numbers translate it through /etc/hosts and /etc/networks (or same API as 'getent hosts' and 'getent networks')

Revision history for this message
RoyK (roysk) wrote :

I vouch for this - comments would be very nice, especially in large environments where you want to tell the other sysadmins 'this rule is to my home computer' or something like that. Often you can't add a reverse dns entry, so name lookups will not always wrork

Changed in ufw:
milestone: none → 0.35
Changed in ufw:
status: Triaged → In Progress
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks to Guillaume for the patch, but I implemented it in a different way due to changes to the codebase, lack of tests, lack of UTF-8 support and how it should work for updating/removing comments.

Changed in ufw:
status: In Progress → Fix Committed
Revision history for this message
Guillaume Gelin (r4mnes) wrote :

No worries, I hope my patch helped other people during those two years! Thank you for finally addressing that issue.

Changed in ufw:
status: Fix Committed → Fix Released
Revision history for this message
Wayne (ufwisalmostok) wrote :

Great, now the real question is: how do I do "ufw status" so that I can see the comments next to each rule! This is why I wanted to make the comment in the first place.
Thanks!

Revision history for this message
Wayne (ufwisalmostok) wrote :

I feel 'ufw status verbose' should show comments next to each rule, but it does not. Ideas?

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

'ufw status' does show the output. Eg:

$ sudo ufw reject 23 comment "disallow telnet"
Rule added
Rule added (v6)

$ sudo ufw status|grep telnet
23 REJECT Anywhere # disallow telnet
23 (v6) REJECT Anywhere (v6) # disallow telnet

Can you file a new bug, giving the output of 'ufw version' and steps to reproduce?

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.