Skip to content

Commit 7eb8daf

Browse files
authored
Use fingerprint to check instead content for public key (#911)
* use fingerprint to check instead content for public key * add fingerprint field for ErrKeyAlreadyExist
1 parent 55ae782 commit 7eb8daf

File tree

2 files changed

+47
-26
lines changed

2 files changed

+47
-26
lines changed

Diff for: models/error.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,9 @@ func (err ErrKeyNotExist) Error() string {
213213

214214
// ErrKeyAlreadyExist represents a "KeyAlreadyExist" kind of error.
215215
type ErrKeyAlreadyExist struct {
216-
OwnerID int64
217-
Content string
216+
OwnerID int64
217+
Fingerprint string
218+
Content string
218219
}
219220

220221
// IsErrKeyAlreadyExist checks if an error is a ErrKeyAlreadyExist.
@@ -224,7 +225,8 @@ func IsErrKeyAlreadyExist(err error) bool {
224225
}
225226

226227
func (err ErrKeyAlreadyExist) Error() string {
227-
return fmt.Sprintf("public key already exists [owner_id: %d, content: %s]", err.OwnerID, err.Content)
228+
return fmt.Sprintf("public key already exists [owner_id: %d, finter_print: %s, content: %s]",
229+
err.OwnerID, err.Fingerprint, err.Content)
228230
}
229231

230232
// ErrKeyNameAlreadyUsed represents a "KeyNameAlreadyUsed" kind of error.

Diff for: models/ssh_key.go

+42-23
Original file line numberDiff line numberDiff line change
@@ -354,41 +354,50 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
354354
return nil
355355
}
356356

357-
// checkKeyContent only checks if key content has been used as public key,
357+
// checkKeyFingerprint only checks if key fingerprint has been used as public key,
358358
// it is OK to use same key as deploy key for multiple repositories/users.
359-
func checkKeyContent(content string) error {
360-
has, err := x.Get(&PublicKey{
361-
Content: content,
362-
Type: KeyTypeUser,
359+
func checkKeyFingerprint(e Engine, fingerprint string) error {
360+
has, err := e.Get(&PublicKey{
361+
Fingerprint: fingerprint,
362+
Type: KeyTypeUser,
363363
})
364364
if err != nil {
365365
return err
366366
} else if has {
367-
return ErrKeyAlreadyExist{0, content}
367+
return ErrKeyAlreadyExist{0, fingerprint, ""}
368368
}
369369
return nil
370370
}
371371

372-
func addKey(e Engine, key *PublicKey) (err error) {
372+
func calcFingerprint(publicKeyContent string) (string, error) {
373373
// Calculate fingerprint.
374374
tmpPath := strings.Replace(path.Join(os.TempDir(), fmt.Sprintf("%d", time.Now().Nanosecond()),
375375
"id_rsa.pub"), "\\", "/", -1)
376376
dir := path.Dir(tmpPath)
377377

378378
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
379-
return fmt.Errorf("Failed to create dir %s: %v", dir, err)
379+
return "", fmt.Errorf("Failed to create dir %s: %v", dir, err)
380380
}
381381

382-
if err = ioutil.WriteFile(tmpPath, []byte(key.Content), 0644); err != nil {
383-
return err
382+
if err := ioutil.WriteFile(tmpPath, []byte(publicKeyContent), 0644); err != nil {
383+
return "", err
384384
}
385385
stdout, stderr, err := process.GetManager().Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)
386386
if err != nil {
387-
return fmt.Errorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr)
387+
return "", fmt.Errorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr)
388388
} else if len(stdout) < 2 {
389-
return errors.New("not enough output for calculating fingerprint: " + stdout)
389+
return "", errors.New("not enough output for calculating fingerprint: " + stdout)
390+
}
391+
return strings.Split(stdout, " ")[1], nil
392+
}
393+
394+
func addKey(e Engine, key *PublicKey) (err error) {
395+
if len(key.Fingerprint) <= 0 {
396+
key.Fingerprint, err = calcFingerprint(key.Content)
397+
if err != nil {
398+
return err
399+
}
390400
}
391-
key.Fingerprint = strings.Split(stdout, " ")[1]
392401

393402
// Save SSH key.
394403
if _, err = e.Insert(key); err != nil {
@@ -405,7 +414,13 @@ func addKey(e Engine, key *PublicKey) (err error) {
405414
// AddPublicKey adds new public key to database and authorized_keys file.
406415
func AddPublicKey(ownerID int64, name, content string) (*PublicKey, error) {
407416
log.Trace(content)
408-
if err := checkKeyContent(content); err != nil {
417+
418+
fingerprint, err := calcFingerprint(content)
419+
if err != nil {
420+
return nil, err
421+
}
422+
423+
if err := checkKeyFingerprint(x, fingerprint); err != nil {
409424
return nil, err
410425
}
411426

@@ -426,11 +441,12 @@ func AddPublicKey(ownerID int64, name, content string) (*PublicKey, error) {
426441
}
427442

428443
key := &PublicKey{
429-
OwnerID: ownerID,
430-
Name: name,
431-
Content: content,
432-
Mode: AccessModeWrite,
433-
Type: KeyTypeUser,
444+
OwnerID: ownerID,
445+
Name: name,
446+
Fingerprint: fingerprint,
447+
Content: content,
448+
Mode: AccessModeWrite,
449+
Type: KeyTypeUser,
434450
}
435451
if err = addKey(sess, key); err != nil {
436452
return nil, fmt.Errorf("addKey: %v", err)
@@ -665,14 +681,15 @@ func HasDeployKey(keyID, repoID int64) bool {
665681

666682
// AddDeployKey add new deploy key to database and authorized_keys file.
667683
func AddDeployKey(repoID int64, name, content string) (*DeployKey, error) {
668-
if err := checkKeyContent(content); err != nil {
684+
fingerprint, err := calcFingerprint(content)
685+
if err != nil {
669686
return nil, err
670687
}
671688

672689
pkey := &PublicKey{
673-
Content: content,
674-
Mode: AccessModeRead,
675-
Type: KeyTypeDeploy,
690+
Fingerprint: fingerprint,
691+
Mode: AccessModeRead,
692+
Type: KeyTypeDeploy,
676693
}
677694
has, err := x.Get(pkey)
678695
if err != nil {
@@ -687,6 +704,8 @@ func AddDeployKey(repoID int64, name, content string) (*DeployKey, error) {
687704

688705
// First time use this deploy key.
689706
if !has {
707+
pkey.Content = content
708+
pkey.Name = name
690709
if err = addKey(sess, pkey); err != nil {
691710
return nil, fmt.Errorf("addKey: %v", err)
692711
}

0 commit comments

Comments
 (0)