diff --git a/openpgp/packet/config.go b/openpgp/packet/config.go index f9208158..4e2b27d1 100644 --- a/openpgp/packet/config.go +++ b/openpgp/packet/config.go @@ -94,6 +94,10 @@ type Config struct { // might be no other way than to tolerate the missing MDC. Setting this flag, allows this // mode of operation. It should be considered a measure of last resort. InsecureAllowUnauthenticatedMessages bool + // KnownNotations is a map of Notation Data names to bools, which controls + // the notation names that are allowed to be present in critical Notation Data + // signature subpackets. + KnownNotations map[string]bool } func (c *Config) Random() io.Reader { @@ -202,3 +206,10 @@ func (c *Config) AllowUnauthenticatedMessages() bool { } return c.InsecureAllowUnauthenticatedMessages } + +func (c *Config) KnownNotation(notationName string) bool { + if c == nil { + return false + } + return c.KnownNotations[notationName] +} diff --git a/openpgp/read.go b/openpgp/read.go index 34d324c0..130de839 100644 --- a/openpgp/read.go +++ b/openpgp/read.go @@ -515,12 +515,15 @@ func CheckArmoredDetachedSignature(keyring KeyRing, signed, signature io.Reader, } // checkSignatureDetails returns an error if: +// - The signature (or one of the binding signatures mentioned below) +// has a unknown critical notation data subpacket // - The primary key of the signing entity is revoked // The signature was signed by a subkey and: // - The signing subkey is revoked // - The primary identity is revoked // - The signature is expired -// - The primary key of the signing entity is expired according to the primary identity binding signature +// - The primary key of the signing entity is expired according to the +// primary identity binding signature // The signature was signed by a subkey and: // - The signing subkey is expired according to the subkey binding signature // - The signing subkey binding signature is expired @@ -534,20 +537,30 @@ func CheckArmoredDetachedSignature(keyring KeyRing, signed, signature io.Reader, func checkSignatureDetails(key *Key, signature *packet.Signature, config *packet.Config) error { now := config.Now() primaryIdentity := key.Entity.PrimaryIdentity() + signedBySubKey := key.PublicKey != key.Entity.PrimaryKey sigsToCheck := []*packet.Signature{ signature, primaryIdentity.SelfSignature } + if signedBySubKey { + sigsToCheck = append(sigsToCheck, key.SelfSignature, key.SelfSignature.EmbeddedSignature) + } + for _, sig := range sigsToCheck { + for _, not := range sig.Notations { + if not.IsCritical && !config.KnownNotation(not.Name) { + return errors.SignatureError("unknown critical notation: " + not.Name) + } + } + } if key.Entity.Revoked(now) || // primary key is revoked - (key.PublicKey != key.Entity.PrimaryKey && key.Revoked(now)) || // subkey is revoked + (signedBySubKey && key.Revoked(now)) || // subkey is revoked primaryIdentity.Revoked(now) { // primary identity is revoked return errors.ErrKeyRevoked } if key.Entity.PrimaryKey.KeyExpired(primaryIdentity.SelfSignature, now) { // primary key is expired return errors.ErrKeyExpired } - if key.PublicKey != key.Entity.PrimaryKey { + if signedBySubKey { if key.PublicKey.KeyExpired(key.SelfSignature, now) { // subkey is expired return errors.ErrKeyExpired } - sigsToCheck = append(sigsToCheck, key.SelfSignature, key.SelfSignature.EmbeddedSignature) } for _, sig := range sigsToCheck { if sig.SigExpired(now) { // any of the relevant signatures are expired diff --git a/openpgp/read_test.go b/openpgp/read_test.go index c9d1f334..10c92b78 100644 --- a/openpgp/read_test.go +++ b/openpgp/read_test.go @@ -435,6 +435,63 @@ func TestDetachedSignatureExpiredCrossSig(t *testing.T) { } } +func TestSignatureUnknownNotation(t *testing.T) { + el, err := ReadArmoredKeyRing(bytes.NewBufferString(criticalNotationSigner)) + if err != nil { + t.Error(err) + } + raw, err := armor.Decode(strings.NewReader(signedMessageWithCriticalNotation)) + if err != nil { + t.Error(err) + return + } + md, err := ReadMessage(raw.Body, el, nil, nil) + if err != nil { + t.Error(err) + return + } + _, err = ioutil.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + const expectedErr string = "openpgp: invalid signature: unknown critical notation: test@example.com" + if md.SignatureError == nil || md.SignatureError.Error() != expectedErr { + t.Errorf("Expected error '%s', but got error '%s'", expectedErr, md.SignatureError) + } +} + +func TestSignatureKnownNotation(t *testing.T) { + el, err := ReadArmoredKeyRing(bytes.NewBufferString(criticalNotationSigner)) + if err != nil { + t.Error(err) + } + raw, err := armor.Decode(strings.NewReader(signedMessageWithCriticalNotation)) + if err != nil { + t.Error(err) + return + } + config := &packet.Config{ + KnownNotations: map[string]bool{ + "test@example.com": true, + }, + } + md, err := ReadMessage(raw.Body, el, nil, config) + if err != nil { + t.Error(err) + return + } + _, err = ioutil.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + if md.SignatureError != nil { + t.Error(md.SignatureError) + return + } +} + func TestReadingArmoredPrivateKey(t *testing.T) { el, err := ReadArmoredKeyRing(bytes.NewBufferString(armoredPrivateKeyBlock)) if err != nil { diff --git a/openpgp/read_write_test_data.go b/openpgp/read_write_test_data.go index d65a3215..a5224424 100644 --- a/openpgp/read_write_test_data.go +++ b/openpgp/read_write_test_data.go @@ -239,3 +239,36 @@ ITEG9mMgp3TGS9ZzSifMZ8UGtHdp9QdBg8NEVPFzDOMGxpc/Bftav7RRRuPiAER+ =aQkm -----END PGP SIGNATURE----- ` + +const signedMessageWithCriticalNotation = `-----BEGIN PGP MESSAGE----- + +owGbwMvMwMH4oOW7S46CznTG09xJDDE3Wl1KUotLuDousDAwcjBYiSmyXL+48d6x +U1PSGUxcj8IUszKBVMpMaWAAAgEGZpAeh9SKxNyCnFS95PzcytRiBi5OAZjyXXzM +f8WYLqv7TXP61Sa4rqT12CI3xaN73YS2pt089f96odCKaEPnWJ3iSGmzJaW/ug10 +2Zo8Wj2k4s7t8wt4H3HtTu+y5UZfV3VOO+l//sdE/o+Lsub8FZH7/eOq7OnbNp4n +vwjE8mqJXetNMfj8r2SCyvkEnlVRYR+/mnge+ib56FdJ8uKtqSxyvgA= +=fRXs +-----END PGP MESSAGE-----` + +const criticalNotationSigner = `-----BEGIN PGP PUBLIC KEY BLOCK----- + +mI0EUmEvTgEEANyWtQQMOybQ9JltDqmaX0WnNPJeLILIM36sw6zL0nfTQ5zXSS3+ +fIF6P29lJFxpblWk02PSID5zX/DYU9/zjM2xPO8Oa4xo0cVTOTLj++Ri5mtr//f5 +GLsIXxFrBJhD/ghFsL3Op0GXOeLJ9A5bsOn8th7x6JucNKuaRB6bQbSPABEBAAG0 +JFRlc3QgTWNUZXN0aW5ndG9uIDx0ZXN0QGV4YW1wbGUuY29tPoi5BBMBAgAjBQJS +YS9OAhsvBwsJCAcDAgEGFQgCCQoLBBYCAwECHgECF4AACgkQSmNhOk1uQJQwDAP6 +AgrTyqkRlJVqz2pb46TfbDM2TDF7o9CBnBzIGoxBhlRwpqALz7z2kxBDmwpQa+ki +Bq3jZN/UosY9y8bhwMAlnrDY9jP1gdCo+H0sD48CdXybblNwaYpwqC8VSpDdTndf +9j2wE/weihGp/DAdy/2kyBCaiOY1sjhUfJ1GogF49rC4jQRSYS9OAQQA6R/PtBFa +JaT4jq10yqASk4sqwVMsc6HcifM5lSdxzExFP74naUMMyEsKHP53QxTF0Grqusag +Qg/ZtgT0CN1HUM152y7ACOdp1giKjpMzOTQClqCoclyvWOFB+L/SwGEIJf7LSCEr +woBuJifJc8xAVr0XX0JthoW+uP91eTQ3XpsAEQEAAYkBPQQYAQIACQUCUmEvTgIb +LgCoCRBKY2E6TW5AlJ0gBBkBAgAGBQJSYS9OAAoJEOCE90RsICyXuqIEANmmiRCA +SF7YK7PvFkieJNwzeK0V3F2lGX+uu6Y3Q/Zxdtwc4xR+me/CSBmsURyXTO29OWhP +GLszPH9zSJU9BdDi6v0yNprmFPX/1Ng0Abn/sCkwetvjxC1YIvTLFwtUL/7v6NS2 +bZpsUxRTg9+cSrMWWSNjiY9qUKajm1tuzPDZXAUEAMNmAN3xXN/Kjyvj2OK2ck0X +W748sl/tc3qiKPMJ+0AkMF7Pjhmh9nxqE9+QCEl7qinFqqBLjuzgUhBU4QlwX1GD +AtNTq6ihLMD5v1d82ZC7tNatdlDMGWnIdvEMCv2GZcuIqDQ9rXWs49e7tq1NncLY +hz3tYjKhoFTKEIq3y3Pp +=h/aX +-----END PGP PUBLIC KEY BLOCK-----`