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

Skip other people signatures only if they have thrown an error #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Alkorin
Copy link

@Alkorin Alkorin commented May 10, 2018

Do not skip valid signatures of other people

@maxtaco maxtaco requested a review from zapu May 31, 2018 20:13
@Alkorin
Copy link
Author

Alkorin commented Jan 23, 2019

Is it possible to take a look ?

@zapu
Copy link

zapu commented Jan 23, 2019

We did, way back when it was submitted. Sorry I didn't leave a comment.

We typically consider signatures issued by 3rd party (i.e. "signatures by other people") there because we only look for signatures made by the primary key itself, to validate integrity of the PGP bundle. If you were to read 3rd party signatures, the internal representation would have to change and they should be handled separately from "regular" signatures, so they are never confused.

Can you explain what issue were you trying to fix?

Let me know if I misunderstood the intent here. Thank you

@Alkorin
Copy link
Author

Alkorin commented Jan 23, 2019

We use it to establish a chain of trust.

We have a KeySigningKey stored on an offline media which sign other gpg keys used in our system.

On client side, applications only knows the public key of the KeySigningKey and can check that the incoming signature is signed by a key which is signed by the KSK.
So in my model, "signatures by other people" is the signature done by the KSK.

Except if I missed something, everything is already there in the lib. The only issue I have is that such signatures are just dropped when the key is loaded.

Sample KSK:

-----BEGIN PGP PUBLIC KEY BLOCK-----

mDMEXEh7IBYJKwYBBAHaRw8BAQdAz9arvnAHltrlhkIkVhI/JcZ7IkRG7NjNADVY
ZmnoJ0y0IUtleSBTaWduaW5nIEtleSA8a3NrQGV4YW1wbGUub3JnPoiQBBMWCAA4
FiEEgwSUTVFDHiYx1enqZ+77a61hH6MFAlxIeyACGwMFCwkIBwIGFQgJCgsCBBYC
AwECHgECF4AACgkQZ+77a61hH6PORgEAzBevTrt0z52BsLQA+FSlKPuw2lL8oGxD
uGaOAZw7N2IBAKflzeXMYuvTVP+Hm8PJVQHPpKK3ZepZUsZ4s5TOhNEN
=XRaB
-----END PGP PUBLIC KEY BLOCK-----

Sample "Other Key" signed by KSK:

-----BEGIN PGP PUBLIC KEY BLOCK-----

mDMEXEh7hxYJKwYBBAHaRw8BAQdAlgqOPVz5cW0F6t9gnp9A9o/a8+DbytGlChSp
UcDLJ0K0IE90aGVyIEtleSA8b3RoZXJrZXlAZXhhbXBsZS5vcmc+iJAEExYIADgW
IQQmjNDC+cMmHB6YHlXpNHjmFyzVZAUCXEh7hwIbAwULCQgHAgYVCAkKCwIEFgID
AQIeAQIXgAAKCRDpNHjmFyzVZEleAQDl/Iaen8u912WPx2SxYBQRLB9YAa8pz5r8
W6TzScoqNAEA/dnaFMUnfnhKlofncaiXwLgz//LnO2DRp/gy7M1efgaIewQQFggA
IxYhBIMElE1RQx4mMdXp6mfu+2utYR+jBQJcSHvnBYMAJ40AAAoJEGfu+2utYR+j
lHABANRmHB4ikLTRoQ+ZeHK7dBDBAcH6TN5mxxzvVaW/SDjxAPwJkO09Oq6opTzU
Cxll1/+qr0f0yOkchM/+bluEkHp7DQ==
=yqMf
-----END PGP PUBLIC KEY BLOCK-----

How the signature is made:

gpg -u "Key Signing Key" --ask-cert-expire --sign-key "Other Key"

Simple parsing with github.com/keybase/go-crypto/openpgp

    p, _ := armor.Decode(bytes.NewBufferString(other_key))
    e, _ := openpgp.ReadEntity(packet.NewReader(p.Body))

    fmt.Printf("%+v\n", e.Identities["Other Key <[email protected]>"])

Without the patch:

&{Name:Other Key <[email protected]> UserId:0xc00004c080 SelfSignature:0xc0000a8000 Signatures:[] Revocation:<nil>}

I only have the self signature

With the patch:

&{Name:Other Key <[email protected]> UserId:0xc00004a080 SelfSignature:0xc0000a8000 Signatures:[0xc0000a8280] Revocation:<nil>}

I also have the signature made by the KSK

And in my model, I consider that a GPG PublicKey (k in the code below) is signed by the KSK GPG PublicKey (signerKey) if at least one Identity of this key have a signature done by the KSK:

type PublicKey struct {
    *openpgp.Entity
}

func (k PublicKey) IsSignedBy(signerKey *PublicKey) *packet.Signature {
    for _, i := range k.Identities {
        for _, s := range i.Signatures {
            // Skip signature if IssuerKeyId doesn't match signer's key
            if s.IssuerKeyId != nil && *s.IssuerKeyId != signerKey.PrimaryKey.KeyId {
                continue
            }
            // Return true if signature s is valid
            if err := signerKey.PrimaryKey.VerifyUserIdSignature(i.Name, k.PrimaryKey, s); err == nil {
                return s
            }
        }
    }
    return nil
}

That's why I need Identities.Signatures. And I extract also the signature CreationDate & signature SigLifetimeSecs to check signature expiration and revoke old signing keys.

Hope it will give your more infos on what I try to do with your library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants