Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/crypto/openpgp: cannot read keys with revoked user ids #25850

Closed
aviau opened this issue Jun 12, 2018 · 4 comments
Closed

x/crypto/openpgp: cannot read keys with revoked user ids #25850

aviau opened this issue Jun 12, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@aviau
Copy link

aviau commented Jun 12, 2018 •
edited

Hello,

I am getting the following error when reading a PGP Key:

openpgp: invalid data: user ID packet not followed by self-signature

It contains one revoked user ID:

[ultimate] (1). Golang Gopher <no-reply@golang.com>
[ revoked] (2)  Golang Gopher <revoked@golang.com>

This was fixed in keybase's go-crypto fork in the following changeset: keybase/go-crypto#5

Example code:

package main

import (
	"fmt"
	"strings"

	"golang.org/x/crypto/openpgp"
)

func main() {
	_, err := openpgp.ReadArmoredKeyRing(
		strings.NewReader(key),
	)
	fmt.Println(err)
}

const key = `
-----BEGIN PGP PUBLIC KEY BLOCK-----

mQENBFsgO5EBCADhREPmcjsPkXe1z7ctvyWL0S7oa9JaoGZ9oPDHFDlQxd0qlX2e
DZJZDg0qYvVixmaULIulApq1puEsaJCn3lHUbHlb4PYKwLEywYXM28JN91KtLsz/
uaEX2KC5WqeP40utmzkNLq+oRX/xnRMgwbO7yUNVG2UlEa6eI+xOXO3YtLdmJMBW
ClQ066ZnOIzEo1JxnIwha1CDBMWLLfOLrg6l8InUqaXbtEBbnaIYO6fXVXELUjkx
nmk7t/QOk0tXCy8muH9UDqJkwDUESY2l79XwBAcx9riX8vY7vwC34pm22fAUVLCJ
x1SJx0J8bkeNp38jKM2Zd9SUQqSbfBopQ4pPABEBAAG0I0dvbGFuZyBHb3BoZXIg
PG5vLXJlcGx5QGdvbGFuZy5jb20+iQFUBBMBCgA+FiEE5Ik5JLcNx6l6rZfw1oFy
9I6cUoMFAlsgO5ECGwMFCQPCZwAFCwkIBwMFFQoJCAsFFgIDAQACHgECF4AACgkQ
1oFy9I6cUoMIkwf8DNPeD23i4jRwd/pylbvxwZintZl1fSwTJW1xcOa1emXaEtX2
depuqhP04fjlRQGfsYAQh7X9jOJxAHjTmhqFBi5sD7QvKU00cPFYbJ/JTx0B41bl
aXnSbGhRPh63QtEZL7ACAs+shwvvojJqysx7kyVRu0EW2wqjXdHwR/SJO6nhNBa2
DXzSiOU/SUA42mmG+5kjF8Aabq9wPwT9wjraHShEweNerNMmOqJExBOy3yFeyDpa
XwEZFzBfOKoxFNkIaVf5GSdIUGhFECkGvBMB935khftmgR8APxdU4BE7XrXexFJU
8RCuPXonm4WQOwTWR0vQg64pb2WKAzZ8HhwTGbQiR29sYW5nIEdvcGhlciA8cmV2
b2tlZEBnb2xhbmcuY29tPokBNgQwAQoAIBYhBOSJOSS3Dcepeq2X8NaBcvSOnFKD
BQJbIDv3Ah0AAAoJENaBcvSOnFKDfWMIAKhI/Tvu3h8fSUxp/gSAcduT6bC1JttG
0lYQ5ilKB/58lBUA5CO3ZrKDKlzW3M8VEcvohVaqeTMKeoQd5rCZq8KxHn/KvN6N
s85REfXfniCKfAbnGgVXX3kDmZ1g63pkxrFu0fDZjVDXC6vy+I0sGyI/Inro0Pzb
tvn0QCsxjapKK15BtmSrpgHgzVqVg0cUp8vqZeKFxarYbYB2idtGRci4b9tObOK0
BSTVFy26+I/mrFGaPrySYiy2Kz5NMEcRhjmTxJ8jSwEr2O2sUR0yjbgUAXbTxDVE
/jg5fQZ1ACvBRQnB7LvMHcInbzjyeTM3FazkkSYQD6b97+dkWwb1iWG5AQ0EWyA7
kQEIALkg04REDZo1JgdYV4x8HJKFS4xAYWbIva1ZPqvDNmZRUbQZR2+gpJGEwn7z
VofGvnOYiGW56AS5j31SFf5kro1+1bZQ5iOONBng08OOo58/l1hRseIIVGB5TGSa
PCdChKKHreJI6hS3mShxH6hdfFtiZuB45rwoaArMMsYcjaezLwKeLc396cpUwwcZ
snLUNd1Xu5EWEF2OdFkZ2a1qYdxBvAYdQf4+1Nr+NRIx1u1NS9c8jp3PuMOkrQEi
bNtc1v6v0Jy52mKLG4y7mC/erIkvkQBYJdxPaP7LZVaPYc3/xskcyijrJ/5ufoD8
K71/ShtsZUXSQn9jlRaYR0EbojMAEQEAAYkBPAQYAQoAJhYhBOSJOSS3Dcepeq2X
8NaBcvSOnFKDBQJbIDuRAhsMBQkDwmcAAAoJENaBcvSOnFKDkFMIAIt64bVZ8x7+
TitH1bR4pgcNkaKmgKoZz6FXu80+SnbuEt2NnDyf1cLOSimSTILpwLIuv9Uft5Pb
OraQbYt3xi9yrqdKqGLv80bxqK0NuryNkvh9yyx5WoG1iKqMj9/FjGghuPrRaT4l
QinNAghGVkEy1+aXGFrG2DsOC1FFI51CC2WVTzZ5RwR2GpiNRfESsU1rZAUqf/2V
yJl9bD5R4SUNy8oQmhOxi+gbhD4Ao34e4W0ilibslI/uawvCiOwlu5NGd8zv5n+U
heiQvzkApQup5c+BhH5zFDFdKJ2CBByxw9+7QjMFI/wgLixKuE0Ob2kAokXf7RlB
7qTZOahrETw=
=IKnw
-----END PGP PUBLIC KEY BLOCK-----
`

