From 7c2cf236f8a54038e1688e6256e6c32b8b4e6913 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 21 Aug 2020 11:45:50 +0100 Subject: [PATCH] Allow addition of gpg keyring with multiple keys (#12487) Related #6778 Signed-off-by: Andrew Thornton Co-authored-by: Lauris BH --- models/gpg_key.go | 128 +++++++++++++++++---------------- models/gpg_key_test.go | 10 +-- routers/api/v1/user/gpg_key.go | 4 +- routers/user/setting/keys.go | 12 +++- 4 files changed, 86 insertions(+), 68 deletions(-) diff --git a/models/gpg_key.go b/models/gpg_key.go index 9e52ca8cad..662eac939b 100644 --- a/models/gpg_key.go +++ b/models/gpg_key.go @@ -106,12 +106,12 @@ func GetGPGImportByKeyID(keyID string) (*GPGKeyImport, error) { // checkArmoredGPGKeyString checks if the given key string is a valid GPG armored key. // The function returns the actual public key on success -func checkArmoredGPGKeyString(content string) (*openpgp.Entity, error) { +func checkArmoredGPGKeyString(content string) (openpgp.EntityList, error) { list, err := openpgp.ReadArmoredKeyRing(strings.NewReader(content)) if err != nil { return nil, ErrGPGKeyParsing{err} } - return list[0], nil + return list, nil } //addGPGKey add key, import and subkeys to database @@ -152,38 +152,40 @@ func addGPGSubKey(e Engine, key *GPGKey) (err error) { } // AddGPGKey adds new public key to database. -func AddGPGKey(ownerID int64, content string) (*GPGKey, error) { - ekey, err := checkArmoredGPGKeyString(content) +func AddGPGKey(ownerID int64, content string) ([]*GPGKey, error) { + ekeys, err := checkArmoredGPGKeyString(content) if err != nil { return nil, err } - - // Key ID cannot be duplicated. - has, err := x.Where("key_id=?", ekey.PrimaryKey.KeyIdString()). - Get(new(GPGKey)) - if err != nil { - return nil, err - } else if has { - return nil, ErrGPGKeyIDAlreadyUsed{ekey.PrimaryKey.KeyIdString()} - } - - //Get DB session sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { return nil, err } + keys := make([]*GPGKey, 0, len(ekeys)) + for _, ekey := range ekeys { + // Key ID cannot be duplicated. + has, err := sess.Where("key_id=?", ekey.PrimaryKey.KeyIdString()). + Get(new(GPGKey)) + if err != nil { + return nil, err + } else if has { + return nil, ErrGPGKeyIDAlreadyUsed{ekey.PrimaryKey.KeyIdString()} + } - key, err := parseGPGKey(ownerID, ekey) - if err != nil { - return nil, err + //Get DB session + + key, err := parseGPGKey(ownerID, ekey) + if err != nil { + return nil, err + } + + if err = addGPGKey(sess, key, content); err != nil { + return nil, err + } + keys = append(keys, key) } - - if err = addGPGKey(sess, key, content); err != nil { - return nil, err - } - - return key, sess.Commit() + return keys, sess.Commit() } //base64EncPubKey encode public key content to base 64 @@ -221,7 +223,11 @@ func GPGKeyToEntity(k *GPGKey) (*openpgp.Entity, error) { if err != nil { return nil, err } - return checkArmoredGPGKeyString(impKey.Content) + keys, err := checkArmoredGPGKeyString(impKey.Content) + if err != nil { + return nil, err + } + return keys[0], err } //parseSubGPGKey parse a sub Key @@ -761,7 +767,7 @@ func verifyWithGPGSettings(gpgSettings *git.GPGSettings, sig *packet.Signature, } // Otherwise we have to parse the key - ekey, err := checkArmoredGPGKeyString(gpgSettings.PublicKeyContent) + ekeys, err := checkArmoredGPGKeyString(gpgSettings.PublicKeyContent) if err != nil { log.Error("Unable to get default signing key: %v", err) return &CommitVerification{ @@ -770,22 +776,9 @@ func verifyWithGPGSettings(gpgSettings *git.GPGSettings, sig *packet.Signature, Reason: "gpg.error.generate_hash", } } - pubkey := ekey.PrimaryKey - content, err := base64EncPubKey(pubkey) - if err != nil { - return &CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.generate_hash", - } - } - k := &GPGKey{ - Content: content, - CanSign: pubkey.CanSign(), - KeyID: pubkey.KeyIdString(), - } - for _, subKey := range ekey.Subkeys { - content, err := base64EncPubKey(subKey.PublicKey) + for _, ekey := range ekeys { + pubkey := ekey.PrimaryKey + content, err := base64EncPubKey(pubkey) if err != nil { return &CommitVerification{ CommittingUser: committer, @@ -793,25 +786,40 @@ func verifyWithGPGSettings(gpgSettings *git.GPGSettings, sig *packet.Signature, Reason: "gpg.error.generate_hash", } } - k.SubsKey = append(k.SubsKey, &GPGKey{ + k := &GPGKey{ Content: content, - CanSign: subKey.PublicKey.CanSign(), - KeyID: subKey.PublicKey.KeyIdString(), - }) - } - if commitVerification := hashAndVerifyWithSubKeys(sig, payload, k, committer, &User{ - Name: gpgSettings.Name, - Email: gpgSettings.Email, - }, gpgSettings.Email); commitVerification != nil { - return commitVerification - } - if keyID == k.KeyID { - // This is a bad situation ... We have a key id that matches our default key but the signature doesn't match. - return &CommitVerification{ - CommittingUser: committer, - Verified: false, - Warning: true, - Reason: BadSignature, + CanSign: pubkey.CanSign(), + KeyID: pubkey.KeyIdString(), + } + for _, subKey := range ekey.Subkeys { + content, err := base64EncPubKey(subKey.PublicKey) + if err != nil { + return &CommitVerification{ + CommittingUser: committer, + Verified: false, + Reason: "gpg.error.generate_hash", + } + } + k.SubsKey = append(k.SubsKey, &GPGKey{ + Content: content, + CanSign: subKey.PublicKey.CanSign(), + KeyID: subKey.PublicKey.KeyIdString(), + }) + } + if commitVerification := hashAndVerifyWithSubKeys(sig, payload, k, committer, &User{ + Name: gpgSettings.Name, + Email: gpgSettings.Email, + }, gpgSettings.Email); commitVerification != nil { + return commitVerification + } + if keyID == k.KeyID { + // This is a bad situation ... We have a key id that matches our default key but the signature doesn't match. + return &CommitVerification{ + CommittingUser: committer, + Verified: false, + Warning: true, + Reason: BadSignature, + } } } return nil diff --git a/models/gpg_key_test.go b/models/gpg_key_test.go index e2f92e7f81..92131f5976 100644 --- a/models/gpg_key_test.go +++ b/models/gpg_key_test.go @@ -102,7 +102,8 @@ Av844q/BfRuVsJsK1NDNG09LC30B0l3LKBqlrRmRTUMHtgchdX2dY+p7GPOoSzlR MkM/fdpyc2hY7Dl/+qFmN5MG5yGmMpQcX+RNNR222ibNC1D3wg== =i9b7 -----END PGP PUBLIC KEY BLOCK-----` - ekey, err := checkArmoredGPGKeyString(testGPGArmor) + keys, err := checkArmoredGPGKeyString(testGPGArmor) + ekey := keys[0] assert.NoError(t, err, "Could not parse a valid GPG armored key", ekey) pubkey := ekey.PrimaryKey @@ -219,9 +220,9 @@ Q0KHb+QcycSgbDx0ZAvdIacuKvBBcbxrsmFUI4LR+oIup0G9gUc0roPvr014jYQL =zHo9 -----END PGP PUBLIC KEY BLOCK-----` - key, err := AddGPGKey(1, testEmailWithUpperCaseLetters) + keys, err := AddGPGKey(1, testEmailWithUpperCaseLetters) assert.NoError(t, err) - + key := keys[0] if assert.Len(t, key.Emails, 1) { assert.Equal(t, "user1@example.com", key.Emails[0].Email) } @@ -371,8 +372,9 @@ epiDVQ== =VSKJ -----END PGP PUBLIC KEY BLOCK----- ` - ekey, err := checkArmoredGPGKeyString(testIssue6599) + keys, err := checkArmoredGPGKeyString(testIssue6599) assert.NoError(t, err) + ekey := keys[0] expire := getExpiryTime(ekey) assert.Equal(t, time.Unix(1586105389, 0), expire) } diff --git a/routers/api/v1/user/gpg_key.go b/routers/api/v1/user/gpg_key.go index 9fbe351c27..7290ef485a 100644 --- a/routers/api/v1/user/gpg_key.go +++ b/routers/api/v1/user/gpg_key.go @@ -118,12 +118,12 @@ func GetGPGKey(ctx *context.APIContext) { // CreateUserGPGKey creates new GPG key to given user by ID. func CreateUserGPGKey(ctx *context.APIContext, form api.CreateGPGKeyOption, uid int64) { - key, err := models.AddGPGKey(uid, form.ArmoredKey) + keys, err := models.AddGPGKey(uid, form.ArmoredKey) if err != nil { HandleAddGPGKeyError(ctx, err) return } - ctx.JSON(http.StatusCreated, convert.ToGPGKey(key)) + ctx.JSON(http.StatusCreated, convert.ToGPGKey(keys[0])) } // swagger:parameters userCurrentPostGPGKey diff --git a/routers/user/setting/keys.go b/routers/user/setting/keys.go index 73ae73da40..a7978fe14e 100644 --- a/routers/user/setting/keys.go +++ b/routers/user/setting/keys.go @@ -41,7 +41,7 @@ func KeysPost(ctx *context.Context, form auth.AddKeyForm) { } switch form.Type { case "gpg": - key, err := models.AddGPGKey(ctx.User.ID, form.Content) + keys, err := models.AddGPGKey(ctx.User.ID, form.Content) if err != nil { ctx.Data["HasGPGError"] = true switch { @@ -63,7 +63,15 @@ func KeysPost(ctx *context.Context, form auth.AddKeyForm) { } return } - ctx.Flash.Success(ctx.Tr("settings.add_gpg_key_success", key.KeyID)) + keyIDs := "" + for _, key := range keys { + keyIDs += key.KeyID + keyIDs += ", " + } + if len(keyIDs) > 0 { + keyIDs = keyIDs[:len(keyIDs)-2] + } + ctx.Flash.Success(ctx.Tr("settings.add_gpg_key_success", keyIDs)) ctx.Redirect(setting.AppSubURL + "/user/settings/keys") case "ssh": content, err := models.CheckPublicKeyString(form.Content)