Skip to contentSkip to conversation

Enforce Shared State Signing when optionsset

PATCH In review Joshua Lock 2 participants 5 messages Jul 30, 2019

lore
Share & tools
lore
#0 lore raw

v2 cover Enforce Shared State Signing when optionsset

There are 2 surprising behaviours in the current shared state signing implementation which this pull request addresses:
1) when signature verification is enabled a failure to verify doesn't prevent the shared state object from being used. A warning is printed and the unsigned shared state object is used to accelerate the task.
2) when signing is enabled the taskhash doesn't change and any existing, unsigned, shared state objects will not be signed. This is particularly problematic when combined with 1.

Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.

Changes since v1:
* removed spurious Python subclass class and handling of an error condition that couldn't occur

The following changes since commit 835f7eac0610325e906591cd81890bebe8627580:

meta/lib/oeqa: Test for bootimg-biosplusefi Source (2019-07-23 22:26:28 +0100)

are available in the Git repository at:

https://github.com/joshuagl/poky joshuagl/signed-sstate2
https://github.com/joshuagl/poky/tree/joshuagl/signed-sstate2

Joshua Lock (3):
sstate: fix log message
classes/sstate: don't use unsigned sstate when verification enabled classes/sstate: regenerate sstate when signing enabled

 meta/classes/sstate.bbclass | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
Signature · 1 line
2.21.0
#1 lore raw

PATCH v2 1/3 sstate: fix log message

Referring to the sstate object as a staging package is an artefact of the code's origins. Switch to referring to an "Sstate package" in order to be more accurate and consistent with the rest of the file.

---
 meta/classes/sstate.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/sstate.bbclass +1 -1
index b604729d84..d8fdcece6a 100644
@@ -329,7 +329,7 @@ def sstate_installpkg(ss, d):
         pstaging_fetch(sstatefetch, d)
 
     if not os.path.isfile(sstatepkg):
-        bb.note("Staging package %s does not exist" % sstatepkg)
+        bb.note("Sstate package %s does not exist" % sstatepkg)
         return False
 
     sstate_clean(ss, d)
Signature · 1 line
2.21.0
#2 lore raw

PATCH v2 2/3 classes/sstate: don't use unsigned sstate when verification enabled

In reply to Joshua Lock

When signature verification of shared state objects is enabled (SSTATE_VERIFY_SIG) use of an unsigned object, even though it produces a warning, seems unexpected. Instead skip unsigned objects and force the non-accelerated task to be run.

---
 meta/classes/sstate.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
meta/classes/sstate.bbclass +2 -1
index d8fdcece6a..3342c5ef50 100644
@@ -340,7 +340,8 @@ def sstate_installpkg(ss, d):
     if bb.utils.to_boolean(d.getVar("SSTATE_VERIFY_SIG"), False):
         signer = get_signer(d, 'local')
         if not signer.verify(sstatepkg + '.sig'):
-            bb.warn("Cannot verify signature on sstate package %s" % sstatepkg)
+            bb.warn("Cannot verify signature on sstate package %s, skipping acceleration..." % sstatepkg)
+            return False
 
     # Empty sstateinst directory, ensure its clean
     if os.path.exists(sstateinst):
Signature · 1 line
2.21.0
#3 lore raw

PATCH v2 3/3 classes/sstate: regenerate sstate when signing enabled

In reply to Joshua Lock

This change ensures that the task signatures changes, and therefore sstate tasks are rerun, when signing is enabled. This has the positive outcome that if signing is enabled new signed shared state objects will be produced, rather than just signing shared state objects for tasks where no work has been performed yet.

The downside of this change is that enabling/disabling sstate object signing alters the taskhash and results in rebuilding the world.

---
 meta/classes/sstate.bbclass | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
meta/classes/sstate.bbclass +12 -8
index 3342c5ef50..ee029196da 100644
@@ -659,8 +659,12 @@ def sstate_package(ss, d):
     if d.getVar('SSTATE_SKIP_CREATION') == '1':
         return
 
+    sstate_create_package = ['sstate_report_unihash', 'sstate_create_package']
+    if d.getVar('SSTATE_SIG_KEY'):
+        sstate_create_package.append('sstate_sign_package')
+
     for f in (d.getVar('SSTATECREATEFUNCS') or '').split() + \
-             ['sstate_report_unihash', 'sstate_create_package', 'sstate_sign_package'] + \
+             sstate_create_package + \
              (d.getVar('SSTATEPOSTCREATEFUNCS') or '').split():
         # All hooks should run in SSTATE_BUILDDIR.
         bb.build.exec_func(f, d, (sstatebuild,))
@@ -776,13 +780,13 @@ sstate_create_package () {
 python sstate_sign_package () {
     from oe.gpg_sign import get_signer
 
-    if d.getVar('SSTATE_SIG_KEY'):
-        signer = get_signer(d, 'local')
-        sstate_pkg = d.getVar('SSTATE_PKG')
-        if os.path.exists(sstate_pkg + '.sig'):
-            os.unlink(sstate_pkg + '.sig')
-        signer.detach_sign(sstate_pkg, d.getVar('SSTATE_SIG_KEY', False), None,
-                           d.getVar('SSTATE_SIG_PASSPHRASE'), armor=False)
+
+    signer = get_signer(d, 'local')
+    sstate_pkg = d.getVar('SSTATE_PKG')
+    if os.path.exists(sstate_pkg + '.sig'):
+        os.unlink(sstate_pkg + '.sig')
+    signer.detach_sign(sstate_pkg, d.getVar('SSTATE_SIG_KEY', False), None,
+                       d.getVar('SSTATE_SIG_PASSPHRASE'), armor=False)
 }
 
 python sstate_report_unihash() {
Signature · 1 line
2.21.0
#4 lore raw

✗ patchtest: failure for Enforce Shared State Signing when optionsset

In reply to Joshua Lock

== Series Details ==

Series: Enforce Shared State Signing when optionsset
Revision: 1
URL : https://patchwork.openembedded.org/series/18968/
State : failure

== Summary ==

Thank you for submitting this patch series to OpenEmbedded Core. This is an automated response. Several tests have been executed on the proposed series by patchtest resulting in the following failures:

* Issue Series does not apply on top of target branch [test_series_merge_on_head]
Suggested fix Rebase your series on top of targeted branch
Targeted branch master (currently at a62d60fc37)

If you believe any of these test results are incorrect, please reply to the mailing list ([email protected]) raising your concerns. Otherwise we would appreciate you correcting the issues and submitting a new version of the patchset if applicable. Please ensure you add/increment the version number when sending the new version (i.e. [PATCH] -> [PATCH v2] -> [PATCH v3] -> ...).

---

Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe