Enforce Shared State Signing when options set
cover Enforce Shared State Signing when options set
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.
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-sstate
https://github.com/joshuagl/poky/tree/joshuagl/signed-sstate
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 | 15 +++++++++++---- meta/lib/oe/gpg_sign.py | 10 ++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-)
Signature · 1 line
2.21.0
RFC PATCH 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
@@ -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
RFC PATCH 2/3 classes/sstate: don't use unsigned sstate when verification enabled
In reply to Joshua LockWhen 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
@@ -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
RFC PATCH 3/3 classes/sstate: regenerate sstate when signing enabled
In reply to Joshua LockThis 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 | 10 ++++++++-- meta/lib/oe/gpg_sign.py | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-)
meta/classes/sstate.bbclass +8 -2
@@ -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,))
@@ -774,7 +778,7 @@ sstate_create_package () { } python sstate_sign_package () { - from oe.gpg_sign import get_signer + from oe.gpg_sign import get_signer, SignFailedError if d.getVar('SSTATE_SIG_KEY'): signer = get_signer(d, 'local')
@@ -783,6 +787,8 @@ python sstate_sign_package () { os.unlink(sstate_pkg + '.sig') signer.detach_sign(sstate_pkg, d.getVar('SSTATE_SIG_KEY', False), None, d.getVar('SSTATE_SIG_PASSPHRASE'), armor=False) + else: + raise SignFailedError("Can't sign sstate packages without key, SSTATE_SIG_KEY empty") } python sstate_report_unihash() {
meta/lib/oe/gpg_sign.py +10 -0
@@ -123,6 +123,16 @@ class LocalSigner(object): return ret +class SignFailedError(bb.build.FuncFailed): + def __init__(self, description, name=None, logfile=None): + self.description = description + self.name = name + self.logfile = logfile + + def __str__(self): + return 'Signing failed: %s' % self.description + + def get_signer(d, backend): """Get signer object for the specified backend""" # Use local signing by default
Signature · 1 line
2.21.0
Re: [RFC PATCH 3/3] classes/sstate: regenerate sstate when signing enabled
Quoted 65 lines · Joshua Lock via Openembedded-core
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.
Signed-off-by Joshua Lock <[email protected]>--- meta/classes/sstate.bbclass | 10 ++++++++-- meta/lib/oe/gpg_sign.py | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-)meta/classes/sstate.bbclass +1
-0@@ -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']meta/classes/sstate.bbclass +3
-1+ 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'] + \meta/classes/sstate.bbclass +4
-1+ sstate_create_package + \ (d.getVar('SSTATEPOSTCREATEFUNCS') or '').split(): # All hooks should run in SSTATE_BUILDDIR. bb.build.exec_func(f, d, (sstatebuild,))@@ -774,7 +778,7 @@ sstate_create_package () { } python sstate_sign_package () { - from oe.gpg_sign import get_signer + from oe.gpg_sign import get_signer, SignFailedError if d.getVar('SSTATE_SIG_KEY'): signer = get_signer(d, 'local')@@ -783,6 +787,8 @@ python sstate_sign_package () { os.unlink(sstate_pkg + '.sig') signer.detach_sign(sstate_pkg, d.getVar('SSTATE_SIG_KEY', False), None, d.getVar('SSTATE_SIG_PASSPHRASE'), armor=False) + else: + raise SignFailedError("Can't sign sstate packages without key, SSTATE_SIG_KEY empty")}
python
sstate_report_unihash(){meta/lib/oe/gpg_sign.py +0
-0@@ -123,6 +123,16 @@ class LocalSigner(object): return ret
meta/lib/oe/gpg_sign.py class LocalSigner(object): · line 135
Quoted 10 lines
meta/lib/oe/gpg_sign.py +10
-0+class SignFailedError(bb.build.FuncFailed): + def __init__(self, description, name=None, logfile=None): + self.description = description + self.name = name + self.logfile = logfile + + def __str__(self): + return 'Signing failed: %s' % self.description + +
Whilst this subclass is quite pythonic, I'm not sure we want to do this. At the back of my mind is the feeling that FuncFailed is a bit pointless :/.
Put differently, what does this buy us that bb.fatal() doesn't?
We do have a few users of FuncFailed in OECore but not many (about 8)...
Cheers,
Richard
Re: [RFC PATCH 3/3] classes/sstate: regenerate sstate when signing enabled
Quoted 71 lines · Richard Purdie
Quoted 65 lines · Joshua Lock via Openembedded-core
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.
Signed-off-by Joshua Lock <[email protected]>---
meta/classes/sstate.bbclass | 10 ++++++++--meta/lib/oe/gpg_sign.py| 10 ++++++++++2 files changed, 18 insertions(+), 2 deletions(-)meta/classes/sstate.bbclass +1
-0@@ -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']meta/classes/sstate.bbclass +3
-1+ 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'] + \meta/classes/sstate.bbclass +4
-1+ sstate_create_package + \ (d.getVar('SSTATEPOSTCREATEFUNCS') or '').split(): # All hooks should run in SSTATE_BUILDDIR. bb.build.exec_func(f, d, (sstatebuild,))@@ -774,7 +778,7 @@ sstate_create_package () { } python sstate_sign_package () { - from oe.gpg_sign import get_signer + from oe.gpg_sign import get_signer, SignFailedError if d.getVar('SSTATE_SIG_KEY'): signer = get_signer(d, 'local')@@ -783,6 +787,8 @@ python sstate_sign_package () { os.unlink(sstate_pkg + '.sig') signer.detach_sign(sstate_pkg, d.getVar('SSTATE_SIG_KEY', False), None, d.getVar('SSTATE_SIG_PASSPHRASE'), armor=False) + else: + raise SignFailedError("Can't sign sstate packages without key, SSTATE_SIG_KEY empty")}
python
sstate_report_unihash(){meta/lib/oe/gpg_sign.py +0
-0@@ -123,6 +123,16 @@ class LocalSigner(object): return ret
Quoted 14 lines
meta/lib/oe/gpg_sign.py class LocalSigner(object): · line 135
Quoted 10 lines
meta/lib/oe/gpg_sign.py +10
-0+class SignFailedError(bb.build.FuncFailed): + def __init__(self, description, name=None, logfile=None): + self.description = description + self.name = name + self.logfile = logfile + + def __str__(self): + return 'Signing failed: %s' % self.description + +Whilst this subclass is quite pythonic, I'm not sure we want to do this. At the back of my mind is the feeling that FuncFailed is a bit pointless :/.
This is pointless in more ways than one, because above I changed the SSTATECREATEFUNCS to only include sstate_sign_package if SSTATE_SIG_KEY is set this exception shouldn’t ever be raised.
I’ll send an update series with this class and its usage dropped.
Joshua
Quoted 11 lines
Put differently, what does this buy us that
bb.fatal()doesn't?We do have a few users of FuncFailed in OECore but not many (about 8)...
Cheers,
Richard