dburgener/cascade

A high level language for SELinux policy

cascade's Issues

Require annotation on ambiguous bindings

From a commit message in #205:

Currently this assumes only one valid match. It's possible that we
could have multiple valid matches. For example:

resource read {}
resource write {}

let foo = [ read write ];

In this case, we don't know if foo is supposed to be a list of
permissions or a list of resources until we start using it. As a future
enhancement, we should detect that there are ambiguous possibilities and
prompt the developer to annotate the binding like so:

let foo<perm> = [ read write ];

Redefining system's base declarations.

Hello,

The resulting .CIL of a .CAS file has the definition all of the system's base classes, such as: "alg_socket", "anon_inode".
This monolithic approach is problematic because the resulting .CIL require manual editing before being insert in the SE's Binary Tree of policies.

I understand why this approach is desired, but shouldn't be any compilation option to disable these embeddings?

Thank you for your support,
Cristian

Create TryFrom<&CascadeString> for FileType

There was code implemented in this PR that has more broad application.

To quote the original comment:
"This seems like something that can/should be resuable. Can you implement a "TryFrom<&CascadeString> for FileType" to wrap the parse() call and add the error message with the range? Then that can get called here and in call_to_fc_rules()."

Error messages about invalid permissions refer to "type"

Internally, that makes sense, but from an SELinux policy developer perspective, that communicates the SELinux version of a "type". We should resolve "No such type" errors so that we say things like "No such permission", "No such object class" etc, based on what we're validating it against.

Unable to start in enforcing

Hello! I am still getting my feet wet with SELinux, and I noticed that when I tried to follow the steps for the newly added 'full system' policy, my Fedora 36 VM will just... hang on start up? As soon as I select the OS I want to boot into, the system fails to proceed any farther, with no output on the screen. I moved the policy in place on the system, updated my config, rebooted in permissive, ran restorecon, then rebooted in enforcing. Any and all help on how to troubleshoot this problem would be greatly appreciated. If any more details are required, I can surely provide them. Thanks!

Late annotations are being dropped

As seen in the following PR:
#192
We added the concept of early and late annotations. Processing the late annotations added a 6% increase in our benchmark times.

Currently we do not leverage any late annotations, so dropping them has no negative functionality impact at this time. Since we don't want to take the performance hit, we will continue with them being dropped for the time being.

We should try to resolve the performance issue before we start processing the late annotations if possible.

Downgrade to warnings unknown types in arguments as well

We have function calls on them downgraded, but not in arguments.

Add the ability for InternalErrors to take a help string of some sort

Often the stack trace is all that is needed when debugging an InternalError, but sometimes there is extra state information available at the time of the error that could help in debugging. It would be nice to be able to pass an optional message to the InternalError handler to be displayed along with the stack trace.

FunctionInfo::symbol_to_caller_symbol() ignores permission lists

I don't think there's a good reason for it, so it should probably handle them. This could cause breakage with function arguments of type [perm] or perm.

Support genhomedircon path substitutions

Two related issues here:

1. Cascade currently infers that a string is a path if it contains "/". That means that the literal "HOME_ROOT" is not a path. This of course may change if we introduce explicitly typed strings in the future.
2. Are there any other features Cascade should provide WRT genhomedircon functionality?

Build failure based on dependencies

lalrpop issue: lalrpop/lalrpop#750

regex issue: rust-lang/regex#982

It looks like an update to the regex package changed its features in a way that's legal from a cargo standpoint, but exposed incorrect dependency handling in other crates including regex, such as lalrpop.

I'm not sure there's a ton that can be ton directly in Cascade. Possible directly including regex 1.7.3 in our Cargo.toml as a pretty gross workaround would force us to use the compatible version. But now that lalrpop is maintained, it's probably cleaner to get a fix into lalrpop and have them make a new release.

Analyze and improve pre-validation steps

The pre-validation steps introduced here: #128
are not very optimized and can be improved.

Currently we have what could potentially be a large clone which sizable policy (which ref policy 3 will become) in a while loop that may take several iterations to resolve. I initially did this to avoid mutability & ownership issues. This can be seen in search_for_recursion

I assume there are also general improvements that can go into this function as well.

Adding annotations on nested extensions doesn't work

extend_type_map() only adds annotations if it also creates a new type. Probably the logic should be that the new type code is in the "if not extension" block, but the annotation code is unconditional.

Cascade is reporting no type errors on the full type.function

error: No such type
    ┌─ /home/pebenito/refpolicy3/policy/system/userdomain.cas:731:9
    │
731 │         samhain_t.domtrans(source);
    │         ^^^^^^^^^^^^^^^^^^

Only samhain_t should be indicated here

Associated resources are not properly converted if they are this.*

In the following example:

