From 626c43fbaf286ca8814d492b7476f5939ac14372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 11 Jan 2017 20:29:19 +0100 Subject: [PATCH] Add signature.GetUntrustedSignatureInformationWithoutVerifying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add signature.GetUntrustedSignatureInformationWithoutVerifying, which returns the contents of a signature without doing any cryptographic verification. Expected uses are 1) debugging verification failures, and (CAREFULLY!) 2) listing which signatures are stored in a registry along with an image. It is STRONGLY recommended to do cryptographic verification in the latter case, and to clearly indicate untrusted signatures in the UI. Signed-off-by: Miloslav Trmač --- .../fixtures/no-optional-fields.signature | Bin 0 -> 383 bytes signature/fixtures_info_test.go | 2 + signature/mechanism.go | 38 ++++++++++++ signature/mechanism_test.go | 56 ++++++++++++++++++ signature/signature.go | 55 +++++++++++++++++ signature/signature_test.go | 39 ++++++++++++ 6 files changed, 190 insertions(+) create mode 100644 signature/fixtures/no-optional-fields.signature diff --git a/signature/fixtures/no-optional-fields.signature b/signature/fixtures/no-optional-fields.signature new file mode 100644 index 0000000000000000000000000000000000000000..482ae3acf0d80c22aa8b1a7b18e48e2b04a6ffa0 GIT binary patch literal 383 zcmV-_0f7Fa0h_?f%)rEWyXccd_m-R!jHeH%Cox3Sb@f*(B^PCuWF{x(C|Ol2Wu~O& zm1LGwg4ikf$=Rtzx<#pJsYR)I$*D?KN+qeqC7F5Y`nidDnQ1__Qmu|sW^Q77Dw2Ab zoNh{HI!K9?QgKG2k*S%LkwH?Lkzs0ziK%&#v8929k!5m9Qfg|lg}J3^qIptkqM@0Q znWd#+T1ujsfpMxqih*I0Nve68nSqIsd8(-?$g+~k0+2frOY(CwlNFNl^GXsk^HPfx ziZj#m5=%;pQbCIH3raHc^S~aet>x;N!@|JG#K6YN1oAEe7pDL$5KP}Q|CQxEdf?iT z>oJuvx30at8&fN@>6W#1n%~38%QyP#Cj0Gwyw4%JuSg;F_u*r=s>SCly6E}jRFg&1 zFWq%aXOwoQ<}TcQdH!|7wT=^6-&tGS@;)RI9^v5X_^{!T;H`fyix$@Xe1Ei|_|g>9 dKT&fXzc>`SoomwQP+E4bv`?bx;PvLU_W&nc%{2f3 literal 0 HcmV?d00001 diff --git a/signature/fixtures_info_test.go b/signature/fixtures_info_test.go index e9f21f7e35..a44a6b7364 100644 --- a/signature/fixtures_info_test.go +++ b/signature/fixtures_info_test.go @@ -9,4 +9,6 @@ const ( TestImageSignatureReference = "testing/manifest" // TestKeyFingerprint is the fingerprint of the private key in this directory. TestKeyFingerprint = "1D8230F6CDB6A06716E414C1DB72F2188BB46CC8" + // TestKeyShortID is the short ID of the private key in this directory. + TestKeyShortID = "DB72F2188BB46CC8" ) diff --git a/signature/mechanism.go b/signature/mechanism.go index 196ad92789..4b4393bcaf 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -4,9 +4,13 @@ package signature import ( "bytes" + "errors" "fmt" + "io/ioutil" + "strings" "github.com/mtrmac/gpgme" + "golang.org/x/crypto/openpgp" ) // SigningMechanism abstracts a way to sign binary blobs and verify their signatures. @@ -21,6 +25,12 @@ type SigningMechanism interface { Sign(input []byte, keyIdentity string) ([]byte, error) // Verify parses unverifiedSignature and returns the content and the signer's identity Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) + // UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, + // along with a short identifier of the key used for signing. + // WARNING: The short key identifier (which correponds to "Key ID" for OpenPGP keys) + // is NOT the same as a "key identity" used in other calls ot this interface, and + // the values may have no recognizable relationship if the public key is not available. + UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) } // A GPG/OpenPGP signing mechanism. @@ -119,3 +129,31 @@ func (m gpgSigningMechanism) Verify(unverifiedSignature []byte) (contents []byte } return signedBuffer.Bytes(), sig.Fingerprint, nil } + +// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, +// along with a short identifier of the key used for signing. +// WARNING: The short key identifier (which correponds to "Key ID" for OpenPGP keys) +// is NOT the same as a "key identity" used in other calls ot this interface, and +// the values may have no recognizable relationship if the public key is not available. +func (m gpgSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { + // This uses the Golang-native OpenPGP implementation instead of gpgme because we are not doing any cryptography. + md, err := openpgp.ReadMessage(bytes.NewReader(untrustedSignature), openpgp.EntityList{}, nil, nil) + if err != nil { + return nil, "", err + } + if !md.IsSigned { + return nil, "", errors.New("The input is not a signature") + } + content, err := ioutil.ReadAll(md.UnverifiedBody) + if err != nil { + // Coverage: An error during reading the body can happen only if + // 1) the message is encrypted, which is not our case (and we don’t give ReadMessage the key + // to decrypt the contents anyway), or + // 2) the message is signed AND we give ReadMessage a correspnding public key, which we don’t. + return nil, "", err + } + + // Uppercase the key ID for minimal consistency with the gpgme-returned fingerprints + // (but note that key ID is a suffix of the fingerprint only for V4 keys, not V3)! + return content, strings.ToUpper(fmt.Sprintf("%016X", md.SignedByKeyId)), nil +} diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index e6cbc0a7b6..5aee945faa 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -147,3 +147,59 @@ func TestGPGSigningMechanismVerify(t *testing.T) { // The various GPG/GPGME failures cases are not obviously easy to reach. } + +func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) { + mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) + require.NoError(t, err) + + // A valid signature + signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") + require.NoError(t, err) + content, shortKeyID, err := mech.UntrustedSignatureContents(signature) + require.NoError(t, err) + assert.Equal(t, []byte("This is not JSON\n"), content) + assert.Equal(t, TestKeyShortID, shortKeyID) + + // Completely invalid signature. + _, _, err = mech.UntrustedSignatureContents([]byte{}) + assert.Error(t, err) + + _, _, err = mech.UntrustedSignatureContents([]byte("invalid signature")) + assert.Error(t, err) + + // Literal packet, not a signature + signature, err = ioutil.ReadFile("./fixtures/unsigned-literal.signature") + require.NoError(t, err) + content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + assert.Error(t, err) + + // Encrypted data, not a signature. + signature, err = ioutil.ReadFile("./fixtures/unsigned-encrypted.signature") + require.NoError(t, err) + content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + assert.Error(t, err) + + // Expired signature + signature, err = ioutil.ReadFile("./fixtures/expired.signature") + require.NoError(t, err) + content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + require.NoError(t, err) + assert.Equal(t, []byte("This signature is expired.\n"), content) + assert.Equal(t, TestKeyShortID, shortKeyID) + + // Corrupt signature + signature, err = ioutil.ReadFile("./fixtures/corrupt.signature") + require.NoError(t, err) + content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + require.NoError(t, err) + assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic ","timestamp":1458239713}}`), content) + assert.Equal(t, TestKeyShortID, shortKeyID) + + // Valid signature with an unknown key + signature, err = ioutil.ReadFile("./fixtures/unknown-key.signature") + require.NoError(t, err) + content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + require.NoError(t, err) + assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic 0.1.13-dev","timestamp":1464633474}}`), content) + assert.Equal(t, "E5476D1110D07803", shortKeyID) +} diff --git a/signature/signature.go b/signature/signature.go index 76b574cc00..23092592bd 100644 --- a/signature/signature.go +++ b/signature/signature.go @@ -45,6 +45,22 @@ type untrustedSignature struct { UntrustedTimestamp *int64 } +// UntrustedSignatureInformation is information available in an untrusted signature. +// This may be useful when debugging signature verification failures, +// or when managing a set of signatures on a single image. +// +// WARNING: Do not use the contents of this for ANY security decisions, +// and be VERY CAREFUL about showing this information to humans in any way which suggest that these values “are probably” reliable. +// There is NO REASON to expect the values to be correct, or not intentionally misleading +// (including things like “✅ Verified by $authority”) +type UntrustedSignatureInformation struct { + UntrustedDockerManifestDigest digest.Digest + UntrustedDockerReference string // FIXME: more precise type? + UntrustedCreatorID *string + UntrustedTimestamp *time.Time + UntrustedShortKeyIdentifier string +} + // newUntrustedSignature returns an untrustedSignature object with // the specified primary contents and appropriate metadata. func newUntrustedSignature(dockerManifestDigest digest.Digest, dockerReference string) untrustedSignature { @@ -233,3 +249,42 @@ func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte DockerReference: unmatchedSignature.UntrustedDockerReference, }, nil } + +// GetUntrustedSignatureInformationWithoutVerifying extracts information available in an untrusted signature, +// WITHOUT doing any cryptographic verification. +// This may be useful when debugging signature verification failures, +// or when managing a set of signatures on a single image. +// +// WARNING: Do not use the contents of this for ANY security decisions, +// and be VERY CAREFUL about showing this information to humans in any way which suggest that these values “are probably” reliable. +// There is NO REASON to expect the values to be correct, or not intentionally misleading +// (including things like “✅ Verified by $authority”) +func GetUntrustedSignatureInformationWithoutVerifying(untrustedSignatureBytes []byte) (*UntrustedSignatureInformation, error) { + // NOTE: This should eventualy do format autodetection. + mech, err := NewGPGSigningMechanism() + if err != nil { + return nil, err + } + + untrustedContents, shortKeyIdentifier, err := mech.UntrustedSignatureContents(untrustedSignatureBytes) + if err != nil { + return nil, err + } + var untrustedDecodedContents untrustedSignature + if err := json.Unmarshal(untrustedContents, &untrustedDecodedContents); err != nil { + return nil, InvalidSignatureError{msg: err.Error()} + } + + var timestamp *time.Time // = nil + if untrustedDecodedContents.UntrustedTimestamp != nil { + ts := time.Unix(*untrustedDecodedContents.UntrustedTimestamp, 0) + timestamp = &ts + } + return &UntrustedSignatureInformation{ + UntrustedDockerManifestDigest: untrustedDecodedContents.UntrustedDockerManifestDigest, + UntrustedDockerReference: untrustedDecodedContents.UntrustedDockerReference, + UntrustedCreatorID: untrustedDecodedContents.UntrustedCreatorID, + UntrustedTimestamp: timestamp, + UntrustedShortKeyIdentifier: shortKeyIdentifier, + }, nil +} diff --git a/signature/signature_test.go b/signature/signature_test.go index b68a159141..3ddcffe574 100644 --- a/signature/signature_test.go +++ b/signature/signature_test.go @@ -336,3 +336,42 @@ func TestVerifyAndExtractSignature(t *testing.T) { assert.Nil(t, sig) assert.Equal(t, signatureData, recorded) } + +func TestGetUntrustedSignatureInformationWithoutVerifying(t *testing.T) { + signature, err := ioutil.ReadFile("./fixtures/image.signature") + require.NoError(t, err) + // Successful parsing, all optional fields present + info, err := GetUntrustedSignatureInformationWithoutVerifying(signature) + require.NoError(t, err) + assert.Equal(t, TestImageSignatureReference, info.UntrustedDockerReference) + assert.Equal(t, TestImageManifestDigest, info.UntrustedDockerManifestDigest) + assert.NotNil(t, info.UntrustedCreatorID) + assert.Equal(t, "atomic ", *info.UntrustedCreatorID) + assert.NotNil(t, info.UntrustedTimestamp) + assert.Equal(t, time.Unix(1458239713, 0), *info.UntrustedTimestamp) + assert.Equal(t, TestKeyShortID, info.UntrustedShortKeyIdentifier) + // Successful parsing, no optional fields present + signature, err = ioutil.ReadFile("./fixtures/no-optional-fields.signature") + require.NoError(t, err) + // Successful parsing + info, err = GetUntrustedSignatureInformationWithoutVerifying(signature) + require.NoError(t, err) + assert.Equal(t, TestImageSignatureReference, info.UntrustedDockerReference) + assert.Equal(t, TestImageManifestDigest, info.UntrustedDockerManifestDigest) + assert.Nil(t, info.UntrustedCreatorID) + assert.Nil(t, info.UntrustedTimestamp) + assert.Equal(t, TestKeyShortID, info.UntrustedShortKeyIdentifier) + + // Completely invalid signature. + _, err = GetUntrustedSignatureInformationWithoutVerifying([]byte{}) + assert.Error(t, err) + + _, err = GetUntrustedSignatureInformationWithoutVerifying([]byte("invalid signature")) + assert.Error(t, err) + + // Valid signature of non-JSON + invalidBlobSignature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") + require.NoError(t, err) + _, err = GetUntrustedSignatureInformationWithoutVerifying(invalidBlobSignature) + assert.Error(t, err) +}