Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MiqAction to annotate container images as non secure at openshift #7536

Merged
merged 1 commit into from
May 9, 2016

Conversation

moolitayer
Copy link

@moolitayer moolitayer commented Mar 27, 2016

Based on:

The metadata created by the MiqAction is meant to be used by Openshift to prevent tagged images from running.

@moolitayer moolitayer changed the title New action: tag container images as non secure at openshift [wip]New action: tag container images as non secure at openshift Mar 27, 2016
@moolitayer
Copy link
Author

@miq-bot add_label wip, providers/containers, control

@@ -59,5 +59,9 @@ def perform_metadata_scan(ost)
scan_via_miq_vm(miq_cnt_group, ost)
end

def prevent_from_starting
ext_management_system.annotate(self, {"manageiq.org/security" => "fail"}) # TODO: handle non openshift providers
Copy link
Author

Choose a reason for hiding this comment

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

@smarterclayton @deads2k This is the patch the annotates non secure images.

  • Which label should we set here?
  • Any other actions needed to prevent containers based on the image from starting on a master openshift deployed using openshift-ansible (I would like to test)

cc @simon3z

@moolitayer moolitayer force-pushed the annotate_image_action branch from 608cabc to f607ddf Compare March 27, 2016 16:58
@smarterclayton
Copy link

We probably need to sort out whether others would set this annotation (a
blackduck scanner, for example), and what policy levels there are.