virtual resource tmp {
	@associated_call
	fn associated_call_from_tmp(domain source) {
		allow(source, tmp, file, [read]);
	}

	fn not_an_associated_call(domain source) {
		allow(source, tmp, file, [write]);
	}
}

resource qwe {
   fn some_func(domain source, resource some_resource) {
       allow(source, some_resource, file, read);
   }
}

@associate([tmp])
domain cvb {
    qwe.some_func(this, this.tmp);
}

The following relevant cil will be created:

(type cvb-tmp)
(roletype object_r cvb-tmp)
(typeattributeset tmp (cvb-tmp))
(typeattributeset resource (cvb-tmp))
(macro cvb-tmp-associated_call_from_tmp ((type this) (type source)) (allow source tmp (file (read))))
(macro cvb-tmp-not_an_associated_call ((type this) (type source)) (allow source tmp (file (write))))
(call cvb-tmp-associated_call_from_tmp (cvb-tmp cvb))
(call qwe-some_func (qwe cvb cvb.tmp))

As you can see most of the cvb.tmp are correctly converted to cvb-tmp with the exception of this.tmp where this is cvb. That remains cvb.tmp.

collect_aliases() ignores duplicates

The code looks like it just discards the second alias. If there is such a conflict, we should report an error.

Additionally, we should add functionality for users to have conditional alias switching to support use cases like subbing out init systems.

Display warnings along with errors on unsuccessful complilations

Currently Cascade only displays warnings if there were no errors. We should display warnings along with errors when a build fails.

Check declarations against reserved keywords

Generally, the compiler currently assumes that declaration names and reserved keywords don't conflict. We should check this and return helpful error messages when users try to declare anything named the same as a reserved keyword.

Can object classes and permissions conditionally resolve based on context?

Two issues:

• If a developer names a type the same thing as an object class or permission, they conflict right now
• Object classes and permissions can conflict on names (notably "bpf" is both an object class and a permission in the capability2 and cap2_userns classes)

When we're resolving an identified to a type, we should know what sort of type we want. Currently, we map it to a type first, and then compare and see if that matches the type we're mapping. Possibly, when we resolve the identifier to its typeinfo, we can pass in information about what we're expecting, and use that information to decide what TypeInfo to resolve to in the event of a conflict.

Member function not being found

error: No such member function
┌─ /home/pebenito/refpolicy3/policy/system/system_api.cas:636:25
│
636 │ policy_config_t.dontaudit_read(source); selinux_config_t.dontaudit_list(source); etc_t.dontaudit_list(source);
│ ^^^^^^^^^^^^^^ policy_config_t does not define a function named dontaudit_read

policy_config_t is an alias to selinux_policy_t, and should have dontaudit_read() via common_file.

Missing passwd object class definition in built in object classes

Propagate collection names and aliases through compilation

Possible interaction of two bugs, needs more investigation.

`system` is both a Cascade keyword and an object class

A rule like:

allow(this, foo, system, [start]);

Fails to compile currently, because system is parsed as the reserved keyword rather than some arbitrary string that will later be consumed as a valid object class.

Expand casted macros that include this.*

If we have a function like:

fn read_boo_this_tmp(domain source) {
    allow(source, this.tmp, file, [read]);
}

And call jkl<boo>.read_boo_this_tmp(this) we want this.tmp to resolve to jkl-tmp, but currently the only macro that is made for read_boot_this_tmp is (macro boo-read_boo_this_tmp ((type this) (type source)) (allow source boo-tmp (file (read)))).

Thus we will need to create a new macro for each casted call to have the correct this.* resolution. In the above case:
(macro jkl-read_boo_this_tmp ((type this) (type source)) (allow source jkl-tmp (file (read))))

Some more wordage from dburgener:

The macros come from the FunctionInfos. Each FunctionInfo creates exactly one macro from that fi. So in order to create a new macro, you'd want to make a copy of the original FunctionInfo, with changes as needed and then add it into the FunctionMap, then everything else proceeds from there. Ideally we'd make the name of the macro be something with reserved characters so it can't possibly be explicitly referred to in the source, but also something at least vaguely sensible to someone interacting at the CIL level, for cross-language compatibility purposes. (Although, we're definitely going to need some sort of shim for that aspect in the long run I think. There's probably not a solution to the cross-language aspect that's 100% as clean as we'd like)

This should be done as part of 0.1

get_name_or_string() returning CascadeString with Range of None

We believe that the code here is erroneously stripping the range of the cascade string.

`:` isn't lexed

file_context("/dev/log", socket, system_u:object_r:devlog_t:mls_systemhigh);

Reports "invalid" character on the ":". At a quick glance, it looks like ":" is handled in context parsing, but not in the lexing. So perhaps just including this in the lexer will pass it on to the rest which is unit tested and should work. On the other hand, maybe we want to parse contexts as contexts? Needs a more comprehensive investigation.

Context should provide a one stop shop for symbol resolution

