Skip to content

OXT-1696: refpolicy: General cleaning of the recipe and patches.#1270

Merged
jean-edouard merged 10 commits intoOpenXT:masterfrom
eric-ch:refpolicy-cleanup-0
Oct 11, 2019
Merged

OXT-1696: refpolicy: General cleaning of the recipe and patches.#1270
jean-edouard merged 10 commits intoOpenXT:masterfrom
eric-ch:refpolicy-cleanup-0

Conversation

@eric-ch
Copy link
Contributor

@eric-ch eric-ch commented Oct 7, 2019

While working on #1259 and upgrading the OE layer. Some changes in the refpolicy recipe from xenclient-oe seems to be shared.

  • Add a step to provide configuration files (modules.conf and booleans.conf). This lets derived policy configure, from the layer, which module to include in the built policy and switch booleans.
  • Remove deprecated/unused modules from the patch-queue: vgmch, txtstat, get-edid.
  • Fix invalid patch documentation.
  • Remove patches one the build system.
  • Refresh the patch-queue following https://openxt.atlassian.net/wiki/spaces/DC/pages/20905986/Dealing+with+quilt to avoid further noise.

@stacktrust
Copy link

Can we tag this PR with the ticket number for OE uprev and/or BATS improvements?

@eric-ch eric-ch force-pushed the refpolicy-cleanup-0 branch from b45f267 to c7454be Compare October 7, 2019 21:49
@eric-ch eric-ch changed the title refpolicy: General cleaning of the recipe and patches. OXT-1696: refpolicy: General cleaning of the recipe and patches. Oct 7, 2019
Copy link
Contributor

@jandryuk jandryuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. +370, -1321 is a nice diff stat.

# stack smashing protection. All domains will
# be allowed to read from /dev/urandom.
#
global_ssp = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want global_ssp = true. That's what policy.booleans.diff was setting.

Copy link
Contributor Author

@eric-ch eric-ch Oct 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I commented on that in the commit message, I'll amend that.

global_ssp documentation says:

This should be enabled when all programs are compiled with ProPolice/SSP stack smashing protection.

It does not appear to be the case in our builds (could not find -fstack-protector*), which matches the absence of new AVCs with that Boolean set to false.

That being said, giving read permissions to /dev/urandom to all domains should not be a huge risk either. So I can switch it back and have another PR to have stack protection enabled for OpenXT machines.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the 64bit conversion, there were a bunch of patches proposed ad-hoc adding /dev/random & /dev/urandom permissions. I submitted f52432c to avoid all the manual additions. I assumed the 64bit conversion was flipping on something that was requiring the use of urandom

Looking back at #1101, it was quark which was reading /dev/urandom. That was probably indirectly through OpenSSL. And quark was never included. I thought there was another one, but I haven't looked hard.

If the policy works with global_ssp = false then your change is fine.

(... should really enable -fstack-protector....)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Yes, we should) Fortunately this is made easy by OE. I'll open a PR after running a test build.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a test build running as well. :)

@jean-edouard
Copy link
Member

@eric-ch eric-ch force-pushed the refpolicy-cleanup-0 branch from c7454be to 5010a98 Compare October 8, 2019 20:34
@eric-ch
Copy link
Contributor Author

eric-ch commented Oct 8, 2019

V2:

  • Removed no-clobber option from cp in do_srctree_copy
  • Moved the global_ssp = false change to a separate commit until SSP is enabled for OpenXT image. This last part triggers a bunch of compilation failures and requires more work, it will be addressed in a different changeset (OXT-1697).

@eric-ch eric-ch force-pushed the refpolicy-cleanup-0 branch from 5010a98 to 8b67651 Compare October 8, 2019 20:43
Eric Chanudet added 10 commits October 10, 2019 16:34
No functional change.
The patches mentioned no longer exist.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
booleans.conf and modules.conf are updated by the build-system during
make conf. This is done in do_compile just before make policy by the
meta-selinux policy.

Since OpenXT adds policy modules for its components, keep the split
configuration file (upstream vs openxt) using the same prefix and
iterate over the configuration files that can be provided by the layer.

The configuration files created are modules.conf and booleans.conf.

Only copy the 'policy/modules' directory from the layer to avoid
accidental overwrites.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
The upstream policy passes POLICY_* variables to EXTRA_OEMAKE to
configure the build.conf of the policy.
The current patch does not completely match the policy configuration
passed in the recipe either.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
vgmch is a deprecated feature of Surfman from XenClient that is no
longer shipped or available in OpenXT.
Remove the associated SELinux module.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
get-edid and igfx_edid are two tools from XenClient retired in OpenXT.
Remove the associated policy module.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
HTML documentation is not installed.
Changing tc_sbindir is not required, the upstream recipe seems to deal
fine without this patch.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
Documentation of the refpolicy generates an xml file. <tag> will create
a new XML tag that will lack any closure, which in turn fails to
generate a valid policy.xml file.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
This policy module duplicates the upstream tboot.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
Apply standard:
https://openxt.atlassian.net/wiki/spaces/DC/pages/20905986/Dealing+with+quilt

No functional change.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1696
SSP does not appear to be enabled by default, so global_ssp can be
switched to false until it is.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1697
@eric-ch eric-ch force-pushed the refpolicy-cleanup-0 branch from 8b67651 to 3f22371 Compare October 10, 2019 22:44
@eric-ch
Copy link
Contributor Author

eric-ch commented Oct 10, 2019

V3:

  • Rebase on top of master.

@jean-edouard
Copy link
Member

Copy link
Member

@jean-edouard jean-edouard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging soon.

@jean-edouard jean-edouard merged commit c567beb into OpenXT:master Oct 11, 2019
@eric-ch eric-ch deleted the refpolicy-cleanup-0 branch February 21, 2020 23:28
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

Successfully merging this pull request may close these issues.

4 participants