There are descriptive annotations "this is true for this object" (such as "
security.manageiq.org/image-has-vulnerability" = "true" and "
security.manageiq.org/detected-vulnerabilities"="[...]"), and then there
are affirmative statements ("images.openshift.io/deny-execution" =
"true"). I suspect we need both.

What information do you have at this current time around the image and its
vulnerability stance? Do we have a clear "warn / block" separation based
on vulnerability level?

On Sun, Mar 27, 2016 at 1:02 PM, ManageIQ Bot [email protected]
wrote:

Checked commit moolitayer/manageiq@f607ddf
moolitayer@f607ddf
with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 1 offense detected

app/models/miq_action.rb


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#7536 (comment)

@deads2k
Copy link

deads2k commented Mar 28, 2016

I suspect we need both.

I agree. I'd like to have a human-readable string annotation for us to display along with the denial. I suspect that the marking process will have better information than we'll have.

@moolitayer moolitayer changed the title [wip]New action: tag container images as non secure at openshift [WIP]New action: tag container images as non secure at openshift Mar 28, 2016
@moolitayer
Copy link
Author

What information do you have at this current time around the image and its
vulnerability stance? Do we have a clear "warn / block" separation based
on vulnerability level?

This action is ad hock and can be executed in response to which ever condition/s the user chooses. Said condition might be OpenSCAP vulnerabilities detected OR it might be something entirely different e.g image name is X. I think the best we can do here is to name the failed ManageIQ policy, So:

"security.manageiq.org/failed-policies" = []
"security.manageiq.org/failed-policy-ids" = []
And as suggested
"images.openshift.io/deny-execution" = "true"

The deny-execution satisfy my immediate goal. Going forward I'm pretty sure I could define a parameterized action that will allow the user to select his annotations (and examples for those could be provided out of the box)

@moolitayer moolitayer changed the title [WIP]New action: tag container images as non secure at openshift [WIP] New action: tag container images as non secure at openshift Mar 29, 2016
@moolitayer moolitayer mentioned this pull request Apr 4, 2016
@miq-bot
Copy link
Member

miq-bot commented Apr 7, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@moolitayer moolitayer force-pushed the annotate_image_action branch from f607ddf to b9eac70 Compare April 20, 2016 15:21
@moolitayer moolitayer changed the title [WIP] New action: tag container images as non secure at openshift New action: tag container images as non secure at openshift Apr 20, 2016
@moolitayer
Copy link
Author

moolitayer commented Apr 20, 2016

@smarterclayton @deads2k Please review used annotations
@jrafanie Could you please review policy parts
fyi @simon3z

@moolitayer moolitayer force-pushed the annotate_image_action branch from b9eac70 to 3839459 Compare April 20, 2016 15:29
"image",
self,
"security.manageiq.org/failed-policy" => causing_policy,
"images.openshift.io/deny-execution" => "true"

Choose a reason for hiding this comment

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

This looks fine to me.

@chessbyte chessbyte added enhancement and removed wip labels Apr 21, 2016
@simon3z
Copy link
Contributor

simon3z commented Apr 29, 2016

@miq-bot add_label darga/yes

@moolitayer
Copy link
Author

moolitayer commented May 5, 2016

@simon3z I should have discovered this sooner but I don't think I can tag with our current inventory.

These are the openshift names I need to know in order to tag:

[root@oshift01 ~]# oc get images 
NAME                                                                      DOCKER REF
sha256:20098eb056620c0ceade04b4bec642fb89c8f3361e89e6ff4729b273f2c799ed   openshift/ruby-20-centos7@sha256:20098eb056620c0ceade04b4bec642fb89c8f3361e89e6ff4729b273f2c799ed
sha256:592872229633f5b906301b1880aeb45272ef83a838794a9c2e53ce07b2a5f3f0   172.30.107.74:5000/test/origin-ruby-sample@sha256:592872229633f5b906301b1880aeb45272ef83a838794a9c2e53ce07b2a5f3f0
sha256:78380eec5f83213b7ef0c0d37a5f285ebb7632ca2c7390b9bd6c07625019cf7e   172.30.107.74:5000/test/origin-ruby-sample@sha256:78380eec5f83213b7ef0c0d37a5f285ebb7632ca2c7390b9bd6c07625019cf7e
sha256:af4c1abc8797bc2df3bfdc92b3ae5fd01984fd4590ffd4899520777f9de2d674   172.30.107.74:5000/test/origin-ruby-sample@sha256:af4c1abc8797bc2df3bfdc92b3ae5fd01984fd4590ffd4899520777f9de2d674
sha256:f334f35cdafe80368b83b53144e2f9a028b2f1170b25f612ce2ad7de890abcba   centos/ruby-22-centos7@sha256:f334f35cdafe80368b83b53144e2f9a028b2f1170b25f612ce2ad7de890abcba

This is the information we look at to create our ManageIQ image inventory:

[root@oshift01 ~]# oc describe pod docker-registry-1-a8bhm|grep -i image
Image(s):           openshift/origin-docker-registry:v1.0.8
    Image:      openshift/origin-docker-registry:v1.0.8
    Image ID:       docker://3629a651e6c11d7435937bdf41da11cf87863c03f2587fa788cf5cbfe8a11b9a

fyi @zeari thoughts?

@simon3z
Copy link
Contributor

simon3z commented May 5, 2016

@simon3z I should have discovered this sooner but I don't think I can tag with our current inventory.

Indeed.

Can't you lookup by "DOCKER REF" or "Image ID"? (cc @deads2k )
If not you have to load them all and find the image yourself (really expensive).

@moolitayer
Copy link
Author

moolitayer commented May 5, 2016

@simon3z I was wrong, we have digest since #4234 which is what I need. Seems to be missing most of the time (even after taking into account that most ContainerImages do not link to a Openshift image)

@moolitayer moolitayer force-pushed the annotate_image_action branch from 3839459 to 6669cad Compare May 5, 2016 12:14
@moolitayer
Copy link
Author

Added handling for images with no digest and usual testing for entity type.

Tested:

ContainerImage.find(8).prevent_from_starting("has openscap failures")

Annotations returned by Openshift:

"annotations": {
"images.openshift.io/deny-execution": "true",
"openshift.io/image.managed": "true",
"security.manageiq.org/failed-policy": "has openscap failures"
}

Another common use case would lead to a guid name (policies can have a descriptive name or a guid name)
... "security.manageiq.org/failed-policy": "2c11dc5a-c080-483e-9ee0-c9ebf6921371"
}

@simon3z Please review

