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

Allow addition of gpg keyring with multiple keys #12487

Merged
merged 2 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 67 additions & 59 deletions models/gpg_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -758,7 +764,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{
Expand All @@ -767,48 +773,50 @@ 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,
Verified: false,
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
Expand Down
10 changes: 6 additions & 4 deletions models/gpg_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, "[email protected]", key.Emails[0].Email)
}
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions routers/api/v1/user/gpg_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions routers/user/setting/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down