-
Notifications
You must be signed in to change notification settings - Fork 17
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
[2/3] Keydata v3 scope changes #270
Changes from 40 commits
74f8b08
4645ce8
ed975b3
a76f332
cc16a83
1af77a7
bfe8980
6c9fb0f
84e6d65
48fab3f
a214639
78e3364
a6e3520
f9a7f8e
8810ae5
dad82f9
d9b9a2a
2e4c29a
f2ffc65
2f0e295
4c10b40
0521626
6625be3
767ac13
dae8903
d42e414
8e77c31
b1e8248
8299964
8cec665
8177fbc
6bc4186
da49e74
f65f537
0cd840a
159895f
e6408fd
284ebca
2a5277c
c12c559
6051c17
8f2f770
27bdb5f
65291ec
07bab29
6a0df9b
29521d8
576b3cf
7957f52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2023 Canonical Ltd | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 3 as | ||
* published by the Free Software Foundation. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package bootenv | ||
|
||
import ( | ||
"testing" | ||
|
||
. "gopkg.in/check.v1" | ||
) | ||
|
||
func Test(t *testing.T) { TestingT(t) } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2023 Canonical Ltd | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 3 as | ||
* published by the Free Software Foundation. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package bootenv | ||
|
||
import ( | ||
"errors" | ||
"sync/atomic" | ||
|
||
"github.com/snapcore/secboot" | ||
) | ||
|
||
var ( | ||
currentModel atomic.Value | ||
currentBootMode atomic.Value | ||
) | ||
|
||
var SetModel = func(model secboot.SnapModel) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than making this function and These probably want some documentation as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The initial branch only allowed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, so the intention was that these functions would only succeed the first time that they are called. It might be better to just convert these to call You'd still want a way to clear them before a test though, so perhaps adding a function to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still can't see how I can workaround the fact that atomic.Value can't be set to nil after it is set. My intention here was to mock both the setters and getters to use I tried changing the setters to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, when I suggested just setting
This obviously isn't safe in production code, but should be fine from test code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right, thanks for this suggestion, it cleaned up the code quite a bit. I implemented it as suggested. |
||
return currentModel.CompareAndSwap(nil, model) | ||
} | ||
|
||
var SetBootMode = func(mode string) bool { | ||
return currentBootMode.CompareAndSwap(nil, mode) | ||
} | ||
|
||
var loadCurrentModel = func() (secboot.SnapModel, error) { | ||
model, ok := currentModel.Load().(secboot.SnapModel) | ||
if !ok { | ||
return nil, errors.New("SetModel hasn't been called yet") | ||
} | ||
return model, nil | ||
} | ||
|
||
var loadCurrentBootMode = func() (string, error) { | ||
mode, ok := currentBootMode.Load().(string) | ||
if !ok { | ||
return "", errors.New("SetBootMode hasn't been called yet") | ||
} | ||
return mode, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2023 Canonical Ltd | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 3 as | ||
* published by the Free Software Foundation. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package bootenv | ||
|
||
import ( | ||
"bytes" | ||
"crypto" | ||
"crypto/ecdsa" | ||
"crypto/elliptic" | ||
"crypto/x509" | ||
"encoding/asn1" | ||
"errors" | ||
|
||
"github.com/snapcore/secboot" | ||
internal_crypto "github.com/snapcore/secboot/internal/crypto" | ||
"golang.org/x/crypto/cryptobyte" | ||
cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" | ||
) | ||
|
||
var ( | ||
ComputeSnapModelHash = computeSnapModelHash | ||
chrisccoulson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
func MockSetModel(f func(secboot.SnapModel) bool) (restore func()) { | ||
origSetModel := SetModel | ||
SetModel = f | ||
return func() { | ||
SetModel = origSetModel | ||
} | ||
} | ||
|
||
func MockSetBootMode(f func(string) bool) (restore func()) { | ||
origSetBootMode := SetBootMode | ||
SetBootMode = f | ||
return func() { | ||
SetBootMode = origSetBootMode | ||
} | ||
} | ||
|
||
func MockLoadCurrentModel(f func() (secboot.SnapModel, error)) (restore func()) { | ||
origLoadCurrentModel := loadCurrentModel | ||
loadCurrentModel = f | ||
return func() { | ||
loadCurrentModel = origLoadCurrentModel | ||
} | ||
} | ||
|
||
func MockLoadCurrenBootMode(f func() (string, error)) (restore func()) { | ||
origLoadCurrentBootMode := loadCurrentBootMode | ||
loadCurrentBootMode = f | ||
return func() { | ||
loadCurrentBootMode = origLoadCurrentBootMode | ||
} | ||
} | ||
|
||
func (d *KeyDataScope) TestSetVersion(version int) { | ||
d.data.Version = version | ||
} | ||
|
||
func unmarshalHashAlg(s *cryptobyte.String) (hashAlg, error) { | ||
var str cryptobyte.String | ||
|
||
if !s.ReadASN1(&str, cryptobyte_asn1.SEQUENCE) { | ||
return 0, errors.New("malformed input") | ||
} | ||
|
||
var oid asn1.ObjectIdentifier | ||
|
||
if !str.ReadASN1ObjectIdentifier(&oid) { | ||
return 0, errors.New("malformed Algorithm identifier") | ||
} | ||
|
||
var null uint8 | ||
|
||
if !str.ReadUint8(&null) { | ||
return 0, errors.New("malformed input") | ||
} | ||
|
||
if len(oid) == len(sha1Oid) { | ||
return hashAlg(crypto.SHA1), nil | ||
} | ||
|
||
switch oid[8] { | ||
case sha224Oid[8]: | ||
return hashAlg(crypto.SHA224), nil | ||
case sha256Oid[8]: | ||
return hashAlg(crypto.SHA256), nil | ||
case sha384Oid[8]: | ||
return hashAlg(crypto.SHA384), nil | ||
case sha512Oid[8]: | ||
return hashAlg(crypto.SHA512), nil | ||
default: | ||
return 0, errors.New("unsupported hash algorithm") | ||
} | ||
} | ||
|
||
func (d *KeyDataScope) TestMatch(KDFAlg crypto.Hash, keyIdentifier []byte) bool { | ||
der, err := x509.MarshalPKIXPublicKey(d.data.PublicKey.PublicKey) | ||
if err != nil { | ||
return false | ||
} | ||
|
||
h := KDFAlg.New() | ||
h.Write(der) | ||
return bytes.Equal(h.Sum(nil), keyIdentifier) | ||
} | ||
|
||
func (d *KeyDataScope) DeriveSigner(key secboot.PrimaryKey, role string) (crypto.Signer, error) { | ||
return d.deriveSigner(key, role) | ||
} | ||
|
||
func NewHashAlg(alg crypto.Hash) hashAlg { | ||
return hashAlg(alg) | ||
} | ||
|
||
func NewEcdsaPublicKey(rand []byte) (ecdsaPublicKey, error) { | ||
var pk ecdsaPublicKey | ||
|
||
privateKey, err := internal_crypto.GenerateECDSAKey(elliptic.P256(), bytes.NewReader(rand)) | ||
if err != nil { | ||
return pk, err | ||
} | ||
|
||
pk.PublicKey = privateKey.Public().(*ecdsa.PublicKey) | ||
|
||
return pk, nil | ||
} | ||
|
||
func (d *KeyDataScope) Data() keyDataScope { | ||
return d.data | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the package itself probably needs a doc comment and maybe a clearer name, I'm not sure why it's called bootenv? it exists to offer key scope support for platforms not doing measured boot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess
bootenv
was the initial name because the key scope support is related to the boot environment (boot mode and models). Maybe a different name with the word "scope" in it?bootscope
/keyscope
? @chrisccoulsonThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bootscope is not a bad name, adding a package doc comment is still also a good idea, as this is meant to be used by platforms that don't do measurements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with bootscope as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed bootenv to bootscope and added package doc comment.