add 'local' parameter to seport

[richm]

• local: true:
  • state: present enforces change to be made even though the
    port mapping could already exists in built in policy
    *state: absent would remove only local modification and would not
    try to remove builtin mapping.

Implemented using local_seport module which is copied from seport module
and update to accept local

[richm]

[citest]

[nhosoi]

If we specify local: true for the state: present case, the same port is allowed to be added twice.

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

Please note that 6514 is defined in the selinux policy. Interestingly, one 6514 is considered a local port. Is it the way how it is implemented?

# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 6514, 7514
# semanage port --delete --proto tcp 6514
# semanage port --list | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      7514, 16514, 6514, 10514
# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 7514

[richm]

If we specify local: true for the state: present case, the same port is allowed to be added twice.

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

Please note that 6514 is defined in the selinux policy. Interestingly, one 6514 is considered a local port. Is it the way how it is implemented?

Correct. It is essentially doing semanage port -m instead of semanage port -a

And this is what allows you to do semanage port -d - you are "removing" the duplicate one, leaving only the read-only system policy in place.

Not sure why it is implemented this way . . . but that's how semanage port -m works.

# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 6514, 7514
# semanage port --delete --proto tcp 6514
# semanage port --list | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      7514, 16514, 6514, 10514
# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 7514

[nhosoi]

If we specify local: true for the state: present case, the same port is allowed to be added twice.

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

Not sure why it is implemented this way . . . but that's how semanage port -m works.

# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 6514, 7514
# semanage port --delete --proto tcp 6514
# semanage port --list | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      7514, 16514, 6514, 10514
# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 7514

Ok..., now I'd like to learn the state value(?) equivalent to semanage port -m if exists...
This ssylog_tls_port_t list was generated by running fedora.linux_system_roles.selinux with [0] twice. Or for state: present, we should not specify local: true?

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

[0] logging_selinux_ports variable

        logging_selinux_ports:
          - ports: 6514
            proto: tcp
            setype: syslog_tls_port_t
            state: present
            local: true

Additional thought... as it does users very little harm by having another local port, I have actually no problem with this issue.

[richm]

If we specify local: true for the state: present case, the same port is allowed to be added twice.

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

Not sure why it is implemented this way . . . but that's how semanage port -m works.

# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 6514, 7514
# semanage port --delete --proto tcp 6514
# semanage port --list | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      7514, 16514, 6514, 10514
# semanage port --list -C | grep tcp | grep syslog_tls_port_t
syslog_tls_port_t              tcp      16514, 7514

Ok..., now I'd like to learn the state value(?) equivalent to semanage port -m if exists... This ssylog_tls_port_t list was generated by running fedora.linux_system_roles.selinux with [0] twice. Or for state: present, we should not specify local: true?

We should always use local: true - otherwise, we will have to have detailed information about the built-in policy of the system in order to know which ports should be used with state: present and which ports to use with state: absent.

syslog_tls_port_t              tcp      7514, 6514, 16514, 6514, 10514

[0] logging_selinux_ports variable

        logging_selinux_ports:
          - ports: 6514
            proto: tcp
            setype: syslog_tls_port_t
            state: present
            local: true

[nhosoi]

lgtm

[bachradsusi]

I originally used local_seport as I wasn't sure whether the shipped module shipped with the role would override the collections module. But according to https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html#embedding-modules-and-plugins-in-roles it should be safe to just copy in and use seport module. It would make the later transition to collection module simpler.

[richm]

I originally used local_seport as I wasn't sure whether the shipped module shipped with the role would override the collections module. But according to https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html#embedding-modules-and-plugins-in-roles it should be safe to just copy in and use seport module. It would make the later transition to collection module simpler.

I wasn't sure how Ansible would handle having two seport modules - one in the role and one in the community.general collection. I'll test with both and see what happens.

[richm]

I originally used local_seport as I wasn't sure whether the shipped module shipped with the role would override the collections module. But according to https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html#embedding-modules-and-plugins-in-roles it should be safe to just copy in and use seport module. It would make the later transition to collection module simpler.

I wasn't sure how Ansible would handle having two seport modules - one in the role and one in the community.general collection. I'll test with both and see what happens.

It works. It uses the one from the role.
Next problem - we'll need to change the way we vendor in the module code - https://src.fedoraproject.org/rpms/linux-system-roles/blob/rawhide/f/linux-system-roles.spec#_367
instead of

  cp -pL .external/ansible/posix/plugins/modules/$module $role/library/$module

we'll have to use

  cp -pL --no-clobber .external/ansible/posix/plugins/modules/$module $role/library/$module

There are a few places in the spec file we'll need to do that.

However, we don't need to do this unless we do a downstream release before community.general 5.6.0 is released: ansible-collections/community.general#582

Next release

5.6.0 probably September 12th

So when 5.6.0 is released, I will remove the local seport.py

[richm]

community.general 5.6.0 seport now supports local - https://github.com/ansible-collections/community.general/blob/5.6.0/plugins/modules/system/seport.py#L52

[richm]

[citest]

[richm]

[citest bad]

[richm]

[citest]

[richm]

Note that this will need a special case handling in the downstream spec file, something like this:

# community.general 5.6.0 added support for the local parameter to seport
# We have to use the FQCN for seport upstream when running with ansible 2.9
# because otherwise the built-in ansible 2.9 seport will always be used
# instead of the collection version.
find -P . -name \*.yml | while read file; do
  sed -i -e \
    's/community.general.system.seport:/seport:/g' \
    "$file"
done

[richm]

ugh - this may be a big problem for users using ansible 2.9 - as soon as you upgrade:

TASK [initial changes] *********************************************************
task path: /path/to/selinux/tests/tests_port.yml:6
Wednesday 14 September 2022  17:03:37 -0600 (0:00:00.907)       0:00:01.984 *** 
ERROR! couldn't resolve module/action 'community.general.system.seport'. This often indicates a misspelling, missing collection, or incorrect module path.

So, even if you don't care about local, you are forced to care about it, because now your playbooks are broken until you deal with it. Even worse - if you are using ansible 2.9 because you don't want to use collections, or are unable to use collections, you now must. I think this is poor UX.

I think we'll have to use local_seport.py until we can drop support for ansible 2.9 - I can't really think of a good way to handle this otherwise that takes into consideration {ansible-2.9,ansible-core-2.x} X {legacy role format,collection format} X {upstream,downstream}.

Other suggestions are welcome

[richm]

[citest]

[richm]

@nhosoi @bachradsusi please review again