Skip to content

Commit

Permalink
Add config.KnownNotations and check critical notations
Browse files Browse the repository at this point in the history
  • Loading branch information
twiss committed Jan 30, 2023
1 parent 08684b7 commit 73a6b8f
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 4 deletions.
11 changes: 11 additions & 0 deletions openpgp/packet/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]
}
21 changes: 17 additions & 4 deletions openpgp/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
57 changes: 57 additions & 0 deletions openpgp/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: [email protected]"
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{
"[email protected]": 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 {
Expand Down
33 changes: 33 additions & 0 deletions openpgp/read_write_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-----`

0 comments on commit 73a6b8f

Please sign in to comment.