Skip to content

Commit

Permalink
Add Notation Data support in subpackets (#33)
Browse files Browse the repository at this point in the history
This commit adds support for parsing and serializing Notation Data
signature subpackets.

It adds a `Notations []*Notation` field to `Signature`, with

	type Notation struct {
		Name            string
		Value           []byte
		IsCritical      bool
		IsHumanReadable bool
	}

Additionally, it adds a `KnownNotations map[string]bool` field to
`Config`, which specifies the notation names that are allowed to be
present in critical Notation Data signature subpackets.

---------

Co-authored-by: Daniel Huigens <[email protected]>
Co-authored-by: marinthiercelin <[email protected]>
  • Loading branch information
3 people authored Jan 31, 2023
1 parent f7f10de commit 34c1fc3
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 4 deletions.
63 changes: 63 additions & 0 deletions openpgp/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,69 @@ func TestNewEntityPrivateSerialization(t *testing.T) {
}
}

func TestNotationPacket(t *testing.T) {
keys, err := ReadArmoredKeyRing(bytes.NewBufferString(keyWithNotation))
if err != nil {
t.Fatal(err)
}

assertNotationPackets(t, keys)

serializedEntity := bytes.NewBuffer(nil)
err = keys[0].SerializePrivate(serializedEntity, nil)
if err != nil {
t.Fatal(err)
}

keys, err = ReadKeyRing(serializedEntity)
if err != nil {
t.Fatal(err)
}

assertNotationPackets(t, keys)
}

func assertNotationPackets(t *testing.T, keys EntityList) {
if len(keys) != 1 {
t.Errorf("Failed to accept key, %d", len(keys))
}

identity := keys[0].Identities["Test <[email protected]>"]

if numSigs, numExpected := len(identity.Signatures), 1; numSigs != numExpected {
t.Fatalf("got %d signatures, expected %d", numSigs, numExpected)
}

notations := identity.Signatures[0].Notations
if numSigs, numExpected := len(notations), 2; numSigs != numExpected {
t.Fatalf("got %d Data Notation subpackets, expected %d", numSigs, numExpected)
}

if notations[0].IsHumanReadable != true {
t.Fatalf("got false, expected true")
}

if notations[0].Name != "[email protected]" {
t.Fatalf("got %s, expected [email protected]", notations[0].Name)
}

if string(notations[0].Value) != "test" {
t.Fatalf("got %s, expected 2", string(notations[0].Value))
}

if notations[1].IsHumanReadable != false {
t.Fatalf("got true, expected false")
}

if notations[1].Name != "[email protected]" {
t.Fatalf("got %s, expected [email protected]", notations[1].Name)
}

if !bytes.Equal(notations[1].Value, []byte{0, 1, 2, 3}) {
t.Fatalf("got %s, expected 3", string(notations[1].Value))
}
}

