Skip to content

Commit

Permalink
Use a separate untrustedSignature instead of privateSignature embeddi…
Browse files Browse the repository at this point in the history 
…ng a Signature

Instead of a privateSignature containing a Signature, and using the
privateSignature type to attach private implementatinos of
json.Marshaler and json.Unmarshaler and other private methods,
use a completely separate private untrustedSignature type.

This allows us to use scarier Untrusted… names for the members, but the
only real code change is that verifyAndExtractSignature now needs to do
a member-by-member copy instead of copying the full Signature struct.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
@mtrmac
mtrmac committed Jan 17, 2017
1 parent 93a9f23 commit 954ed4b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 48 deletions.
8 changes: 3 additions & 5 deletions signature/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ func SignDockerManifest(m []byte, dockerReference string, mech SigningMechanism,
if err != nil {
return nil, err
}
sig := privateSignature{
Signature{
DockerManifestDigest: manifestDigest,
DockerReference: dockerReference,
},
sig := untrustedSignature{
UntrustedDockerManifestDigest: manifestDigest,
UntrustedDockerReference: dockerReference,
}
return sig.sign(mech, keyIdentity)
}
Expand Down
54 changes: 32 additions & 22 deletions signature/signature.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,35 @@ func (err InvalidSignatureError) Error() string {
}

// Signature is a parsed content of a signature.
// The only way to get this structure from a blob should be as a return value from a successful call to verifyAndExtractSignature below.
type Signature struct {
DockerManifestDigest digest.Digest
DockerReference string // FIXME: more precise type?
}

// Wrap signature to add to it some methods which we don't want to make public.
type privateSignature struct {
Signature
// untrustedSignature is a parsed content of a signature.
type untrustedSignature struct {
UntrustedDockerManifestDigest digest.Digest
UntrustedDockerReference string // FIXME: more precise type?
}

// Compile-time check that privateSignature implements json.Marshaler
var _ json.Marshaler = (*privateSignature)(nil)
// Compile-time check that untrustedSignature implements json.Marshaler
var _ json.Marshaler = (*untrustedSignature)(nil)

// MarshalJSON implements the json.Marshaler interface.
func (s privateSignature) MarshalJSON() ([]byte, error) {
func (s untrustedSignature) MarshalJSON() ([]byte, error) {
return s.marshalJSONWithVariables(time.Now().UTC().Unix(), "atomic "+version.Version)
}

// Implementation of MarshalJSON, with a caller-chosen values of the variable items to help testing.
func (s privateSignature) marshalJSONWithVariables(timestamp int64, creatorID string) ([]byte, error) {
if s.DockerManifestDigest == "" || s.DockerReference == "" {
func (s untrustedSignature) marshalJSONWithVariables(timestamp int64, creatorID string) ([]byte, error) {
if s.UntrustedDockerManifestDigest == "" || s.UntrustedDockerReference == "" {
return nil, errors.New("Unexpected empty signature content")
}
critical := map[string]interface{}{
"type": signatureType,
"image": map[string]string{"docker-manifest-digest": s.DockerManifestDigest.String()},
"identity": map[string]string{"docker-reference": s.DockerReference},
"image": map[string]string{"docker-manifest-digest": s.UntrustedDockerManifestDigest.String()},
"identity": map[string]string{"docker-reference": s.UntrustedDockerReference},
}
optional := map[string]interface{}{
"creator": creatorID,
Expand All @@ -66,11 +68,11 @@ func (s privateSignature) marshalJSONWithVariables(timestamp int64, creatorID st
return json.Marshal(signature)
}

// Compile-time check that privateSignature implements json.Unmarshaler
var _ json.Unmarshaler = (*privateSignature)(nil)
// Compile-time check that untrustedSignature implements json.Unmarshaler
var _ json.Unmarshaler = (*untrustedSignature)(nil)

// UnmarshalJSON implements the json.Unmarshaler interface
func (s *privateSignature) UnmarshalJSON(data []byte) error {
func (s *untrustedSignature) UnmarshalJSON(data []byte) error {
err := s.strictUnmarshalJSON(data)
if err != nil {
if _, ok := err.(jsonFormatError); ok {
Expand All @@ -82,7 +84,7 @@ func (s *privateSignature) UnmarshalJSON(data []byte) error {

// strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal jsonFormatError error type.
// Splitting it into a separate function allows us to do the jsonFormatError → InvalidSignatureError in a single place, the caller.
func (s *privateSignature) strictUnmarshalJSON(data []byte) error {
func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error {
var untyped interface{}
if err := json.Unmarshal(data, &untyped); err != nil {
return err
Expand Down Expand Up @@ -128,7 +130,7 @@ func (s *privateSignature) strictUnmarshalJSON(data []byte) error {
if err != nil {
return err
}
s.DockerManifestDigest = digest.Digest(digestString)
s.UntrustedDockerManifestDigest = digest.Digest(digestString)

identity, err := mapField(c, "identity")
if err != nil {
Expand All @@ -141,13 +143,18 @@ func (s *privateSignature) strictUnmarshalJSON(data []byte) error {
if err != nil {
return err
}
s.DockerReference = reference
s.UntrustedDockerReference = reference

return nil
}

// Sign formats the signature and returns a blob signed using mech and keyIdentity
func (s privateSignature) sign(mech SigningMechanism, keyIdentity string) ([]byte, error) {
// (If it seems surprising that this is a method on untrustedSignature, note that there
// isn’t a good reason to think that a key used by the user is trusted by any component
// of the system just because it is a private key — actually the presence of a private key
// on the system increases the likelihood of an a successful attack on that private key
// on that particular system.)
func (s untrustedSignature) sign(mech SigningMechanism, keyIdentity string) ([]byte, error) {
json, err := json.Marshal(s)
if err != nil {
return nil, err
Expand Down Expand Up @@ -178,16 +185,19 @@ func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte
return nil, err
}

var unmatchedSignature privateSignature
var unmatchedSignature untrustedSignature
if err := json.Unmarshal(signed, &unmatchedSignature); err != nil {
return nil, InvalidSignatureError{msg: err.Error()}
}
if err := rules.validateSignedDockerManifestDigest(unmatchedSignature.DockerManifestDigest); err != nil {
if err := rules.validateSignedDockerManifestDigest(unmatchedSignature.UntrustedDockerManifestDigest); err != nil {
return nil, err
}
if err := rules.validateSignedDockerReference(unmatchedSignature.DockerReference); err != nil {
if err := rules.validateSignedDockerReference(unmatchedSignature.UntrustedDockerReference); err != nil {
return nil, err
}
signature := unmatchedSignature.Signature // Policy OK.
return &signature, nil
// signatureAcceptanceRules have accepted this value.
return &Signature{
DockerManifestDigest: unmatchedSignature.UntrustedDockerManifestDigest,
DockerReference: unmatchedSignature.UntrustedDockerReference,
}, nil
}
39 changes: 18 additions & 21 deletions signature/signature_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func TestInvalidSignatureError(t *testing.T) {

func TestMarshalJSON(t *testing.T) {
// Empty string values
s := privateSignature{Signature{DockerManifestDigest: "", DockerReference: "_"}}
s := untrustedSignature{UntrustedDockerManifestDigest: "", UntrustedDockerReference: "_"}
_, err := s.MarshalJSON()
assert.Error(t, err)
s = privateSignature{Signature{DockerManifestDigest: "_", DockerReference: ""}}
s = untrustedSignature{UntrustedDockerManifestDigest: "_", UntrustedDockerReference: ""}
_, err = s.MarshalJSON()
assert.Error(t, err)

// Success
s = privateSignature{Signature{DockerManifestDigest: "digest!@#", DockerReference: "reference#@!"}}
s = untrustedSignature{UntrustedDockerManifestDigest: "digest!@#", UntrustedDockerReference: "reference#@!"}
marshaled, err := s.marshalJSONWithVariables(0, "CREATOR")
require.NoError(t, err)
assert.Equal(t, []byte("{\"critical\":{\"identity\":{\"docker-reference\":\"reference#@!\"},\"image\":{\"docker-manifest-digest\":\"digest!@#\"},\"type\":\"atomic container signature\"},\"optional\":{\"creator\":\"CREATOR\",\"timestamp\":0}}"),
Expand All @@ -41,7 +41,7 @@ func TestMarshalJSON(t *testing.T) {
}

// Return the result of modifying validJSON with fn and unmarshaling it into *sig
func tryUnmarshalModifiedSignature(t *testing.T, sig *privateSignature, validJSON []byte, modifyFn func(mSI)) error {
func tryUnmarshalModifiedSignature(t *testing.T, sig *untrustedSignature, validJSON []byte, modifyFn func(mSI)) error {
var tmp mSI
err := json.Unmarshal(validJSON, &tmp)
require.NoError(t, err)
Expand All @@ -51,12 +51,12 @@ func tryUnmarshalModifiedSignature(t *testing.T, sig *privateSignature, validJSO
testJSON, err := json.Marshal(tmp)
require.NoError(t, err)

*sig = privateSignature{}
*sig = untrustedSignature{}
return json.Unmarshal(testJSON, sig)
}

func TestUnmarshalJSON(t *testing.T) {
var s privateSignature
var s untrustedSignature
// Invalid input. Note that json.Unmarshal is guaranteed to validate input before calling our
// UnmarshalJSON implementation; so test that first, then test our error handling for completeness.
err := json.Unmarshal([]byte("&"), &s)
Expand All @@ -69,17 +69,15 @@ func TestUnmarshalJSON(t *testing.T) {
assert.Error(t, err)

// Start with a valid JSON.
validSig := privateSignature{
Signature{
DockerManifestDigest: "digest!@#",
DockerReference: "reference#@!",
},
validSig := untrustedSignature{
UntrustedDockerManifestDigest: "digest!@#",
UntrustedDockerReference: "reference#@!",
}
validJSON, err := validSig.MarshalJSON()
require.NoError(t, err)

// Success
s = privateSignature{}
s = untrustedSignature{}
err = json.Unmarshal(validJSON, &s)
require.NoError(t, err)
assert.Equal(t, validSig, s)
Expand Down Expand Up @@ -140,11 +138,9 @@ func TestSign(t *testing.T) {
mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory)
require.NoError(t, err)

sig := privateSignature{
Signature{
DockerManifestDigest: "digest!@#",
DockerReference: "reference#@!",
},
sig := untrustedSignature{
UntrustedDockerManifestDigest: "digest!@#",
UntrustedDockerReference: "reference#@!",
}

// Successful signing
Expand All @@ -159,24 +155,25 @@ func TestSign(t *testing.T) {
return nil
},
validateSignedDockerReference: func(signedDockerReference string) error {
if signedDockerReference != sig.DockerReference {
if signedDockerReference != sig.UntrustedDockerReference {
return errors.Errorf("Unexpected signedDockerReference")
}
return nil
},
validateSignedDockerManifestDigest: func(signedDockerManifestDigest digest.Digest) error {
if signedDockerManifestDigest != sig.DockerManifestDigest {
if signedDockerManifestDigest != sig.UntrustedDockerManifestDigest {
return errors.Errorf("Unexpected signedDockerManifestDigest")
}
return nil
},
})
require.NoError(t, err)

assert.Equal(t, sig.Signature, *verified)
assert.Equal(t, sig.UntrustedDockerManifestDigest, verified.DockerManifestDigest)
assert.Equal(t, sig.UntrustedDockerReference, verified.DockerReference)

// Error creating blob to sign
_, err = privateSignature{}.sign(mech, TestKeyFingerprint)
_, err = untrustedSignature{}.sign(mech, TestKeyFingerprint)
assert.Error(t, err)

// Error signing
Expand Down

0 comments on commit 954ed4b

Please sign in to comment.