• Ondrej Mosnacek's avatar
    selinux: fix cond_list corruption when changing booleans · d8f5f0ea
    Ondrej Mosnacek authored
    Currently, duplicate_policydb_cond_list() first copies the whole
    conditional avtab and then tries to link to the correct entries in
    cond_dup_av_list() using avtab_search(). However, since the conditional
    avtab may contain multiple entries with the same key, this approach
    often fails to find the right entry, potentially leading to wrong rules
    being activated/deactivated when booleans are changed.
    
    To fix this, instead start with an empty conditional avtab and add the
    individual entries one-by-one while building the new av_lists. This
    approach leads to the correct result, since each entry is present in the
    av_lists exactly once.
    
    The issue can be reproduced with Fedora policy as follows:
    
        # sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
        allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
        allow ftpd_t public_content_rw_t:dir { add_name create link remove_name rename reparent rmdir setattr unlink watch watch_reads write }; [ ftpd_anon_write ]:True
        # setsebool ftpd_anon_write=off ftpd_connect_all_unreserved=off ftpd_connect_db=off ftpd_full_access=off
    
    On fixed kernels, the sesearch output is the same after the setsebool
    command:
    
        # sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
        allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
        allow ftpd_t public_content_rw_t:dir { add_name create link remove_name rename reparent rmdir setattr unlink watch watch_reads write }; [ ftpd_anon_write ]:True
    
    While on the broken kernels, it will be different:
    
        # sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
        allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
        allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
        allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
    
    While there, also simplify the computation of nslots. This changes the
    nslots values for nrules 2 or 3 to just two slots instead of 4, which
    makes the sequence more consistent.
    
    Cc: stable@vger.kernel.org
    Fixes: c7c556f1 ("selinux: refactor changing booleans")
    Signed-off-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
    Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
    d8f5f0ea
avtab.c 16.3 KB