func TestEntityPrivateSerialization(t *testing.T) {
keys, err := ReadArmoredKeyRing(bytes.NewBufferString(armoredPrivateKeyBlock))
if err != nil {
Expand Down
18 changes: 18 additions & 0 deletions openpgp/keys_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,3 +518,21 @@ XLCBln+wdewpU4ChEffMUDRBfqfQco/YsMqWV7bHJHAO0eC/DMKCjyU90xdH7R/d
QgqsfguR1PqPuJxpXV4bSr6CGAAAAA==
=MSvh
-----END PGP PRIVATE KEY BLOCK-----`

const keyWithNotation = `-----BEGIN PGP PRIVATE KEY BLOCK-----
xVgEY9gIshYJKwYBBAHaRw8BAQdAF25fSM8OpFlXZhop4Qpqo5ywGZ4jgWlR
ppjhIKDthREAAQC+LFpzFcMJYcjxGKzBGHN0Px2jU4d04YSRnFAik+lVVQ6u
zRdUZXN0IDx0ZXN0QGV4YW1wbGUuY29tPsLACgQQFgoAfAUCY9gIsgQLCQcI
CRD/utJOCym8pR0UgAAAAAAQAAR0ZXh0QGV4YW1wbGUuY29tdGVzdB8UAAAA
AAASAARiaW5hcnlAZXhhbXBsZS5jb20AAQIDAxUICgQWAAIBAhkBAhsDAh4B
FiEEEMCQTUVGKgCX5rDQ/7rSTgspvKUAAPl5AP9Npz90LxzrB97Qr2DrGwfG
wuYn4FSYwtuPfZHHeoIabwD/QEbvpQJ/NBb9EAZuow4Rirlt1yv19mmnF+j5
8yUzhQjHXQRj2AiyEgorBgEEAZdVAQUBAQdARXAo30DmKcyUg6co7OUm0RNT
z9iqFbDBzA8A47JEt1MDAQgHAAD/XKK3lBm0SqMR558HLWdBrNG6NqKuqb5X
joCML987ZNgRD8J4BBgWCAAqBQJj2AiyCRD/utJOCym8pQIbDBYhBBDAkE1F
RioAl+aw0P+60k4LKbylAADRxgEAg7UfBDiDPp5LHcW9D+SgFHk6+GyEU4ev
VppQxdtxPvAA/34snHBX7Twnip1nMt7P4e2hDiw/hwQ7oqioOvc6jMkP
=Z8YJ
-----END PGP PRIVATE KEY BLOCK-----
`
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]
}
29 changes: 29 additions & 0 deletions openpgp/packet/notation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package packet

// Notation type represents a Notation Data subpacket
// see https://tools.ietf.org/html/rfc4880#section-5.2.3.16
type Notation struct {
Name string
Value []byte
IsCritical bool
IsHumanReadable bool
}

func (notation *Notation) getData() []byte {
nameData := []byte(notation.Name)
nameLen := len(nameData)
valueLen := len(notation.Value)

data := make([]byte, 8+nameLen+valueLen)
if notation.IsHumanReadable {
data[0] = 0x80
}

data[4] = byte(nameLen >> 8)
data[5] = byte(nameLen)
data[6] = byte(valueLen >> 8)
data[7] = byte(valueLen)
copy(data[8:8+nameLen], nameData)
copy(data[8+nameLen:], notation.Value)
return data
}
38 changes: 38 additions & 0 deletions openpgp/packet/notation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package packet

import (
"bytes"
"testing"
)

func TestNotationGetData(t *testing.T) {
notation := Notation{
Name: "[email protected]",
Value: []byte("test-value"),
IsCritical: true,
IsHumanReadable: true,
}
expected := []byte{0x80, 0, 0, 0, 0, 14, 0, 10}
expected = append(expected, []byte(notation.Name)...)
expected = append(expected, []byte(notation.Value)...)
data := notation.getData()
if !bytes.Equal(expected, data) {
t.Fatalf("Expected %s, got %s", expected, data)
}
}

func TestNotationGetDataNotHumanReadable(t *testing.T) {
notation := Notation{
Name: "[email protected]",
Value: []byte("test-value"),
IsCritical: true,
IsHumanReadable: false,
}
expected := []byte{0, 0, 0, 0, 0, 14, 0, 10}
expected = append(expected, []byte(notation.Name)...)
expected = append(expected, []byte(notation.Value)...)
data := notation.getData()
if !bytes.Equal(expected, data) {
t.Fatalf("Expected %s, got %s", expected, data)
}
}
38 changes: 38 additions & 0 deletions openpgp/packet/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type Signature struct {
IssuerFingerprint []byte
SignerUserId *string
IsPrimaryId *bool
Notations []*Notation

// TrustLevel and TrustAmount can be set by the signer to assert that
// the key is not only valid but also trustworthy at the specified
Expand Down Expand Up @@ -248,6 +249,7 @@ const (
keyExpirationSubpacket signatureSubpacketType = 9
prefSymmetricAlgosSubpacket signatureSubpacketType = 11
issuerSubpacket signatureSubpacketType = 16
notationDataSubpacket signatureSubpacketType = 20
prefHashAlgosSubpacket signatureSubpacketType = 21
prefCompressionSubpacket signatureSubpacketType = 22
primaryUserIdSubpacket signatureSubpacketType = 25
Expand Down Expand Up @@ -367,6 +369,31 @@ func parseSignatureSubpacket(sig *Signature, subpacket []byte, isHashed bool) (r
}
sig.IssuerKeyId = new(uint64)
*sig.IssuerKeyId = binary.BigEndian.Uint64(subpacket)
case notationDataSubpacket:
// Notation data, section 5.2.3.16
if !isHashed {
return
}
if len(subpacket) < 8 {
err = errors.StructuralError("notation data subpacket with bad length")
return
}

nameLength := uint32(subpacket[4])<<8 | uint32(subpacket[5])
valueLength := uint32(subpacket[6])<<8 | uint32(subpacket[7])
if len(subpacket) != int(nameLength) + int(valueLength) + 8 {
err = errors.StructuralError("notation data subpacket with bad length")
return
}

notation := Notation{
IsHumanReadable: (subpacket[0] & 0x80) == 0x80,
Name: string(subpacket[8: (nameLength + 8)]),
Value: subpacket[(nameLength + 8) : (valueLength + nameLength + 8)],
IsCritical: isCritical,
}

sig.Notations = append(sig.Notations, &notation)
case prefHashAlgosSubpacket:
// Preferred hash algorithms, section 5.2.3.8
if !isHashed {
Expand Down Expand Up @@ -938,6 +965,17 @@ func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubp
subpackets = append(subpackets, outputSubpacket{true, keyFlagsSubpacket, false, []byte{flags}})
}

for _, notation := range sig.Notations {
subpackets = append(
subpackets,
outputSubpacket{
true,
notationDataSubpacket,
notation.IsCritical,
notation.getData(),
})
}

// The following subpackets may only appear in self-signatures.

var features = byte(0x00)
Expand Down
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 _, notation := range sig.Notations {
if notation.IsCritical && !config.KnownNotation(notation.Name) {
return errors.SignatureError("unknown critical notation: " + notation.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
Loading

0 comments on commit 34c1fc3

Please sign in to comment.