Merge lp:~jdstrand/click-reviewers-tools/click-reviewers-tools.modern-yaml into lp:click-reviewers-tools

[Jamie Strandboge]

  * add security yaml checks
  * cr_lint.py: don't allow same key in 'binaries' and 'services'

[Daniel Holbach (dholbach)]

+1. Pinged Martin Albisetti to see if somebody on their team can give another review.

review: Approve

[Ricardo Kirkner (ricardokirkner)]

I didn't see anything obvious that would prevent from landing this branch, but I think it would be very beneficial to start spending some time improving the readability of this code base. I'd be happy to work on it provided Martin can approve of the time spent to do so.

[lp:~jdstrand/click-reviewers-tools/click-reviewers-tools.modern-yaml]

449. By Jamie Strandboge on 2015-05-18

merge with trunk

450. By Jamie Strandboge on 2015-05-18

clickreviews/cr_security.py: implement feedback from MP review
- remove commented out code
- use more descriptive variable name

451. By Jamie Strandboge on 2015-05-18

update comment in check_security_caps()

452. By Jamie Strandboge on 2015-05-18

clickreviews/tests/test_cr_lint.py: update to include test on the error
string too

[Jamie Strandboge (jdstrand)]

Thanks for the reviews, I've updated the branch based on Ricardo's feedback.

=== modified file 'clickreviews/cr_framework.py'
--- clickreviews/cr_framework.py	2015-04-03 20:51:02 +0000
+++ clickreviews/cr_framework.py	2015-05-18 21:07:49 +0000
@@ -66,12 +66,12 @@
     def _extract_framework_policy(self):
         '''Get framework policy files'''
         policy_dict = dict()
-        unknown_dict = dict()
+        unknown = []
         fpdir = os.path.join(self.unpack_dir, "meta", "framework-policy")
         for i in glob.glob("%s/*" % fpdir):
             rel_i = os.path.basename(i)
             if not os.path.isdir(i) or rel_i not in self.framework_policy_dirs:
-                unknown_dict.append(os.path.relpath(i, self.unpack_dir))
+                unknown.append(os.path.relpath(i, self.unpack_dir))
                 continue
 
             policy_dict[rel_i] = dict()
@@ -79,21 +79,20 @@
                 rel_j = os.path.basename(j)
                 if not os.path.isdir(j) or \
                    rel_j not in self.framework_policy_subdirs:
-                    unknown_dict.append(os.path.relpath(j, self.unpack_dir))
+                    unknown.append(os.path.relpath(j, self.unpack_dir))
                     continue
 
                 policy_dict[rel_i][rel_j] = dict()
                 for k in glob.glob("%s/*" % j):
                     rel_k = os.path.basename(k)
                     if not os.path.isfile(k):
-                        unknown_dict.append(os.path.relpath(k,
-                                            self.unpack_dir))
+                        unknown.append(os.path.relpath(k, self.unpack_dir))
                         continue
 
                     fh = open_file_read(k)
                     policy_dict[rel_i][rel_j][rel_k] = fh.read()
                     fh.close()
-        return (policy_dict, unknown_dict)
+        return (policy_dict, unknown)
 
     def _has_framework_in_metadir(self):
         '''Check if snap has meta/<name>.framework'''
@@ -223,7 +222,7 @@
                         if j not in self.framework_policy[other] or \
                            k not in self.framework_policy[other][j]:
                             t = 'error'
-                            s = "Could not find mathcing '%s/%s/%s'" % (other,
+                            s = "Could not find matching '%s/%s/%s'" % (other,
                                                                         j, k)
                         self._add_result(t, n, s)
 

=== modified file 'clickreviews/cr_lint.py'
--- clickreviews/cr_lint.py	2015-05-01 15:18:50 +0000
+++ clickreviews/cr_lint.py	2015-05-18 21:07:49 +0000
@@ -1055,3 +1055,35 @@
             t = 'warn'
             s = "meta/readme.md is too short"
         self._add_result(t, n, s)
+
+    def check_snappy_services_and_binaries(self):
+        '''Services and binaries should not overlap'''
+        if not self.is_snap:
+            return
+        for exe_t in ['binaries', 'services']:
+            if exe_t not in self.pkg_yaml:
+                break
+            if exe_t == 'binaries':
+                other_exe_t = 'services'
+            else:
+                other_exe_t = 'binaries'
+
+            if other_exe_t not in self.pkg_yaml:
+                break
+
+            for a in self.pkg_yaml[exe_t]:
+                if 'name' not in a:
+                    continue
+                # handle 'exec' in binaries
+                app = os.path.basename(a['name'])
+
+                t = 'info'
+                n = 'snappy_%s_in_%s' % (app, other_exe_t)
+                s = 'OK'
+                for other_a in self.pkg_yaml[other_exe_t]:
+                    other_app = os.path.basename(other_a['name'])
+                    if app == other_app:
+                        t = 'error'
+                        s = "'%s' in both 'services' and 'binaries'" % app
+                        break
+                self._add_result(t, n, s)

=== modified file 'clickreviews/cr_security.py'
--- clickreviews/cr_security.py	2015-05-04 18:29:04 +0000
+++ clickreviews/cr_security.py	2015-05-18 21:07:49 +0000
@@ -19,6 +19,7 @@
 from clickreviews.cr_common import ClickReview, error, open_file_read
 import clickreviews.cr_common as cr_common
 import clickreviews.apparmor_policy as apparmor_policy
+import copy
 import json
 import os
 
@@ -68,8 +69,7 @@
                                'comment',
                                'copyright',
                                'name']
-        self.required_fields = ['policy_groups',
-                                'policy_version']
+        self.required_fields = ['policy_version']
         self.redflag_fields = ['abstractions',
                                'binary',
                                'read_path',
@@ -147,6 +147,9 @@
                 self._extract_security_profile(app)
             self.security_apps_profiles.append(app)
 
+        # snappy
+        self.sec_skipped_types = ['oem']  # these don't need security items
+
     def _override_framework_policies(self, overrides):
         # override major framework policies
         self.major_framework_policy.update(overrides)
@@ -464,7 +467,27 @@
                 continue
             self._add_result(t, n, s)
 
-            if m['template'] not in self._get_templates(vendor, version):
+            found = False
+            if m['template'] in self._get_templates(vendor, version):
+                found = True
+            elif self.is_snap:
+                frameworks = []
+                if 'framework' in self.pkg_yaml:
+                    frameworks = [x.strip() for x in
+                                  self.pkg_yaml['framework'].split(',')]
+                elif 'frameworks' in self.pkg_yaml:
+                    frameworks = self.pkg_yaml['frameworks']
+                elif 'type' in self.pkg_yaml and \
+                     self.pkg_yaml['type'] == 'framework':
+                    # frameworks may reference their own policy groups
+                    frameworks.append(self.pkg_yaml['name'])
+
+                for f in frameworks:
+                    if m['template'].startswith("%s_" % f):
+                        found = True
+                        break
+
+            if not found:
                 t = 'error'
                 s = "specified unsupported template '%s'" % m['template']
 
@@ -590,7 +613,7 @@
             n = 'policy_groups_exists_%s (%s)' % (app, f)
             if 'policy_groups' not in m:
                 # If template not specified, we just use the default
-                self._add_result('warn', n, 'no policy groups specified')
+                self._add_result('info', n, 'no policy groups specified')
                 continue
             elif 'policy_version' not in m:
                 self._add_result('error', n,
@@ -625,6 +648,18 @@
                     s = 'duplicate policy groups found: %s' % ", ".join(tmp)
             self._add_result(t, n, s)
 
+            frameworks = []
+            if self.is_snap:
+                if 'framework' in self.pkg_yaml:
+                    frameworks = [x.strip() for x in
+                                  self.pkg_yaml['framework'].split(',')]
+                elif 'frameworks' in self.pkg_yaml:
+                    frameworks = self.pkg_yaml['frameworks']
+                elif 'type' in self.pkg_yaml and \
+                     self.pkg_yaml['type'] == 'framework':
+                    # frameworks may reference their own policy groups
+                    frameworks.append(self.pkg_yaml['name'])
+
             # If we got here, we can see if valid policy groups were specified
             for i in m['policy_groups']:
                 t = 'info'
@@ -640,10 +675,20 @@
                     continue
 
                 found = False
+                framework_found = False
                 for j in policy_groups:
                     if i == os.path.basename(j):
                         found = True
                         break
+                    else:
+                        for f in frameworks:
+                            if i.startswith("%s_" % f):
+                                framework_found = True
+                                break
+                        if framework_found:
+                            found = True
+                            break
+
                 if not found:
                     t = 'error'
                     s = "unsupported policy_group '%s'" % i
@@ -656,7 +701,11 @@
                     l = None
                     manual_review = False
 
-                    aa_type = self._get_policy_group_type(vendor, version, i)
+                    if framework_found:
+                        aa_type = 'framework'
+                    else:
+                        aa_type = self._get_policy_group_type(vendor, version,
+                                                              i)
                     if i == "debug":
                         t = 'error'
                         s = "(REJECT) %s policy group " % aa_type + \
@@ -668,9 +717,11 @@
                         if i == "debug":
                             l = 'http://askubuntu.com/a/562123/94326'
                         manual_review = True
+                    elif aa_type == 'framework':
+                        s = "OK (matches '%s' framework)" % i.split('_')[0]
                     elif aa_type != "common":
                         t = 'error'
-                        s = "policy group '%s' has" % i + \
+                        s = "policy group '%s' has " % i + \
                             "unknown type '%s'" % (aa_type)
                     self._add_result(t, n, s, l, manual_review=manual_review)
 
@@ -720,7 +771,7 @@
             (f, m) = self._get_security_manifest(app)
 
             t = 'info'
-            n = 'ignored_fields (%s)' % f
+            n = 'required_fields (%s)' % f
             s = "OK"
             not_found = []
             for i in self.required_fields:
@@ -751,3 +802,409 @@
                                      "could not find '%s' in profile" % v)
                     continue
                 self._add_result(t, n, s)
+
+    # This will be really nice to get rid of when the click compat manifest
+    # is gone
+    def _compare_security_yamls(self, yaml, click_m):
+        '''Compare two security yamls'''
+        def find_match(name, key, value, my_dict):
+            if 'name' in my_dict and my_dict['name'] == name and \
+               key in my_dict and my_dict[key] == value:
+                return True
+            return False
+
+        for first in [yaml, click_m]:
+            if first == yaml:
+                second = click_m
+                second_m = "click-manifest"
+                first_m = "package.yaml"
+            else:
+                second = yaml
+                first_m = "click-manifest"
+                second_m = "package.yaml"
+            for exe_t in ['binaries', 'services']:
+                t = 'info'
+                n = 'yaml_%s' % exe_t
+                s = 'OK'
+                if exe_t in first and exe_t not in second:
+                    t = 'error'
+                    s = "%s missing '%s'" % (second_m, exe_t)
+                elif exe_t not in first and exe_t in second:
+                    t = 'error'
+                    s = "%s has extra '%s'" % (second_m, exe_t)
+                self._add_result(t, n, s)
+
+                if t == 'error':
+                    continue
+                elif exe_t not in first and exe_t not in second:
+                    continue
+
+                t = 'info'
+                n = 'yaml_%s_entries' % exe_t
+                s = 'OK'
+                if len(first[exe_t]) < len(second[exe_t]):
+                    t = 'error'
+                    s = "%s has extra '%s' entries" % (second_m, exe_t)
+                self._add_result(t, n, s)
+
+                for fapp in first[exe_t]:
+                    t = 'info'
+                    n = 'yaml_%s_%s' % (exe_t, fapp['name'])
+                    s = 'OK'
+                    sapp = None
+                    for tmp in second[exe_t]:
+                        if tmp['name'] == fapp['name']:
+                            sapp = tmp
+                    if sapp is None:
+                        t = 'error'
+                        s = "%s missing '%s'" % (second_m, fapp['name'])
+                        self._add_result(t, n, s)
+                        continue
+                    elif first == yaml and "security-override" in fapp or \
+                            second == yaml and "security-override" in sapp:
+                        # no reason to check security-override since apparmor
+                        # hook entry will point to this file
+                        continue
+                    elif first == yaml and "security-policy" in fapp or \
+                            second == yaml and "security-policy" in sapp:
+                        # no reason to check security-policy since apparmor
+                        # profile hook is used instead
+                        continue
+                    elif 'caps' not in fapp and 'caps' not in sapp and \
+                            second == yaml and 'security-template' not in sapp:
+                        # no caps in yaml, policy_groups is empty in click
+                        # manifest, unless security-template is in yaml
+                        t = 'error'
+                        s = "'policy_groups' not found in click manifest " + \
+                            "(should default to ['networking'])"
+                        self._add_result(t, n, s)
+                        continue
+                    self._add_result(t, n, s)
+
+                    for key in ['security-template', 'caps']:
+                        t = 'info'
+                        n = 'yaml_%s_%s' % (exe_t, second_m)
+                        s = 'OK'
+
+                        if key not in fapp:
+                            continue
+
+                        if key == 'caps':
+                            fapp['caps'] = set(fapp['caps'])
+                            if 'caps' in sapp:
+                                sapp['caps'] = set(sapp['caps'])
+
+                        if not find_match(fapp['name'], key, fapp[key], sapp):
+                            # handle snappy defaults for security-template
+                            # and caps
+                            if key == 'security-template' and \
+                               second == yaml and key not in sapp and \
+                               key in fapp and fapp[key] == 'default':
+                                # if yaml missing security-template, click may
+                                # specify 'default'
+                                self._add_result(t, n, s)
+                                continue
+                            elif key == 'caps' and second == yaml and \
+                                    'security-template' in sapp and \
+                                    key not in sapp and \
+                                    (key not in fapp or key in fapp and
+                                     fapp[key] == set([])):
+                                # when security-template is specified, then
+                                # caps won't default to 'networking' when
+                                # missing, so click manifest can omit or be []
+                                self._add_result(t, n, s)
+                                continue
+                            elif key == 'caps' and second == yaml and \
+                                    'security-template' not in sapp and \
+                                    key not in sapp and key in fapp and \
+                                    fapp[key] == set(['networking']):
+                                # no caps in yaml, policy_groups is networking
+                                # in click manifest wen security-template not
+                                # specified in yaml
+                                self._add_result(t, n, s)
+                                continue
+                            elif key == 'caps' and second == click_m and \
+                                    key not in sapp and key in fapp and \
+                                    fapp[key] == set([]):
+                                # no caps in click manifest, caps is [] in yaml
+                                self._add_result(t, n, s)
+                                continue
+                            t = 'error'
+                            s = "%s has different '%s' for '%s'" % \
+                                (second_m, key, fapp['name']) + \
+                                " - '%s:%s' vs '%s:%s'" % (first_m, fapp,
+                                                           second_m, sapp)
+                        self._add_result(t, n, s)
+
+    def _convert_click_security_to_yaml(self):
+        '''Convert click manifest to yaml'''
+        converted = dict()
+        for app in sorted(self.security_apps):
+            if 'snappy-systemd' in self.manifest['hooks'][app]:
+                key = 'services'
+            elif 'bin-path' in self.manifest['hooks'][app]:
+                key = 'binaries'
+            else:
+                t = 'error'
+                n = 'yaml_click_%s' % app
+                s = "click manifest missing 'snappy-systemd/bin-path' for " + \
+                    "'%s'" % app
+                self._add_result(t, n, s)
+                continue
+
+            if key not in converted:
+                converted[key] = []
+            tmp = dict()
+            tmp['name'] = app
+
+            (f, m) = self._get_security_manifest(app)
+            if 'template' in m:
+                tmp['security-template'] = m['template']
+
+            if 'policy_groups' in m:
+                tmp['caps'] = m['policy_groups']
+
+            converted[key].append(copy.deepcopy(tmp))
+
+        for app in sorted(self.security_apps_profiles):
+            if 'snappy-systemd' in self.manifest['hooks'][app]:
+                key = 'services'
+            elif 'bin-path' in self.manifest['hooks'][app]:
+                key = 'binaries'
+            else:
+                t = 'error'
+                n = 'yaml_click_%s' % app
+                s = "click manifest missing 'snappy-systemd/bin-path' for " + \
+                    "'%s'" % app
+                self._add_result(t, n, s)
+                continue
+
+            if key not in converted:
+                converted[key] = []
+            tmp = dict()
+            tmp['name'] = app
+
+            (f, p) = self._get_security_profile(app)
+            tmp['security-policy'] = {'apparmor': f}
+
+            converted[key].append(copy.deepcopy(tmp))
+
+        return copy.deepcopy(converted)
+
+    def check_security_yaml_and_click(self):
+        '''Verify click and security yaml are in sync (not including
+           override)
+        '''
+        if not self.is_snap or self.pkg_yaml['type'] in self.sec_skipped_types:
+            return
+
+        # setup a small dict that is a subset of self.pkg_yaml
+        yml = dict()
+        for exe_t in ['binaries', 'services']:
+            if exe_t not in self.pkg_yaml:
+                continue
+            yml[exe_t] = copy.deepcopy(self.pkg_yaml[exe_t])
+            for item in yml[exe_t]:
+                # account for binaries doing different things with 'name/exec'
+                if exe_t == 'binaries' and 'exec' not in item and \
+                   '/' in item['name']:
+                    item['name'] = os.path.basename(item['name'])
+
+        converted = self._convert_click_security_to_yaml()
+        self._compare_security_yamls(yml, converted)
+
+    def check_security_yaml_override_and_click(self):
+        '''Verify click and security yaml override are in sync'''
+        if not self.is_snap or self.pkg_yaml['type'] in self.sec_skipped_types:
+            return
+
+        for exe_t in ['services', 'binaries']:
+            if exe_t not in self.pkg_yaml:
+                continue
+
+            for item in self.pkg_yaml[exe_t]:
+                if 'name' not in item:
+                    t = 'error'
+                    n = 'yaml_override_click_name'
+                    s = "package.yaml malformed. Could not find 'name' " + \
+                        "for entry in '%s'" % item
+                    self._add_result(t, n, s)
+                    continue
+
+                app = item['name']
+                t = 'info'
+                n = 'yaml_override_click_%s' % app
+                s = "OK"
+                if 'security-override' not in item:
+                    s = "OK (skipping unspecified override)"
+                elif 'apparmor' not in item['security-override']:
+                    t = 'error'
+                    s = "'apparmor' not specified in 'security-override' " + \
+                        "for '%s'" % app
+                elif item['security-override']['apparmor'] not in \
+                        self.security_manifests:
+                    t = 'error'
+                    s = "'%s' not found in click manifest for '%s'" % \
+                        (item['security-override']['apparmor'], app)
+                # NOTE: we skip 'seccomp' because there isn't currently a
+                # click hook for it
+                self._add_result(t, n, s)
+
+    def check_security_yaml_override(self):
+        '''Verify security yaml override'''
+        for exe_t in ['services', 'binaries']:
+            if exe_t not in self.pkg_yaml:
+                continue
+
+            for item in self.pkg_yaml[exe_t]:
+                if 'name' not in item:
+                    t = 'error'
+                    n = 'yaml_override_name'
+                    s = "package.yaml malformed. Could not find 'name' " + \
+                        "for entry in '%s'" % item
+                    self._add_result(t, n, s)
+                    continue
+
+                app = item['name']
+                t = 'info'
+                n = 'yaml_override_format_%s' % app
+                s = "OK"
+                if 'security-override' not in item:
+                    s = "OK (skipping unspecified override)"
+                elif 'apparmor' not in item['security-override']:
+                    t = 'error'
+                    s = "'apparmor' not specified in 'security-override' " + \
+                        "for '%s'" % app
+                elif 'seccomp' not in item['security-override']:
+                    t = 'error'
+                    s = "'seccomp' not specified in 'security-override' " + \
+                        "for '%s'" % app
+                self._add_result(t, n, s)
+
+    def check_security_yaml_policy(self):
+        '''Verify security yaml policy'''
+        for exe_t in ['services', 'binaries']:
+            if exe_t not in self.pkg_yaml:
+                continue
+
+            for item in self.pkg_yaml[exe_t]:
+                if 'name' not in item:
+                    t = 'error'
+                    n = 'yaml_policy_name'
+                    s = "package.yaml malformed. Could not find 'name' " + \
+                        "for entry in '%s'" % item
+                    self._add_result(t, n, s)
+                    continue
+
+                app = item['name']
+                t = 'info'
+                n = 'yaml_policy_format_%s' % app
+                s = "OK"
+                if 'security-policy' not in item:
+                    s = "OK (skipping unspecified policy)"
+                elif 'apparmor' not in item['security-policy']:
+                    t = 'error'
+                    s = "'apparmor' not specified in 'security-policy' " + \
+                        "for '%s'" % app
+                elif 'seccomp' not in item['security-policy']:
+                    t = 'error'
+                    s = "'seccomp' not specified in 'security-policy' for " + \
+                        "'%s'" % app
+                self._add_result(t, n, s)
+
+        # TODO: error if specified
+
+    def check_security_yaml_combinations(self):
+        '''Verify security yaml uses valid combinations'''
+        if not self.is_snap or self.pkg_yaml['type'] in self.sec_skipped_types:
+            return
+
+        for exe_t in ['services', 'binaries']:
+            if exe_t not in self.pkg_yaml:
+                continue
+            for item in self.pkg_yaml[exe_t]:
+                if 'name' not in item:
+                    t = 'error'
+                    n = 'yaml_combinations_name'
+                    s = "package.yaml malformed. Could not find 'name' " + \
+                        "for entry in '%s'" % item
+                    self._add_result(t, n, s)
+                    continue
+
+                app = item['name']
+
+                t = 'info'
+                n = 'yaml_combinations_%s' % app
+                s = "OK"
+                if "security-policy" in item:
+                    for i in ['security-override', 'security-template',
+                              'caps']:
+                        if i in item:
+                            t = 'error'
+                            s = "Found '%s' with 'security-policy'" % (i)
+                            break
+                elif "security-override" in item:
+                    for i in ['security-policy', 'security-template', 'caps']:
+                        if i in item:
+                            t = 'error'
+                            s = "Found '%s' with 'security-override'" % (i)
+                            break
+                self._add_result(t, n, s)
+
+    def check_security_template(self):
+        '''Check snap security-template'''
+        if not self.is_snap or self.pkg_yaml['type'] in self.sec_skipped_types:
+            return
+
+        for exe_t in ['services', 'binaries']:
+            if exe_t not in self.pkg_yaml:
+                continue
+            for item in self.pkg_yaml[exe_t]:
+                if 'security-template' not in item:
+                    tmpl = ""
+                else:
+                    tmpl = item['security-template']
+
+                if 'name' not in item:
+                    t = 'error'
+                    n = 'yaml_security-template_name'
+                    s = "package.yaml malformed. Could not find 'name' " + \
+                        "for entry in '%s'" % item
+                    self._add_result(t, n, s)
+                    continue
+
+                # Handle bin/exec concept with bianries
+                app = os.path.basename(item['name'])
+
+                t = 'info'
+                n = 'yaml_security-template_%s' % app
+                s = "OK"
+                if not isinstance(tmpl, str):
+                    t = 'error'
+                    s = "'%s/%s' malformed: '%s' is not str" % (exe_t, app,
+                                                                tmpl)
+                    self._add_result(t, n, s)
+                    continue
+                self._add_result(t, n, s)
+
+                t = 'info'
+                n = 'yaml_security-template_in_manifest_%s' % app
+                s = "OK"
+                if app not in self.manifest['hooks']:
+                    t = 'error'
+                    s = "'%s' not found in click manifest" % app
+                    self._add_result(t, n, s)
+                    continue
+                elif 'apparmor' not in self.manifest['hooks'][app] and \
+                     'apparmor-profile' not in self.manifest['hooks'][app]:
+                    t = 'error'
+                    s = "'apparmor' not found in click manifest for '%s'" % app
+                    self._add_result(t, n, s)
+                    continue
+
+        # TODO: error if not 'common' or is 'unconfined'
+
+    def check_security_caps(self):
+        '''Check snap caps'''
+        # TODO - check caps on their own instead of just via compat click
+        # manifest

=== modified file 'clickreviews/cr_tests.py'
--- clickreviews/cr_tests.py	2015-04-03 22:17:04 +0000
+++ clickreviews/cr_tests.py	2015-05-18 21:07:49 +0000
@@ -745,12 +745,57 @@
         self._update_test_readme_md()
 
     def set_test_security_manifest(self, app, key, value):
-        '''Set key in security manifest to value. If value is None, remove
-           key'''
+        '''Set key in security manifest and package.yaml to value. If value is
+           None, remove key, if key is None, remove app.
+        '''
+        # package.yaml - we don't know if it is a service or a binary with
+        # these values, so just use 'binaries'
+        if key is None:
+            if 'binaries' in self.test_pkg_yaml:
+                for b in self.test_pkg_yaml['binaries']:
+                    if 'name' in b and b['name'] == app:
+                        self.test_pkg_yaml['binaries'].remove(b)
+                        break
+        elif value is None:
+            if 'binaries' in self.test_pkg_yaml:
+                found = False
+                for b in self.test_pkg_yaml['binaries']:
+                    if 'name' in b and b['name'] == app:
+                        if key in b:
+                            b.remove(key)
+                            found = True
+                            break
+        else:
+            found = False
+            k = key
+            if key == 'template':
+                k = 'security-template'
+            elif key == 'policy_groups':
+                k = 'caps'
+            if 'binaries' in self.test_pkg_yaml:
+                for b in self.test_pkg_yaml['binaries']:
+                    if 'name' in b and b['name'] == app:
+                        # Found the entry, so update key/value
+                        b[k] = value
+                        found = True
+                        break
+            # Did not find the entry, so create one
+            if not found:
+                if 'binaries' not in self.test_pkg_yaml:
+                    self.test_pkg_yaml['binaries'] = []
+                self.test_pkg_yaml['binaries'].append({'name': app,
+                                                       k: value})
+        self._update_test_pkg_yaml()
+
+        # click manifest
         if app not in self.test_security_manifests:
             self.test_security_manifests[app] = dict()
 
-        if value is None:
+        if key is None:
+            if app in self.test_security_manifests:
+                del self.test_security_manifests[app]
+                del self.test_manifest["hooks"][app]
+        elif value is None:
             if key in self.test_security_manifests[app]:
                 self.test_security_manifests[app].pop(key, None)
         else:
@@ -973,8 +1018,8 @@
         self._update_test_framework_policy_unknown()
 
     def set_test_systemd(self, app, key, value):
-        '''Set systemd entries. If key is None, snappy-systemd from manifest
-           and yaml.
+        '''Set systemd entries. If key is None, remove snappy-systemd from
+           manifest and yaml.
 
            Note the click manifest and the package.yaml use different
            storage types. pkg_yaml['services'] is a list of dictionaries where

=== modified file 'clickreviews/tests/test_cr_lint.py'
--- clickreviews/tests/test_cr_lint.py	2015-05-01 15:18:50 +0000
+++ clickreviews/tests/test_cr_lint.py	2015-05-18 21:07:49 +0000
@@ -1357,3 +1357,80 @@
         r = c.click_report
         expected_counts = {'info': None, 'warn': 0, 'error': 1}
         self.check_results(r, expected_counts)
+
+    def test_check_snappy_services_and_binaries1(self):
+        '''Test check_snappy_services_and_binaries() - different'''
+        self.set_test_pkg_yaml("name", self.test_name.split('_')[0])
+        self.set_test_pkg_yaml("services", [{"name": "foo",
+                                             "start": "bin/foo"}])
+        self.set_test_pkg_yaml("binaries", [{"name": "bar"}])
+        c = ClickReviewLint(self.test_name)
+        c.check_snappy_services_and_binaries()
+        r = c.click_report
+        expected_counts = {'info': 2, 'warn': 0, 'error': 0}
+        self.check_results(r, expected_counts)
+
+    def test_check_snappy_services_and_binaries2(self):
+        '''Test check_snappy_services_and_binaries() - different (exec)'''
+        self.set_test_pkg_yaml("name", self.test_name.split('_')[0])
+        self.set_test_pkg_yaml("services", [{"name": "foo",
+                                             "start": "bin/foo"}])
+        self.set_test_pkg_yaml("binaries", [{"name": "bar",
+                                             "exec": "bin/foo"}])
+        c = ClickReviewLint(self.test_name)
+        c.check_snappy_services_and_binaries()
+        r = c.click_report
+        expected_counts = {'info': 2, 'warn': 0, 'error': 0}
+        self.check_results(r, expected_counts)
+
+    def test_check_snappy_services_and_binaries3(self):
+        '''Test check_snappy_services_and_binaries() - same'''
+        self.set_test_pkg_yaml("name", self.test_name.split('_')[0])
+        self.set_test_pkg_yaml("services", [{"name": "foo",
+                                             "start": "bin/foo"}])
+        self.set_test_pkg_yaml("binaries", [{"name": "foo"}])
+        c = ClickReviewLint(self.test_name)
+        c.check_snappy_services_and_binaries()
+        r = c.click_report
+        expected_counts = {'info': 0, 'warn': 0, 'error': 2}
+        self.check_results(r, expected_counts)
+
+        m = r['error']['lint_snappy_foo_in_services']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)
+        m = r['error']['lint_snappy_foo_in_binaries']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)
+
+    def test_check_snappy_services_and_binaries4(self):
+        '''Test check_snappy_services_and_binaries() - same (subdir)'''
+        self.set_test_pkg_yaml("name", self.test_name.split('_')[0])
+        self.set_test_pkg_yaml("services", [{"name": "foo",
+                                             "start": "bin/foo"}])
+        self.set_test_pkg_yaml("binaries", [{"name": "bin/foo"}])
+        c = ClickReviewLint(self.test_name)
+        c.check_snappy_services_and_binaries()
+        r = c.click_report
+        expected_counts = {'info': 0, 'warn': 0, 'error': 2}
+        self.check_results(r, expected_counts)
+
+        m = r['error']['lint_snappy_foo_in_services']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)
+        m = r['error']['lint_snappy_foo_in_binaries']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)
+
+    def test_check_snappy_services_and_binaries5(self):
+        '''Test check_snappy_services_and_binaries() - same (exec, subdir)'''
+        self.set_test_pkg_yaml("name", self.test_name.split('_')[0])
+        self.set_test_pkg_yaml("services", [{"name": "foo",
+                                             "start": "bin/foo"}])
+        self.set_test_pkg_yaml("binaries", [{"name": "foo",
+                                             "exec": "bin/foo"}])
+        c = ClickReviewLint(self.test_name)
+        c.check_snappy_services_and_binaries()
+        r = c.click_report
+        expected_counts = {'info': 0, 'warn': 0, 'error': 2}
+        self.check_results(r, expected_counts)
+
+        m = r['error']['lint_snappy_foo_in_services']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)
+        m = r['error']['lint_snappy_foo_in_binaries']['text']
+        self.assertIn("'foo' in both 'services' and 'binaries'", m)

=== modified file 'clickreviews/tests/test_cr_security.py'
--- clickreviews/tests/test_cr_security.py	2015-05-01 15:03:57 +0000
+++ clickreviews/tests/test_cr_security.py	2015-05-18 21:07:49 +0000
@@ -32,6 +32,16 @@
         self.default_security_json = "%s.apparmor" % \
             self.default_appname
 
+    def _set_yaml_binary(self, entries, name=None):
+        d = dict()
+        if name is None:
+            d['name'] = self.default_appname
+        else:
+            d['name'] = name
+        for (key, value) in entries:
+            d[key] = value
+        self.set_test_pkg_yaml("binaries", [d])
+
     def test_check_policy_version_vendor(self):
         '''Test check_policy_version() - valid'''
         for v in [1.0]:  # update when have more vendor policy
@@ -482,6 +492,52 @@
         expected_counts = {'info': None, 'warn': 0, 'error': 1}
         self.check_results(report, expected_counts)
 
+    def test_check_template_snappy_framework_deprecated(self):
+        '''Test check_template() - in deprecated framework declaration'''
+        self.set_test_pkg_yaml("framework", "fwk")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "fwk_foo")
+        c.check_template()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_template_snappy_framework_deprecated2(self):
+        '''Test check_template() - in deprecated framework declaration list'''
+        self.set_test_pkg_yaml("framework", "fwk, somethingelse")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "fwk_foo")
+        c.check_template()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_template_snappy_frameworks(self):
+        '''Test check_template() - in frameworks declaration'''
+        self.set_test_pkg_yaml("frameworks", ["fwk"])
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "fwk_foo")
+        c.check_template()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_template_snappy_framework_type(self):
+        '''Test check_template() - type framework'''
+        self.set_test_pkg_yaml("type", "framework")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template",
+                                        "%s_foo" %
+                                        self.test_name.split('_')[0])
+        c.check_template()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
     def test_check_template_webapp(self):
         '''Test check_template() - webapp'''
         c = ClickReviewSecurity(self.test_name)
@@ -762,7 +818,7 @@
         c = ClickReviewSecurity(self.test_name)
         c.check_policy_groups()
         report = c.click_report
-        expected_counts = {'info': 0, 'warn': 1, 'error': 0}
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
         self.check_results(report, expected_counts)
 
     def test_check_policy_groups_bad_policy_version(self):
@@ -833,6 +889,54 @@
         expected_counts = {'info': None, 'warn': 0, 'error': 1}
         self.check_results(report, expected_counts)
 
+    def test_check_policy_groups_snappy_framework_deprecated(self):
+        '''Test check_policy_groups() - in deprecated framework declaration'''
+        self.set_test_pkg_yaml("framework", "fwk")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["fwk_foo"])
+        c.check_policy_groups()
+        report = c.click_report
+        expected_counts = {'info': 4, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_policy_groups_snappy_framework_deprecated2(self):
+        '''Test check_policy_groups() - in deprecated framework declaration
+           list
+        '''
+        self.set_test_pkg_yaml("framework", "fwk, somethingelse")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["fwk_foo"])
+        c.check_policy_groups()
+        report = c.click_report
+        expected_counts = {'info': 4, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_policy_groups_snappy_frameworks(self):
+        '''Test check_policy_groups() - in frameworks declaration'''
+        self.set_test_pkg_yaml("frameworks", ["fwk"])
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["fwk_foo"])
+        c.check_policy_groups()
+        report = c.click_report
+        expected_counts = {'info': 4, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_policy_groups_snappy_framework_type(self):
+        '''Test check_policy_groups() - type framework'''
+        self.set_test_pkg_yaml("type", "framework")
+        c = ClickReviewSecurity(self.test_name)
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups",
+                                        ["%s_foo" %
+                                         self.test_name.split('_')[0]])
+        c.check_policy_groups()
+        report = c.click_report
+        expected_counts = {'info': 4, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
     def test_check_policy_groups_pushhelper_no_hook(self):
         '''Test check_policy_groups_pushhelper() - no hook'''
         c = ClickReviewSecurity(self.test_name)
@@ -1154,3 +1258,573 @@
         report = c.click_report
         expected_counts = {'info': None, 'warn': 1, 'error': 0}
         self.check_results(report, expected_counts)
+
+    def test_check_security_template_default(self):
+        '''Test check_security_template() - default'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_template()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_template_default_with_exec(self):
+        '''Test check_security_template() - default with exec'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([('exec', 'bin/%s' % self.default_appname)])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_template()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_template_nondefault(self):
+        '''Test check_security_template() - nondefault'''
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "nondefault")
+        self._set_yaml_binary([('security-template', 'nondefault')])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_template()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_template_bad(self):
+        '''Test check_security_template() - {}'''
+        self._set_yaml_binary([('security-template', {})])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_template()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_name_relative(self):
+        '''Test check_security_yaml_and_click() - relative path'''
+        self._set_yaml_binary([('caps', ['networking'])],
+                              name="bin/%s" % self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+
+        # update the manifest and test_manifest
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 5, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_name_exec(self):
+        '''Test check_security_yaml_and_click() - uses exec'''
+        self._set_yaml_binary([('caps', ['networking']),
+                               ('exec', "bin/%s" % self.default_appname)],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+
+        # update the manifest and test_manifest
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 6, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_matching_template(self):
+        '''Test check_security_yaml_and_click() - matching default template'''
+        self._set_yaml_binary([('caps', ['networking'])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "default")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['security-template']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 6, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_security_override(self):
+        '''Test check_security_yaml_and_click() - security-override'''
+        self._set_yaml_binary([('security-template', 'default')])
+        self._set_yaml_binary([('security-override', {})])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "default")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_security_policy(self):
+        '''Test check_security_yaml_and_click() - security-policy'''
+        self._set_yaml_binary([('security-template', 'default')])
+        self._set_yaml_binary([('security-policy', {})])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "default")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_matching_caps(self):
+        '''Test check_security_yaml_and_click() - matching default caps'''
+        self._set_yaml_binary([('caps', [])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ['networking'])
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['caps']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 5, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_matching_no_caps(self):
+        '''Test check_security_yaml_and_click() - matching no caps'''
+        self._set_yaml_binary([('caps', [])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", None)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 5, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_matching_no_caps_template(self):
+        '''Test check_security_yaml_and_click() - matching no caps with
+           template
+        '''
+        self._set_yaml_binary([('security-template', 'nondefault')])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", None)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "nondefault")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 5, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch0(self):
+        '''Test check_security_yaml_and_click() - missing app in hooks'''
+        self._set_yaml_binary([])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", None)
+        c = ClickReviewSecurity(self.test_name)
+
+        del c.manifest["hooks"][self.default_appname]
+        self._update_test_manifest()
+        c.security_apps.remove(self.default_appname)
+
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch1(self):
+        '''Test check_security_yaml_and_click() - missing bin-path in hooks'''
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 2}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch2(self):
+        '''Test check_security_yaml_and_click() - missing apparmor in hooks'''
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.manifest["hooks"][self.default_appname]['apparmor']
+        c.security_apps.remove(self.default_appname)
+        self._update_test_manifest()
+
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch3(self):
+        '''Test check_security_yaml_and_click() - missing security-template'''
+        self._set_yaml_binary([('caps', ['networking'])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "nondefault")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['security-template']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch4(self):
+        '''Test check_security_yaml_and_click() - missing click template'''
+        self._set_yaml_binary([("security-template", "nondefault"),
+                               ('caps', ['networking'])],
+                              name=self.default_appname)
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", None)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch5(self):
+        '''Test check_security_yaml_and_click() - different templates'''
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "other")
+        self._set_yaml_binary([("security-template", "nondefault"),
+                               ('caps', ['networking'])],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 2}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch6(self):
+        '''Test check_security_yaml_and_click() - missing caps in yaml'''
+        self._set_yaml_binary([('caps', [])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["1", "2"])
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['caps']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch7(self):
+        '''Test check_security_yaml_and_click() - missing policy_groups'''
+        self._set_yaml_binary([('caps', ['networking'])],
+                              name=self.default_appname)
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", None)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch8(self):
+        '''Test check_security_yaml_and_click() - different caps/groups'''
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["1", "2"])
+        self._set_yaml_binary([('caps', ["3"])],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 2}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch9(self):
+        '''Test check_security_yaml_and_click() - unordered caps/groups'''
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ["1", "2"])
+        self._set_yaml_binary([('caps', ["2", "1"])],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': 5, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch10(self):
+        '''Test check_security_yaml_and_click() - missing caps in both'''
+        self._set_yaml_binary([('caps', [])])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", None)
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['caps']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_and_click_mismatch11(self):
+        '''Test check_security_yaml_and_click() - default caps with template'''
+        self._set_yaml_binary([('security-template', 'nondefault')])
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_groups", ['networking'])
+        self.set_test_security_manifest(self.default_appname,
+                                        "template", "nondefault")
+        c = ClickReviewSecurity(self.test_name)
+        c.manifest["hooks"][self.default_appname]['bin-path'] = "bin/path"
+        del c.pkg_yaml['binaries'][0]['caps']
+        c.check_security_yaml_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations(self):
+        '''Test check_security_yaml_combinations()'''
+        self._set_yaml_binary([], name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations1(self):
+        '''Test check_security_yaml_combinations() - template'''
+        self._set_yaml_binary([('security-template', 'foo')],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations2(self):
+        '''Test check_security_yaml_combinations() - caps'''
+        self._set_yaml_binary([('caps', ['networking'])],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations3(self):
+        '''Test check_security_yaml_combinations() - template,caps'''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('caps', ['networking'])],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations4(self):
+        '''Test check_security_yaml_combinations() - override'''
+        self._set_yaml_binary([('security-override', {'apparmor': 'foo.aa',
+                                                      'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations5(self):
+        '''Test check_security_yaml_combinations() - override, template'''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('security-override', {'apparmor': 'foo.aa',
+                                                      'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations6(self):
+        '''Test check_security_yaml_combinations() - override, caps'''
+        self._set_yaml_binary([('caps', ['networking']),
+                               ('security-override', {'apparmor': 'foo.aa',
+                                                      'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations7(self):
+        '''Test check_security_yaml_combinations() - override, caps, template
+        '''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('caps', ['networking']),
+                               ('security-override', {'apparmor': 'foo.aa',
+                                                      'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations8(self):
+        '''Test check_security_yaml_combinations() - override, caps, template,
+           policy
+        '''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('caps', ['networking']),
+                               ('security-policy', {'apparmor': 'foo.aa',
+                                                    'seccomp': 'foo.sc'}),
+                               ('security-override', {'apparmor': 'foo.aa',
+                                                      'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations9(self):
+        '''Test check_security_yaml_combinations() - policy'''
+        self._set_yaml_binary([('security-policy', {'apparmor': 'foo.aa',
+                                                    'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations10(self):
+        '''Test check_security_yaml_combinations() - policy, template'''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('security-policy', {'apparmor': 'foo.aa',
+                                                    'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations11(self):
+        '''Test check_security_yaml_combinations() - policy, caps'''
+        self._set_yaml_binary([('caps', ['networking']),
+                               ('security-policy', {'apparmor': 'foo.aa',
+                                                    'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_combinations12(self):
+        '''Test check_security_yaml_combinations() - policy, caps, template
+        '''
+        self._set_yaml_binary([('security-template', 'foo'),
+                               ('caps', ['networking']),
+                               ('security-policy', {'apparmor': 'foo.aa',
+                                                    'seccomp': 'foo.sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_combinations()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override_and_click(self):
+        '''Test check_security_yaml_override_and_click()'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override_and_click()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override_and_click_bad(self):
+        '''Test check_security_yaml_override_and_click() - bad'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([('security-override', {'apparmor':
+                                                      'something.else'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override_and_click()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override(self):
+        '''Test check_security_yaml_override()'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override2(self):
+        '''Test check_security_yaml_override() - seccomp/apparmor specified'''
+        self._set_yaml_binary([('security-override', {'apparmor': 'aa',
+                                                      'seccomp': 'sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override_missing1(self):
+        '''Test check_security_yaml_override() - missing apparmor'''
+        self._set_yaml_binary([('security-override', {'seccomp': 'sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_override_missing2(self):
+        '''Test check_security_yaml_override() - missing seccomp'''
+        self._set_yaml_binary([('security-override', {'apparmor': 'aa'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_override()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_policy(self):
+        '''Test check_security_yaml_policy()'''
+        self.set_test_security_manifest(self.default_appname, "template", None)
+        self._set_yaml_binary([])
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_policy()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_policy2(self):
+        '''Test check_security_yaml_policy() - seccomp/apparmor specified'''
+        self._set_yaml_binary([('security-policy', {'apparmor': 'aa',
+                                                    'seccomp': 'sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_policy()
+        report = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_policy_missing1(self):
+        '''Test check_security_yaml_policy() - missing apparmor'''
+        self._set_yaml_binary([('security-policy', {'seccomp': 'sc'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_policy()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)
+
+    def test_check_security_yaml_policy_missing2(self):
+        '''Test check_security_yaml_policy() - missing seccomp'''
+        self._set_yaml_binary([('security-policy', {'apparmor': 'aa'})],
+                              name=self.default_appname)
+        c = ClickReviewSecurity(self.test_name)
+        c.check_security_yaml_policy()
+        report = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(report, expected_counts)

=== modified file 'debian/changelog'
--- debian/changelog	2015-05-14 14:42:37 +0000
+++ debian/changelog	2015-05-18 21:07:49 +0000
@@ -3,6 +3,8 @@
   [ Jamie Strandboge ]
   * Makefile: perform run-pyflakes in check target
   * cr_systemd.py: add bus-name checks and update testsuite
+  * add security yaml checks
+  * cr_lint.py: don't allow same key in 'binaries' and 'services'
 
   [ Marcus Tomlinson ]
   * cr_scope.py: add "keywords" to the list of optional scope .ini keys

=== modified file 'run-pep8'
--- run-pep8	2014-08-22 13:22:48 +0000
+++ run-pep8	2015-05-18 21:07:49 +0000
@@ -2,8 +2,12 @@
 set -e
 
 echo "= pep8 ="
-for i in ./bin/update-* ./bin/click-check-* ./bin/click-show-files ./bin/click-review \
-	 ./clickreviews/*py ./clickreviews/tests/*py ; do
+for i in ./clickreviews/*py \
+         ./clickreviews/tests/*py \
+         ./bin/update-* \
+         ./bin/click-check-* \
+         ./bin/click-show-files \
+         ./bin/click-review ; do
     echo "Checking $i"
     pep8 $i
 done