independent with attribute declararion oeder for attachment

Hi,

When I read the source code of checkpolicy, I noticed an interesting functionality is commented out by #if 0 - #endif.
That is automatically attributes definition on type declaration statements. I can look the author intended to try to attach attributes with any types before this attributes declared.

Try to see, by less +1700 checkpolicy-1.23.4/policy_parse.y. ---- checkpolicy-1.23.4/policy_parse.y --

        while ((id = queue_remove(id_queue))) {
                attr = hashtab_search(policydbp->p_types.table, id);
                if (!attr) {
                        sprintf(errormsg, "attribute %s is not declared", id);
#if 1
                        /* treat it as a fatal error */
                        yyerror(errormsg);
                        return -1;
#else
                        /* Warn but automatically define the attribute.
                           Useful for quickly finding all those attributes you
                           forgot to declare. */
                        yywarn(errormsg);
                        attr = (type_datum_t *) malloc(sizeof(type_datum_t));
                        if (!attr) {
                                yyerror("out of memory");
                                return -1;
                        }
                        memset(attr, 0, sizeof(type_datum_t));
                        attr->isattr = TRUE;
                        ret = hashtab_insert(policydbp->p_types.table,
                                             id, (hashtab_datum_t) attr);
                        if (ret) {
                                yyerror("hash table overflow");
                                return -1;
                        }
                        newattr = 1;
#endif
                } else {
                        newattr = 0;
                }

-----------------------------------------

The disabled section works similar as an ATTRIBUTE statement. But such automaticalyl declaration conflicts with normal ATTRIBUTE statement by "duplicate declaration for attribute %s\n".

Currently, we must declare an attribute before attachment to any types. Thus, almost attributes are declared in attrib.te and attrib.te's merging order for policy.conf is earlier than any *.te files.

The attached checkpolicy-1.23.4-O4A.patch resolves this limitation. For example, we can use an attribute declared in postgresql.te for apache's configuration although apache.te is merged into policy.conf ealier than postgresql.te.

Of cause, existing semantics is not changed without an exception. When we declare a type with undeclared attributes and thoes attributes are not declared untill the last, the attached attributes are ignored. There are two reason. (1) It's harmless since any TE statements with undeclared attributes are restricted. (2) We should not worry about the dependence of type and attribute, so this feature make reduce 'ifdef/ifndef' macros.
Currently, checkpolicy will abort when we declare a type with undeclared attribtes. Is this difference so fatal ?

The following actions are same as current checkpolicy.
* ALLOW and any TE statements with undeclared attributes are restricted.
* Duplicate attribute declaration is restricted.
* Any attribute need a declaration by ATTRIBUTE statement.

The following four patches are sample of the out of order of attribute declaration.
- apache.te-1.23.17-3.attribute.patch
- ftpd.te-1.23.17-3.attribute.patch
- mysqld.te-1.23.17-3.attribute.patch
- postgresql.te-1.23.17-3.attribute.patch

BTW, I noticed a problem that any CGI program works in httpd_sys_script_t can not connect to PostgreSQL via UNIX domain socket. This patch resolve it. Since I think configuration for apache is done in postgresql.te is strange, I used postgresql_connectable_a as a interface for PostgreSQL client application.

Thank.
--
KaiGai Kohei <[email protected]>

@@ -219,17 +219,15 @@
 # Creation of lock files for apache2
 lock_domain(httpd)
 
-# connect to mysql
-ifdef(`mysqld.te', `
-can_unix_connect(httpd_php_t, mysqld_t)
-can_unix_connect(httpd_t, mysqld_t)
-can_unix_connect(httpd_sys_script_t, mysqld_t)
-allow httpd_php_t mysqld_var_run_t:dir search;
-allow httpd_php_t mysqld_var_run_t:sock_file write;
-allow { httpd_t httpd_sys_script_t } mysqld_db_t:dir search;
-allow { httpd_t httpd_sys_script_t } mysqld_db_t:sock_file rw_file_perms;
-allow { httpd_t httpd_sys_script_t } mysqld_var_run_t:sock_file rw_file_perms;
-')
+# connect to mysql/PostgreSQL
+typeattribute httpd_t mysqld_connectable_a;
+typeattribute httpd_php_t mysqld_connectable_a;
+typeattribute httpd_sys_script_t mysqld_connectable_a;
+
+typeattribute httpd_t postgresql_connectable_a;
+typeattribute httpd_php_t postgresql_connectable_a;
+typeattribute httpd_sys_script_t postgresql_connectable_a;
+
 allow httpd_t bin_t:dir search;
 allow httpd_t sbin_t:dir search;
 allow httpd_t httpd_log_t:dir remove_name;

@@ -18,4 +18,8 @@
 extern unsigned int policyvers;
 extern unsigned int mlspol;
 
+/* isattr of type_datum_t takes the value of FALSE, TRUE and TRUE_BUT_UNDECLARED
+   An attribute with TRUE_BUT_UNDECLARED will be reclaimed before phase 2.  */
+#define TRUE_BUT_UNDECLARED 2
+
 #endif

@@ -157,6 +157,27 @@
 	return 0;
 }
 
+static int check_undeclared_attr(hashtab_key_t key __attribute__ ((unused)),
+				 hashtab_datum_t datum, void *p __attribute__ ((unused)))
+{
+	type_datum_t *typdatum;
+
+	typdatum = (type_datum_t *) datum;
+	if (typdatum->isattr==TRUE_BUT_UNDECLARED)
+		return 1;
+	return 0;
+}
+
+static void destroy_undeclared_attr(hashtab_key_t key, hashtab_datum_t datum, void *p __attribute__ ((unused)))
+{
+	type_datum_t *typdatum;
+
+	typdatum = (type_datum_t *) datum;
+	ebitmap_destroy(&typdatum->types);
+	free(key);
+	free(datum);
+}
+
 #ifdef EQUIVTYPES
 static int insert_type_rule(avtab_key_t *k, avtab_datum_t *d, 
 			    struct avtab_node *type_rules)

@@ -600,6 +621,10 @@
 			fprintf(stderr, "%s:  error(s) encountered while parsing configuration\n", argv[0]);
 			exit(1);
 		}
+		/* Remove undeclared and automatically generated attributes before phase 2. */
+		hashtab_map_remove_on_error(policydb.p_types.table,
+					    check_undeclared_attr, destroy_undeclared_attr, 0);
+
 		rewind(yyin);
 		policydb_lineno = 1;
 		source_file[0] = '\0';

@@ -113,6 +113,21 @@
 #
 # Type for access to anon ftp
 #
-r_dir_file(ftpd_t,ftpd_anon_t)
+typeattribute ftpd_anon_t ftpd_file_ro_a;
 type ftpd_anon_rw_t, file_type, sysadmfile, customizable;
-create_dir_file(ftpd_t,ftpd_anon_rw_t)
+typeattribute ftpd_anon_rw_t ftpd_file_rw_a;
+
+# Any files which can be accessed by FTPd should be attach
+# the following attributes.
+#
+# ftpd_file_path_a represents directories which are the way to target files.
+# ftpd_file_ro_a represents Read-Only files. (e.g )
+# 
+attribute ftpd_file_path_a;
+attribute ftpd_file_ro_a;
+attribute ftpd_file_rw_a;
+
+allow ftpd_t ftpd_file_path_a : dir {getattr search};
+r_dir_file(ftpd_t, ftpd_file_ro_a);
+create_dir_file(ftpd_t, ftpd_file_rw_a);
+

@@ -89,3 +89,9 @@
 }
 ')
 
+# mysqld_connectable_a : a domain can connect mysqld via UNIX domain socket.
+attribute mysqld_connectable_a;
+can_unix_connect(mysqld_connectable_a, mysqld_t)
+allow mysqld_connectable_a {mysqld_var_run_t mysqld_db_t} : dir search;
+allow mysqld_connectable_a {mysqld_var_run_t mysqld_db_t} : sock_file rw_file_perms;
+

@@ -12,6 +12,8 @@
 #
 type postgresql_port_t, port_type;
 daemon_domain(postgresql)
+attribute postgresql_connectable_a;
+
 allow initrc_t postgresql_exec_t:lnk_file read;
 allow postgresql_t usr_t:file { getattr read };
 

@@ -113,13 +115,11 @@
 allow postgresql_t mail_spool_t:dir { search };
 lock_domain(postgresql)
 can_exec(postgresql_t, { shell_exec_t bin_t postgresql_exec_t ls_exec_t } )
-ifdef(`apache.te', `
-# 
-# Allow httpd to work with postgresql
-#
-allow httpd_t postgresql_tmp_t:sock_file rw_file_perms;
-can_unix_connect(httpd_t, postgresql_t)
-')
+
+# Allow postgresql_connectable_a to connect with postgresql via UNIX domain socket.
+allow postgresql_connectable_a tmp_t:dir search;
+allow postgresql_connectable_a postgresql_tmp_t:sock_file rw_file_perms;
+can_unix_connect(postgresql_connectable_a, postgresql_t)
 
 ifdef(`distro_gentoo', `
 # "su - postgres ..." is called from initrc_t

It is undesirable to implicitly declare symbols by using them. In fact with the loadable policy modules this becomes problematic since we have to keep track of all dependancy information. For example, since roles are declared implicitly when assigned a type it isn't possible to discern declaring a role from using a role. In a module using a role should always have a dependancy on the role but declaring it wouldn't. This is something we are trying to solve but adding more implictly declared symbols would only make it harder.

Joshua Brindle

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Hi Joshua, Thanks for your comments.

This patch does NOT mean implict declaration of attribute, ALL attributes require explict declaration when it's used in any TE statements such as ALLOW. I wrote this patch to resolve declaration ordering problem. The feature of problem is different from ROLE statement does not has explicitly declaration statement.

Currently, the declaration statement of attribute must be placed in forward of where it is used by any TE statements and TYPE declaration. After you apply this patch, the declaration statement of attribute must be placed in forward or backward of where it is used.
So, we still require ATTRIBUTE statments for explict declaration.

We can always discover an attribute declaration per effective attribute in policy sources.
(An attached attribute to type without declaration is ignored.) Does the declaration order in the policy source have a significant effect for Tresys's loadable policy module ?

Thanks,

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

That is a legacy of when attributes didn't need to be declared at all. Originally, attributes were implicitly declared upon first use in a type declaration. Later, we introduced and required explicit declaration of attributes. The disabled code was an easy way for people to quickly generate a list of attributes that lacked declarations when converting their policies. As everyone has since converted their policies to explicit declaration of attributes, it is no longer necessary.

That was intentional; it ensured that one could easily review all defined attribute with some inline documentation (comments) in one location. Of course, since that time, people have introduced some attribute definitions in other .te files, although primarily in macros. And some of the attribute definitions are domain-specific, e.g. the nscd_*_domain ones.

As an example, one could have a typo in an attribute name, which would no longer be caught by the compiler. In that case, you wouldn't get the
expected allow rules generated for the type. You note that this is
"harmless" because it simply means fewer permissions being allowed, but it could have unexpected results, e.g. lack of proper attributes on a domain or type could prevent the admin or init from killing a process or acting on a file (until the policy was fixed, of course).

I think that the reference policy is addressing this kind of issue by exporting explicit macro interfaces for every case where you need to export access to a type defined in one module to another module.

Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

Hi, Stephen. Thanks for your comments.

I didn't know why did the disabled section exist in checkpolicy. Excuse me, I have worked on SELinux for only one year and a few monthes.

OK. I'll think it as a policy-writing guideline that an attribute must be declared in application specific macros file when we want to declare the application specific attribtue.
# Maybe, it should be append to one of FAQs.

Indeed, a typo may cause lack of crucial permissions.
I can print a message to avoid it, but checkpolicy can't distinguish any typoes from intentionally ignored attributes.
So, please leave a prior patch for a while. orz

The attached macro is an example of explicit interfaces by macro. I think deep dependency relationship between two or more applications are so complication and easy to cause human error.
I think we should disestablish policies described directly between some applications, and define an interface by macros or attributes on intead.

Thanks,

KaiGai Kohei <[email protected]>

diff -rNU3 policy-1.23.17/domains/program/unused/apache.te policy-1.23.17.kg/domains/program/unused/apache.te
@@ -219,6 +219,11 @@
 # Creation of lock files for apache2
 lock_domain(httpd)
 
+# connect to PostgreSQL
+postgresql_connectable_domain(httpd_t)
+postgresql_connectable_domain(httpd_php_t)
+postgresql_connectable_domain(httpd_sys_script_t)
+
 # connect to mysql
 ifdef(`mysqld.te', `
 can_unix_connect(httpd_php_t, mysqld_t)

diff -rNU3 policy-1.23.17/domains/program/unused/postgresql.te policy-1.23.17.kg/domains/program/unused/postgresql.te
@@ -113,13 +113,6 @@
 allow postgresql_t mail_spool_t:dir { search };
 lock_domain(postgresql)
 can_exec(postgresql_t, { shell_exec_t bin_t postgresql_exec_t ls_exec_t } )
-ifdef(`apache.te', `
-# 
-# Allow httpd to work with postgresql
-#
-allow httpd_t postgresql_tmp_t:sock_file rw_file_perms;
-can_unix_connect(httpd_t, postgresql_t)
-')
 
 ifdef(`distro_gentoo', `
 # "su - postgres ..." is called from initrc_t

diff -rNU3 policy-1.23.17/macros/program/postgresql_macros.te policy-1.23.17.kg/macros/program/postgresql_macros.te
@@ -0,0 +1,16 @@
+# Macros for PostgreSQL
+
+#-----------------------------------------------------
+# An Interface for a domain can connect to PostgreSQL
+#                             (via UNIX domain socket)
+# usage: postgresql_connectable_domain(DOMAIN)
+
+define(`postgresql_connectable_domain',`
+ifdef(`postgresql.te',`
+allow $1 tmp_t:dir {search getattr};
+allow $1 postgresql_tmp_t:sock_file rw_file_perms;
+can_unix_connect($1, postgresql_t)
+
+',`') dnl The End Of postgresql.te
+') dnl The End Of postgresql_connectable_domain
+