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

firewall-port check reports the inverse of reality #6073

Open
0byt3 opened this issue Oct 22, 2024 · 3 comments
Open

firewall-port check reports the inverse of reality #6073

0byt3 opened this issue Oct 22, 2024 · 3 comments

Comments

@0byt3
Copy link

0byt3 commented Oct 22, 2024

Here is the behaviour I am seeing with /etc/xapi.d/plugins/firewall-port:
Screenshot from 2024-10-22 08-02-03

The change that would work.

    check)
        if [[ -z `iptables -S $CHAIN | grep " $PORT "` ]]
        then
            echo "Port $PORT open: true"
        else
            echo "Port $PORT open: false"
        fi
        ;;

to

    check)
        if [[ -z `iptables -n -v -S $CHAIN | grep " $PORT "` ]]
        then
            echo "Port $PORT open: true"
        else
            echo "Port $PORT open: false"
        fi
        ;;

If I knew how to do a git pull request and all that I'd do it, however I do not.

@robhoes
Copy link
Member

robhoes commented Oct 22, 2024

The commit message that added the check option says:

CP-40754 The firewall-port script returns true if port 80 is blocked and false if it is closed, this is captured in set_https_only to update the DB based on the tate of the network not the requested setting should there be a failure.

So it does indeed return true if the port is blocked, but unfortunately, the string says "open". So the best fix would be to change the word "open" to "closed" in the script, to avoid other side effects.

@robhoes
Copy link
Member

robhoes commented Oct 22, 2024

Ah, then we do in addition need to update L133 in dbsync_slave.ml to match...

@0byt3
Copy link
Author

0byt3 commented Oct 22, 2024

I had edited my suggested fix to incorporate forcing iptables to report all ports as numeric. 3493 is for NUT server and so iptables replaces 3493 with "nut" in the output and so it won't match, but then I ended up removing the inverted check fix.

existing check code:

    check)
        if [[ -z `iptables -S $CHAIN | grep " $PORT "` ]]
        then
            echo "Port $PORT open: true"
        else
            echo "Port $PORT open: false"
        fi
        ;;

Need to add the "-v" to make sure ports like 3493 don't output as "nut" and therefore would affect the grep matching

    check)
        if [[ -z `iptables -v -S $CHAIN | grep " $PORT "` ]]
        then
            echo "Port $PORT closed: true"
        else
            echo "Port $PORT closed: false"
        fi
        ;;

or

 check)
        if ! iptables -v -S $CHAIN | grep -s -q " $PORT "
        then
            echo "Port $PORT closed: true"
        else
            echo "Port $PORT closed: false"
        fi
        ;;

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

No branches or pull requests

2 participants