symbol_in_context(), by design only resolves bound symbols. There's also TypeInfo;:typeinfo_from_string(), which turns a string into a TI, while performing "this" resolution, but not bound symbol lookup.

As a result, various call-sites for these seem to augment with various mixes of lookups.

We should create a new function in Context that does full resolution, including "this", bound symbols, aliases, and lookups in the global maps (both the Type and Function maps). Once that exists, caller sites can be converted to use it one by one.

This is too big a code change for pre-0.1, but something we should consider for the next major release after 0.1

Drop default SIDs

Currently if a policy doesn't specify any initial_context() calls, there are default types (kernel_sid, security_sid and unlabled_sid) generated for the mandatory sids, and all the sid configuration is done. This could potentially be opt-in behavior, but doing it implicitly isn't the right answer, since the user won't even know about these types to work with them. The blocker on removing them now is that the tests rely on this behavior. So we'll need to augment the tests to add a file of initial_context() calls in all tests in order to avoid breaking nearly all the tests while removing the default initial_contexts()

Teach CascadeStrings about files

Now that CompileError uses SimpleFiles under the hood, we can just pass around file_ids instead of file references, which makes this feasible. It will simplify error handling, and eliminate the class of errors where a file and range can be mismatched.

Explicit duplicated types in inheritance should error

This currently compiles fine:

virtual resource foo {}

virtual resource bar {}

resource baz inherits foo, foo, bar {}

domain requires_allow {
        allow(this, baz, file, read);
}

It should error and tell the user that "foo" was listed twice when inheriting baz.

On the other hand, something like this:

virtual resource foo {}

virtual resource bar inherits foo {}

virtual resource baz inherits foo {}

resource qux inherits bar, baz {}

domain requires_allow {
        allow(this, qux, file, read);
}

Compiles fine and should continue to do so, because the duplication isn't explicit. Since qux inherits bar and baz which each inherit foo, that's fine, and the compiler should internally dedup and only inherit foo once.

The second case is working correctly today, but we want to ensure that it continues working when we cause the first case to error.

@associated_call force Virtual Type

Hello,

The @associate_call expects a virtual function. This condition is not well stipulated in the documentation.

The problem is generated by the method: apply_associate_annotations::inherit_annotations
Caused by (as far as I saw): A new type is generated (mock.mock_conf in the example I have presented), which tries to inherit the mock_conf type. Since mock_conf is not virtual, an error is thrown.

Code to reproduce the problem:

• the following .CAS code could not be compiled:

resource mock_conf  {
	@associated_call
	fn read(domain source) {
		allow(source, this, file, [ read open getattr ]);
	}
	file_context("/home/csandu/Documents", [file dir], this);
}

@associate([mock_conf])
domain mock {
	mock_conf.read(this);
}

Is this the expected behavior? If so, why one should "expect" virtual when using @associated_call(s)?

Thank you for your support,
Cristian Sandu

Error messages for object class permissions with a "2" are reported against the "2" version of the class

If you do something like:

allow(this, this: resource, capability, foo);

Then you will get a message that "foo" is not in the "capability2" object class. The expectation is that the error message should refer to the "capability" object class instead.

Unify annotation handling

Currently, types, functions and modules all can be annotated and have their own mechanisms to convert the raw Annotation data from the parser into AnnotationInfo structs for internal use. This can be unified into one general "get_annotations()" function, and then smaller wrappers for types, functions and modules that verify that the annotation listed is valid in that context.

Can't call functions on arguments

virtual domain parent {
    fn some_func(domain source) {
        allow(source, this, process, signal);
    }
}

domain child inherits parent {}

domain other {
    fn call_signal(parent to_call) {
        to_call.some_func(this);
    }
}

I think policy like the above isn't working now and should

Context parsing should do its own error handling

Currently Context::try_from() (both the String and &str variants) return () on error and trust the level above to write a good error message. This discards information about what failed in parsing. try_from() should return something more helpful, and the level above can decide what it wants to do. Perhaps rather than a full error, try_from() might return just a help string. Or maybe just a normal CompileError works. More analysis should be done on callers.

Duplicate paths in file_context()s are silently dropped

It seems like this should be an error instead.

#168 introduces clone() of rules

#168

Because of the way lifetimes are handled in rules, introducing the clone() is the expedient way to handle SID extraction, but isn't ideal from a performance standpoint. Ideally, lifetimes inside ValidatedStatements should be cleaned up, but to avoid this clone() and to enable returning the various maps from get_reduced_infos() so we can call generate_sexp() outside and refactor the library API.

Unexpected order dependencies

Generally speaking policy should not be order dependent. In the current implementation, let bindings, module definitions and system definitions may be order dependent. If they are split across multiple files, the order they are processed in is compiler defined, which makes this problem worse.

Cascade should preprocess the definitions to know what bindings and definitions are defined before processing their definitions to remove this dependency.