Skip to content

Commit

Permalink
Merge pull request #236 from tri-adam/verify-refactor
Browse files Browse the repository at this point in the history 
Refactor verifyTask
  • Loading branch information
@tri-adam
tri-adam authored Oct 20, 2022
2 parents 4aa8254 + aaf5bf1 commit 4fb1380
Show file tree
Hide file tree
Showing 7 changed files with 761 additions and 745 deletions.
91 changes: 40 additions & 51 deletions pkg/integrity/clearsign.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package integrity
import (
"bytes"
"crypto"
"encoding/json"
"errors"
"io"
"time"
Expand All @@ -20,22 +19,13 @@ import (

var errClearsignedMsgNotFound = errors.New("clearsigned message not found")

// Hash functions specified for OpenPGP in RFC4880, excluding those that are not currently
// recommended by NIST.
var supportedPGPAlgorithms = []crypto.Hash{
crypto.SHA224,
crypto.SHA256,
crypto.SHA384,
crypto.SHA512,
}

type clearsignEncoder struct {
e *openpgp.Entity
config *packet.Config
}

// newClearsignEncoder returns an encoder that signs messages in clear-sign format using entity e. If
// timeFunc is not nil, it is used to generate signature timestamps.
// newClearsignEncoder returns an encoder that signs messages in clear-sign format using entity e.
// If timeFunc is not nil, it is used to generate signature timestamps.
func newClearsignEncoder(e *openpgp.Entity, timeFunc func() time.Time) *clearsignEncoder {
return &clearsignEncoder{
e: e,
Expand All @@ -46,65 +36,64 @@ func newClearsignEncoder(e *openpgp.Entity, timeFunc func() time.Time) *clearsig
}

// signMessage signs the message from r in clear-sign format, and writes the result to w. On
// success, the hash function and fingerprint of the signing key are returned.
func (s *clearsignEncoder) signMessage(w io.Writer, r io.Reader) (crypto.Hash, []byte, error) {
plaintext, err := clearsign.Encode(w, s.e.PrivateKey, s.config)
// success, the hash function is returned.
func (en *clearsignEncoder) signMessage(w io.Writer, r io.Reader) (crypto.Hash, error) {
plaintext, err := clearsign.Encode(w, en.e.PrivateKey, en.config)
if err != nil {
return 0, nil, err
return 0, err
}
defer plaintext.Close()

_, err = io.Copy(plaintext, r)
return s.config.Hash(), s.e.PrimaryKey.Fingerprint, err
return en.config.Hash(), err
}

// verifyAndDecodeJSON reads the first clearsigned message in data, verifies its signature, and
// returns the signing entity any suffix of data which follows the message. The plaintext is
// unmarshalled to v (if not nil).
func verifyAndDecodeJSON(data []byte, v interface{}, kr openpgp.KeyRing) (*openpgp.Entity, []byte, error) {
// Decode clearsign block and check signature.
e, plaintext, rest, err := verifyAndDecode(data, kr)
if err != nil {
return e, rest, err
}
type clearsignDecoder struct {
kr openpgp.KeyRing
}

// Unmarshal plaintext, if requested.
if v != nil {
err = json.Unmarshal(plaintext, v)
// newClearsignDecoder returns a decoder that verifies messages in clear-sign format using key
// material from kr.
func newClearsignDecoder(kr openpgp.KeyRing) *clearsignDecoder {
return &clearsignDecoder{
kr: kr,
}
return e, rest, err
}

// verifyAndDecode reads the first clearsigned message in data, verifies its signature, and returns
// the signing entity, plaintext and suffix of data which follows the message.
func verifyAndDecode(data []byte, kr openpgp.KeyRing) (*openpgp.Entity, []byte, []byte, error) {
// verifyMessage reads a message from r, verifies its signature, and returns the message contents.
// On success, the signing entity is set in vr.
func (de *clearsignDecoder) verifyMessage(r io.Reader, h crypto.Hash, vr *VerifyResult) ([]byte, error) {
data, err := io.ReadAll(r)
if err != nil {
return nil, err
}

// Decode clearsign block.
b, rest := clearsign.Decode(data)
b, _ := clearsign.Decode(data)
if b == nil {
return nil, nil, rest, errClearsignedMsgNotFound
return nil, errClearsignedMsgNotFound
}

// Hash functions specified for OpenPGP in RFC4880, excluding those that are not currently
// recommended by NIST.
expectedHashes := []crypto.Hash{
crypto.SHA224,
crypto.SHA256,
crypto.SHA384,
crypto.SHA512,
}

// Check signature.
e, err := openpgp.CheckDetachedSignatureAndHash(
kr,
vr.e, err = openpgp.CheckDetachedSignatureAndHash(
de.kr,
bytes.NewReader(b.Bytes),
b.ArmoredSignature.Body,
supportedPGPAlgorithms,
expectedHashes,
nil,
)
return e, b.Plaintext, rest, err
}

// isLegacySignature reads the first clearsigned message in data, and returns true if the plaintext
// contains a legacy signature.
func isLegacySignature(data []byte) (bool, error) {
// Decode clearsign block.
b, _ := clearsign.Decode(data)
if b == nil {
return false, errClearsignedMsgNotFound
if err != nil {
return nil, err
}

// The plaintext of legacy signatures always begins with "SIFHASH", and non-legacy signatures
// never do, as they are JSON.
return bytes.HasPrefix(b.Plaintext, []byte("SIFHASH:\n")), nil
return b.Plaintext, err
}
124 changes: 68 additions & 56 deletions pkg/integrity/clearsign_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"bufio"
"bytes"
"crypto"
"encoding/json"
"errors"
"io"
"reflect"
Expand All @@ -22,10 +21,8 @@ import (
"github.com/sebdah/goldie/v2"
)

type testType struct {
One int
Two int
}
var testMessage = `{"One":1,"Two":2}
`

func Test_clearsignEncoder_signMessage(t *testing.T) {
e := getTestEntity(t)
Expand All @@ -36,23 +33,18 @@ func Test_clearsignEncoder_signMessage(t *testing.T) {
tests := []struct {
name string
en *clearsignEncoder
r io.Reader
wantErr bool
wantHash crypto.Hash
wantFP []byte
}{
{
name: "EncryptedKey",
en: newClearsignEncoder(encrypted, fixedTime),
r: strings.NewReader(`{"One":1,"Two":2}`),
wantErr: true,
},
{
name: "OK",
en: newClearsignEncoder(e, fixedTime),
r: strings.NewReader(`{"One":1,"Two":2}`),
wantHash: crypto.SHA256,
wantFP: e.PrimaryKey.Fingerprint,
},
}

Expand All @@ -61,7 +53,7 @@ func Test_clearsignEncoder_signMessage(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
b := bytes.Buffer{}

ht, fp, err := tt.en.signMessage(&b, tt.r)
ht, err := tt.en.signMessage(&b, strings.NewReader(testMessage))
if got, want := err, tt.wantErr; (got != nil) != want {
t.Fatalf("got error %v, wantErr %v", got, want)
}
Expand All @@ -71,27 +63,16 @@ func Test_clearsignEncoder_signMessage(t *testing.T) {
t.Errorf("got hash %v, want %v", got, want)
}

if got, want := fp, tt.wantFP; !bytes.Equal(got, want) {
t.Errorf("got fingerprint %v, want %v", got, want)
}

g := goldie.New(t, goldie.WithTestNameForDir(true))
g.Assert(t, tt.name, b.Bytes())
}
})
}
}

func TestVerifyAndDecodeJSON(t *testing.T) {
func Test_clearsignDecoder_verifyMessage(t *testing.T) {
e := getTestEntity(t)

testValue := testType{1, 2}

testMessage, err := json.Marshal(testValue)
if err != nil {
t.Fatal(err)
}

// This is used to corrupt the plaintext.
corruptClearsign := func(w io.Writer, s string) error {
_, err := strings.NewReplacer(`{"One":1,"Two":2}`, `{"One":2,"Two":4}`).WriteString(w, s)
Expand Down Expand Up @@ -126,37 +107,79 @@ func TestVerifyAndDecodeJSON(t *testing.T) {
tests := []struct {
name string
hash crypto.Hash
el openpgp.EntityList
corrupter func(w io.Writer, s string) error
output interface{}
de *clearsignDecoder
wantErr error
wantEntity *openpgp.Entity
}{
{name: "ErrUnknownIssuer", el: openpgp.EntityList{}, wantErr: pgperrors.ErrUnknownIssuer},
{name: "CorruptedClearsign", el: openpgp.EntityList{e}, corrupter: corruptClearsign},
{name: "CorruptedSignature", el: openpgp.EntityList{e}, corrupter: corruptSignature},
{name: "VerifyOnly", el: openpgp.EntityList{e}, wantEntity: e},
{name: "DefaultHash", el: openpgp.EntityList{e}, output: &testType{}, wantEntity: e},
{name: "SHA1", hash: crypto.SHA1, el: openpgp.EntityList{e}, wantErr: pgperrors.StructuralError("hash algorithm mismatch with cleartext message headers")}, //nolint:lll
{name: "SHA224", hash: crypto.SHA224, el: openpgp.EntityList{e}, output: &testType{}, wantEntity: e},
{name: "SHA256", hash: crypto.SHA256, el: openpgp.EntityList{e}, output: &testType{}, wantEntity: e},
{name: "SHA384", hash: crypto.SHA384, el: openpgp.EntityList{e}, output: &testType{}, wantEntity: e},
{name: "SHA512", hash: crypto.SHA512, el: openpgp.EntityList{e}, output: &testType{}, wantEntity: e},
{
name: "UnknownIssuer",
de: newClearsignDecoder(openpgp.EntityList{}),
wantErr: pgperrors.ErrUnknownIssuer,
},
{
name: "CorruptedClearsign",
corrupter: corruptClearsign,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantErr: pgperrors.SignatureError("hash tag doesn't match"),
},
{
name: "CorruptedSignature",
corrupter: corruptSignature,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantErr: pgperrors.StructuralError("signature subpacket truncated"),
},
{
name: "DefaultHash",
de: newClearsignDecoder(openpgp.EntityList{e}),
wantEntity: e,
},
{
name: "SHA1",
hash: crypto.SHA1,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantErr: pgperrors.StructuralError("hash algorithm mismatch with cleartext message headers"),
},
{
name: "SHA224",
hash: crypto.SHA224,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantEntity: e,
},
{
name: "SHA256",
hash: crypto.SHA256,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantEntity: e,
},
{
name: "SHA384",
hash: crypto.SHA384,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantEntity: e,
},
{
name: "SHA512",
hash: crypto.SHA512,
de: newClearsignDecoder(openpgp.EntityList{e}),
wantEntity: e,
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
b := bytes.Buffer{}

cs := clearsignEncoder{
// Sign and encode message.
en := clearsignEncoder{
e: e,
config: &packet.Config{
DefaultHash: tt.hash,
Time: fixedTime,
},
}
_, _, err := cs.signMessage(&b, bytes.NewReader(testMessage))
h, err := en.signMessage(&b, strings.NewReader(testMessage))
if err != nil {
t.Fatal(err)
}
Expand All @@ -170,31 +193,20 @@ func TestVerifyAndDecodeJSON(t *testing.T) {
}
}

// Verify and decode.
e, rest, err := verifyAndDecodeJSON(b.Bytes(), tt.output, tt.el)
// Decode and verify message.
var vr VerifyResult
message, err := tt.de.verifyMessage(bytes.NewReader(b.Bytes()), h, &vr)

// Shouldn't be any trailing bytes.
if n := len(rest); n != 0 {
t.Errorf("%v trailing bytes", n)
}

// Verify the error (if any) is appropriate.
if tt.corrupter == nil {
if got, want := err, tt.wantErr; !errors.Is(got, want) {
t.Fatalf("got error %v, want %v", got, want)
}
} else if err == nil {
t.Errorf("got nil error despite corruption")
if got, want := err, tt.wantErr; !errors.Is(got, want) {
t.Fatalf("got error %v, want %v", got, want)
}

if err == nil {
if tt.output != nil {
if got, want := tt.output, &testValue; !reflect.DeepEqual(got, want) {
t.Errorf("got value %v, want %v", got, want)
}
if got, want := string(message), testMessage; got != want {
t.Errorf("got message %v, want %v", got, want)
}

if got, want := e, tt.wantEntity; !reflect.DeepEqual(got, want) {
if got, want := vr.e, tt.wantEntity; !reflect.DeepEqual(got, want) {
t.Errorf("got entity %+v, want %+v", got, want)
}
}
Expand Down
17 changes: 15 additions & 2 deletions pkg/integrity/select.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"sort"

"github.com/ProtonMail/go-crypto/openpgp/clearsign"
"github.com/sylabs/sif/v2/pkg/sif"
)

Expand Down Expand Up @@ -97,6 +98,19 @@ func getObjectSignatures(f *sif.FileImage, id uint32) ([]sif.Descriptor, error)
return sigs, nil
}

// isLegacySignature returns true if data contains a legacy signature.
func isLegacySignature(data []byte) bool {
// Legacy signatures always encoded in clear-sign format.
b, _ := clearsign.Decode(data)
if b == nil {
return false
}

// The plaintext of legacy signatures always begins with "SIFHASH", and non-legacy signatures
// never do, as they are JSON.
return bytes.HasPrefix(b.Plaintext, []byte("SIFHASH:\n"))
}

// getGroupSignatures returns descriptors in f that contain signature objects linked to the object
// group with identifier groupID. If legacy is true, only legacy signatures are considered.
// Otherwise, only non-legacy signatures are considered. If no such signatures are found, a
Expand All @@ -112,8 +126,7 @@ func getGroupSignatures(f *sif.FileImage, groupID uint32, legacy bool) ([]sif.De
return false, err
}

isLegacy, err := isLegacySignature(b)
return isLegacy == legacy, err
return isLegacySignature(b) == legacy, err
},
)
if err != nil {
Expand Down
Loading

0 comments on commit 4fb1380

Please sign in to comment.