Go version: go version go1.10.1 linux/amd64
@gopherbot gopherbot added this to the Unreleased milestone Jun 12, 2018
@aviau aviau changed the title x/crypto user ID packet not followed by self-signature x/crypto/openpgp user ID packet not followed by self-signature Jun 12, 2018
@bcmills
Copy link
Contributor

bcmills commented Jun 12, 2018

CC @FiloSottile

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 12, 2018
@aviau aviau mentioned this issue Jun 12, 2018
Closed
@gopherbot
Copy link

Change https://golang.org/cl/118376 mentions this issue: openpgp: read keys with uid packet not followed by self-sig

@aviau
Copy link
Author

aviau commented Jun 12, 2018 •
edited

With the author's permission, I have adapted Keybase's changes to the current code base and created a test.

@aviau aviau changed the title x/crypto/openpgp user ID packet not followed by self-signature x/crypto/openpgp allow reading keys with revoked user ids Jun 12, 2018
@aviau aviau changed the title x/crypto/openpgp allow reading keys with revoked user ids x/crypto/openpgp cannot read keys with revoked user ids Jun 12, 2018
@aviau
Copy link
Author

aviau commented Jun 13, 2018 •
edited

I have added @FiloSottile to the reviewers on my changeset. This is my first contribution so let me know if there is something else I need to do to get things going.

@FiloSottile FiloSottile changed the title x/crypto/openpgp cannot read keys with revoked user ids x/crypto/openpgp: cannot read keys with revoked user ids Jun 13, 2018
lucab added a commit to lucab/torcx that referenced this issue Jun 21, 2018
@lucab
glide: bump x/crypto to latest
3f2d76f 
This is to hopefully fix buggy parsing of public keys, see
golang/go#25850
@lucab lucab mentioned this issue Jun 21, 2018
Merged
zapu pushed a commit to keybase/go-crypto that referenced this issue Aug 2, 2018
@aviau @zapu
openpgp: read keys with revoked user ids
e368a21 
We were already supported revoked user IDs, but cherry-pick the test
from mainline.

Original commit:

The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
zapu pushed a commit to keybase/go-crypto that referenced this issue Aug 2, 2018
@aviau @zapu
openpgp: read keys with revoked user ids (test only)
567252f 
We were already supported revoked user IDs, but cherry-pick the test
from mainline.

Original commit:

The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
zapu pushed a commit to keybase/go-crypto that referenced this issue Aug 6, 2018
@aviau @zapu
openpgp: read keys with revoked user ids (test only)
2c2b831 
We were already supported revoked user IDs, but cherry-pick the test
from mainline.

Original commit:

The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 14, 2019
chintanparikh pushed a commit to opendoor-labs/openpgp that referenced this issue Dec 11, 2019
@aviau @FiloSottile
openpgp: read keys with revoked user ids
f1c3cb6 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@aviau @FiloSottile
openpgp: read keys with revoked user ids
e63ed03 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@aviau @c-expert-zigbee
openpgp: read keys with revoked user ids
6bc4fb0 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@aviau @c-expert-zigbee
openpgp: read keys with revoked user ids
f03b17c 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@aviau @FiloSottile
openpgp: read keys with revoked user ids
82dd390 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@aviau @FiloSottile
openpgp: read keys with revoked user ids
faed15c 
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.

See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.

This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.

Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.

In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.

Fixes golang/go#25850

Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@aviau @bcmills @gopherbot