Allow addition of gpg keyring with multiple keys (#12487)

Related #6778

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lauris BH <lauris@nix.lv>

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
 
-	if err = addGPGKey(sess, key, content); err != nil {
-		return nil, err
-	}
+		key, err := parseGPGKey(ownerID, ekey)
+		if err != nil {
+			return nil, err
+		}
 
-	return key, sess.Commit()
+		if err = addGPGKey(sess, key, content); err != nil {
+			return nil, err
+		}
+		keys = append(keys, key)
+	}
+	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

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)
 }

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

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)