@moolitayer moolitayer changed the title New action: tag container images as non secure at openshift Add MiqAction to annotate container images as non secure at openshift May 5, 2016
@moolitayer
Copy link
Author

ping @simon3z

return
end

if inputs[:synchronous]
Copy link
Contributor

Choose a reason for hiding this comment

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

@moolitayer @Fryguy @gmcculloug @blomquisg can anyone (after this patch) take the refactoring of the repeating:

if inputs[:synchronous]
  log_do_something
  do_something
else
  log_queuing_do_something
  queue(do_something)
end

It seems pretty common in this module.

Copy link
Author

Choose a reason for hiding this comment

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

Sure I'll give it a try @simon3z

Copy link
Author

Choose a reason for hiding this comment

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

@simon3z
Copy link
Contributor

simon3z commented May 6, 2016

@moolitayer: few minor naming changes.

@moolitayer moolitayer force-pushed the annotate_image_action branch 2 times, most recently from ae4c0ec to 21ee2a0 Compare May 6, 2016 17:19
end

if inputs[:synchronous]
MiqPolicy.logger.info("MIQ(#{__method__}): Now executing [#{action.description}] for event "\
Copy link
Author

Choose a reason for hiding this comment

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

@gmcculloug Noticed it's Now executing or Queueing in most calls

@jrafanie
Copy link
Member

jrafanie commented May 6, 2016

Looks good @moolitayer. I had minor comments. Once they're fixed and @simon3z gives the 👍 , I'm good with this.

Note, I don't think we have an out of box deny/stop a vm thing but it seems like they'd have similar mechanics.

@moolitayer moolitayer force-pushed the annotate_image_action branch from 21ee2a0 to 8c69985 Compare May 6, 2016 19:24
@moolitayer
Copy link
Author

Address comment

@moolitayer
Copy link
Author

@simon3z please reivew

return
end

unless rec.try(:ext_management_system).kind_of?(ManageIQ::Providers::Openshift::ContainerManagerMixin)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like something that should be refactored in the future. We need to be able to define actions and action handler methods for each manager type and provider.

/cc @durandom

@simon3z
Copy link
Contributor

simon3z commented May 9, 2016

LGTM 👍 cc @blomquisg @jrafanie @chessbyte

@@ -82,5 +82,14 @@ def has_compliance_policies?
!plist.blank?
end

def annotate_deny_execution(causing_policy)
Copy link
Member

@blomquisg blomquisg May 9, 2016

Choose a reason for hiding this comment

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

Based on this error message, this call is only available for OpenShift images. This should really be implemented on an OpenShift implementation of ContainerImage.

But, since ContainerImage does not yet support STI, add an error handler here in case someone calls this method directly. Stopping calls via MiqAction seems insufficient to protect this method.

Copy link
Contributor

Choose a reason for hiding this comment

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

But, since ContainerImage does not yet support STI, add an error handler here in case someone calls this method directly. Stopping calls via MiqAction seems insufficient to protect this method.

@moolitayer can you take care of that? Thanks.

Copy link
Author

Choose a reason for hiding this comment

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

Sure

@moolitayer moolitayer force-pushed the annotate_image_action branch from 8c69985 to 97d50db Compare May 9, 2016 14:24
@miq-bot
Copy link
Member

miq-bot commented May 9, 2016

Checked commit moolitayer@97d50db with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 2 offenses detected

app/models/miq_action.rb

@chessbyte chessbyte closed this May 9, 2016
@chessbyte chessbyte reopened this May 9, 2016
@moolitayer moolitayer closed this May 9, 2016
@moolitayer moolitayer reopened this May 9, 2016
@chessbyte chessbyte merged commit 3b300aa into ManageIQ:master May 9, 2016
@chessbyte chessbyte added this to the Sprint 40 Ending May 9, 2016 milestone May 9, 2016
chessbyte added a commit that referenced this pull request May 9, 2016
Add MiqAction to annotate container images as non secure at openshift
(cherry picked from commit 3b300aa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants