2021-07-24 11:16:34 +01:00
|
|
|
// Copyright 2021 The Gitea Authors. All rights reserved.
|
2022-11-27 13:20:29 -05:00
|
|
|
// SPDX-License-Identifier: MIT
|
2021-07-24 11:16:34 +01:00
|
|
|
|
2021-12-10 16:14:24 +08:00
|
|
|
package asymkey
|
2021-07-24 11:16:34 +01:00
|
|
|
|
|
|
|
|
import (
|
|
|
|
|
"bufio"
|
2022-05-20 22:08:52 +08:00
|
|
|
"context"
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
"errors"
|
2021-07-24 11:16:34 +01:00
|
|
|
"fmt"
|
|
|
|
|
"io"
|
|
|
|
|
"os"
|
|
|
|
|
"path/filepath"
|
|
|
|
|
"strings"
|
|
|
|
|
"sync"
|
|
|
|
|
"time"
|
|
|
|
|
|
2025-03-27 19:40:14 +00:00
|
|
|
"forgejo.org/models/db"
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
"forgejo.org/modules/container"
|
2025-03-27 19:40:14 +00:00
|
|
|
"forgejo.org/modules/log"
|
|
|
|
|
"forgejo.org/modules/setting"
|
|
|
|
|
"forgejo.org/modules/util"
|
2021-07-24 11:16:34 +01:00
|
|
|
)
|
|
|
|
|
|
|
|
|
|
// _____ __ .__ .__ .___
|
|
|
|
|
// / _ \ __ ___/ |_| |__ ___________|__|_______ ____ __| _/
|
|
|
|
|
// / /_\ \| | \ __\ | \ / _ \_ __ \ \___ // __ \ / __ |
|
|
|
|
|
// / | \ | /| | | Y ( <_> ) | \/ |/ /\ ___// /_/ |
|
|
|
|
|
// \____|__ /____/ |__| |___| /\____/|__| |__/_____ \\___ >____ |
|
|
|
|
|
// \/ \/ \/ \/ \/
|
|
|
|
|
// ____ __.
|
|
|
|
|
// | |/ _|____ ___.__. ______
|
|
|
|
|
// | <_/ __ < | |/ ___/
|
|
|
|
|
// | | \ ___/\___ |\___ \
|
|
|
|
|
// |____|__ \___ > ____/____ >
|
|
|
|
|
// \/ \/\/ \/
|
|
|
|
|
//
|
|
|
|
|
// This file contains functions for creating authorized_keys files
|
|
|
|
|
//
|
|
|
|
|
// There is a dependence on the database within RegeneratePublicKeys however most of these functions probably belong in a module
|
|
|
|
|
|
|
|
|
|
const (
|
|
|
|
|
tplCommentPrefix = `# gitea public key`
|
2021-11-23 03:44:26 +01:00
|
|
|
tplPublicKey = tplCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s` + "\n"
|
2021-07-24 11:16:34 +01:00
|
|
|
)
|
|
|
|
|
|
|
|
|
|
var sshOpLocker sync.Mutex
|
|
|
|
|
|
|
|
|
|
// AuthorizedStringForKey creates the authorized keys string appropriate for the provided key
|
|
|
|
|
func AuthorizedStringForKey(key *PublicKey) string {
|
|
|
|
|
sb := &strings.Builder{}
|
2023-07-04 20:36:08 +02:00
|
|
|
_ = setting.SSH.AuthorizedKeysCommandTemplateTemplate.Execute(sb, map[string]any{
|
2021-07-24 11:16:34 +01:00
|
|
|
"AppPath": util.ShellEscape(setting.AppPath),
|
|
|
|
|
"AppWorkPath": util.ShellEscape(setting.AppWorkPath),
|
|
|
|
|
"CustomConf": util.ShellEscape(setting.CustomConf),
|
|
|
|
|
"CustomPath": util.ShellEscape(setting.CustomPath),
|
|
|
|
|
"Key": key,
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
return fmt.Sprintf(tplPublicKey, util.ShellEscape(sb.String()), key.Content)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// appendAuthorizedKeysToFile appends new SSH keys' content to authorized_keys file.
|
|
|
|
|
func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
|
|
|
|
|
// Don't need to rewrite this file if builtin SSH server is enabled.
|
|
|
|
|
if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
sshOpLocker.Lock()
|
|
|
|
|
defer sshOpLocker.Unlock()
|
|
|
|
|
|
|
|
|
|
if setting.SSH.RootPath != "" {
|
|
|
|
|
// First of ensure that the RootPath is present, and if not make it with 0700 permissions
|
|
|
|
|
// This of course doesn't guarantee that this is the right directory for authorized_keys
|
|
|
|
|
// but at least if it's supposed to be this directory and it doesn't exist and we're the
|
|
|
|
|
// right user it will at least be created properly.
|
|
|
|
|
err := os.MkdirAll(setting.SSH.RootPath, 0o700)
|
|
|
|
|
if err != nil {
|
|
|
|
|
log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
|
|
|
|
|
f, err := os.OpenFile(fPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600)
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
defer f.Close()
|
|
|
|
|
|
2025-03-08 00:43:41 +00:00
|
|
|
fi, err := f.Stat()
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2021-07-24 11:16:34 +01:00
|
|
|
|
2025-03-08 00:43:41 +00:00
|
|
|
// .ssh directory should have mode 700, and authorized_keys file should have mode 600.
|
|
|
|
|
if fi.Mode().Perm() > 0o600 {
|
|
|
|
|
log.Error("authorized_keys file has unusual permission flags: %s - setting to -rw-------", fi.Mode().Perm().String())
|
|
|
|
|
if err = f.Chmod(0o600); err != nil {
|
|
|
|
|
return err
|
2021-07-24 11:16:34 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
for _, key := range keys {
|
|
|
|
|
if key.Type == KeyTypePrincipal {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
if _, err = f.WriteString(key.AuthorizedString()); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
type InspectionFinding struct {
|
|
|
|
|
Type InspectionFindingType
|
|
|
|
|
Comment string
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
type InspectionFindingType int
|
|
|
|
|
|
|
|
|
|
const (
|
|
|
|
|
InspectionResultFileMissing InspectionFindingType = iota // authorized_keys is absent, must regenerate
|
|
|
|
|
InspectionResultUnexpectedKey // authorized_keys contains at least one unexpected key
|
|
|
|
|
InspectionResultMissingExpectedKey // authorized_keys does not contain a key that is in the DB
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
func InspectPublicKeys(ctx context.Context) ([]InspectionFinding, error) {
|
|
|
|
|
if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
|
|
|
|
|
return []InspectionFinding{}, nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
sshOpLocker.Lock()
|
|
|
|
|
defer sshOpLocker.Unlock()
|
|
|
|
|
|
|
|
|
|
path := filepath.Join(setting.SSH.RootPath, "authorized_keys")
|
|
|
|
|
file, err := os.OpenFile(path, os.O_RDONLY, 0o600)
|
|
|
|
|
if errors.Is(err, os.ErrNotExist) {
|
|
|
|
|
return []InspectionFinding{{Type: InspectionResultFileMissing}}, nil
|
|
|
|
|
} else if err != nil {
|
|
|
|
|
return nil, err
|
|
|
|
|
}
|
|
|
|
|
defer file.Close()
|
|
|
|
|
|
|
|
|
|
// Create a set of all the expected output in the `authorized_keys` file.
|
|
|
|
|
expectedKeys := make(container.Set[string])
|
|
|
|
|
if err := db.GetEngine(ctx).Where("type != ?", KeyTypePrincipal).Iterate(new(PublicKey), func(idx int, bean any) (err error) {
|
|
|
|
|
keyWithComment := (bean.(*PublicKey)).AuthorizedString()
|
|
|
|
|
if !strings.HasPrefix(keyWithComment, tplCommentPrefix) {
|
|
|
|
|
return fmt.Errorf("unexpected AuthorizedString")
|
|
|
|
|
}
|
|
|
|
|
key := strings.TrimSpace(keyWithComment[len(tplCommentPrefix):])
|
|
|
|
|
expectedKeys.Add(key)
|
|
|
|
|
return nil
|
|
|
|
|
}); err != nil {
|
|
|
|
|
return nil, err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
findings := []InspectionFinding{}
|
|
|
|
|
|
|
|
|
|
scanner := bufio.NewScanner(file)
|
|
|
|
|
lineNumber := 0
|
|
|
|
|
for scanner.Scan() {
|
|
|
|
|
lineNumber++
|
|
|
|
|
|
|
|
|
|
line := scanner.Text()
|
|
|
|
|
if strings.HasPrefix(line, tplCommentPrefix) {
|
|
|
|
|
// After the public key comment, we expect a public key
|
|
|
|
|
lineNumber++
|
|
|
|
|
if !scanner.Scan() {
|
|
|
|
|
return nil, fmt.Errorf("authorized_keys file %s had a prefix comment but no key on line %d", path, lineNumber)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
key := strings.TrimSpace(scanner.Text())
|
|
|
|
|
if !expectedKeys.Remove(key) {
|
|
|
|
|
findings = append(findings, InspectionFinding{
|
|
|
|
|
Type: InspectionResultUnexpectedKey,
|
|
|
|
|
Comment: fmt.Sprintf("Key on line %d of %s does not exist in database", lineNumber, path),
|
|
|
|
|
})
|
|
|
|
|
}
|
|
|
|
|
} else {
|
|
|
|
|
findings = append(findings, InspectionFinding{
|
|
|
|
|
Type: InspectionResultUnexpectedKey,
|
|
|
|
|
Comment: fmt.Sprintf("Unexpected key on line %d of %s", lineNumber, path),
|
|
|
|
|
})
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if err = scanner.Err(); err != nil {
|
|
|
|
|
return nil, err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
for key := range expectedKeys {
|
|
|
|
|
findings = append(findings, InspectionFinding{
|
|
|
|
|
Type: InspectionResultMissingExpectedKey,
|
|
|
|
|
Comment: fmt.Sprintf("Key in database is not present in %s: %s", path, key),
|
|
|
|
|
})
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return findings, nil
|
|
|
|
|
}
|
|
|
|
|
|
2021-07-24 11:16:34 +01:00
|
|
|
// RewriteAllPublicKeys removes any authorized key and rewrite all keys from database again.
|
2023-10-15 23:46:06 +08:00
|
|
|
// Note: db.GetEngine(ctx).Iterate does not get latest data after insert/delete, so we have to call this function
|
2021-07-24 11:16:34 +01:00
|
|
|
// outside any session scope independently.
|
2023-09-25 15:17:37 +02:00
|
|
|
func RewriteAllPublicKeys(ctx context.Context) error {
|
2021-07-24 11:16:34 +01:00
|
|
|
// Don't rewrite key if internal server
|
|
|
|
|
if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
sshOpLocker.Lock()
|
|
|
|
|
defer sshOpLocker.Unlock()
|
|
|
|
|
|
|
|
|
|
if setting.SSH.RootPath != "" {
|
|
|
|
|
// First of ensure that the RootPath is present, and if not make it with 0700 permissions
|
|
|
|
|
// This of course doesn't guarantee that this is the right directory for authorized_keys
|
|
|
|
|
// but at least if it's supposed to be this directory and it doesn't exist and we're the
|
|
|
|
|
// right user it will at least be created properly.
|
|
|
|
|
err := os.MkdirAll(setting.SSH.RootPath, 0o700)
|
|
|
|
|
if err != nil {
|
|
|
|
|
log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
|
|
|
|
|
tmpPath := fPath + ".tmp"
|
|
|
|
|
t, err := os.OpenFile(tmpPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
defer func() {
|
|
|
|
|
t.Close()
|
|
|
|
|
if err := util.Remove(tmpPath); err != nil {
|
|
|
|
|
log.Warn("Unable to remove temporary authorized keys file: %s: Error: %v", tmpPath, err)
|
|
|
|
|
}
|
|
|
|
|
}()
|
|
|
|
|
|
|
|
|
|
if setting.SSH.AuthorizedKeysBackup {
|
|
|
|
|
isExist, err := util.IsExist(fPath)
|
|
|
|
|
if err != nil {
|
|
|
|
|
log.Error("Unable to check if %s exists. Error: %v", fPath, err)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if isExist {
|
|
|
|
|
bakPath := fmt.Sprintf("%s_%d.gitea_bak", fPath, time.Now().Unix())
|
|
|
|
|
if err = util.CopyFile(fPath, bakPath); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
if err := regeneratePublicKeys(ctx, t); err != nil {
|
2021-07-24 11:16:34 +01:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2023-09-30 22:16:47 +02:00
|
|
|
if err := t.Sync(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := t.Close(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2021-07-24 11:16:34 +01:00
|
|
|
return util.Rename(tmpPath, fPath)
|
|
|
|
|
}
|
|
|
|
|
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
// regeneratePublicKeys regenerates the authorized_keys file
|
|
|
|
|
func regeneratePublicKeys(ctx context.Context, t io.StringWriter) error {
|
2023-07-04 20:36:08 +02:00
|
|
|
if err := db.GetEngine(ctx).Where("type != ?", KeyTypePrincipal).Iterate(new(PublicKey), func(idx int, bean any) (err error) {
|
2021-07-24 11:16:34 +01:00
|
|
|
_, err = t.WriteString((bean.(*PublicKey)).AuthorizedString())
|
|
|
|
|
return err
|
|
|
|
|
}); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
// Preserve any authorized_keys contents that are not generated from Forgejo
|
|
|
|
|
if setting.SSH.AllowUnexpectedAuthorizedKeys {
|
|
|
|
|
fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
|
|
|
|
|
isExist, err := util.IsExist(fPath)
|
2021-07-24 11:16:34 +01:00
|
|
|
if err != nil {
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
log.Error("Unable to check if %s exists. Error: %v", fPath, err)
|
2021-07-24 11:16:34 +01:00
|
|
|
return err
|
|
|
|
|
}
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
if isExist {
|
|
|
|
|
f, err := os.Open(fPath)
|
2021-07-24 11:16:34 +01:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
defer f.Close()
|
|
|
|
|
|
|
|
|
|
scanner := bufio.NewScanner(f)
|
|
|
|
|
for scanner.Scan() {
|
|
|
|
|
line := scanner.Text()
|
|
|
|
|
if strings.HasPrefix(line, tplCommentPrefix) {
|
|
|
|
|
scanner.Scan()
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
_, err = t.WriteString(line + "\n")
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if err = scanner.Err(); err != nil {
|
|
|
|
|
return fmt.Errorf("regeneratePublicKeys scan: %w", err)
|
|
|
|
|
}
|
2024-03-19 10:20:36 +08:00
|
|
|
}
|
2021-07-24 11:16:34 +01:00
|
|
|
}
|
feat: ensure only expected ssh public keys are in authorized_keys file (#10010)
A security vulnerability that was fixed in #9840 had the potential to corrupt the `authorized_keys` file that Forgejo is managing to allow ssh access. In the event that it was corrupted, the existing behaviour of Forgejo is to maintain the contents that it finds in the `authorized_keys` file, potentially making an exploit of a Forgejo server persistent despite attempts to rewrite the key file.
This feature adds a new layer of security resiliency in order to prevent persistent ssh key corruption. When Forgejo starts up, if relevant, Forgejo will read the `authorized_keys` file and validate the file's contents. If any keys are found in the file that are not expected, then Forgejo will terminate its startup in order to signal to the server administrator that a critical security risk is present that must be addressed:
```
2025/11/07 10:13:50 modules/ssh/init.go:86:Init() [F] An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:
Option 1: Delete the file /home/forgejo/.ssh/authorized_keys, and Forgejo will recreate it with only expected ssh public keys.
Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.
Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 2 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 3 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 4 of /home/forgejo/.ssh/authorized_keys
Unexpected key on line 5 of /home/forgejo/.ssh/authorized_keys
```
As noted in the log message, the server administrator can address this problem in one of two ways:
- If they delete the file that contains the unexpected keys, Forgejo will regenerate it containing only the expected keys from the Forgejo database.
- If they would like to run their server with ssh keys that are not managed by Forgejo (for example, if they're reusing a `git` ssh user that is accessed through `git@server` and does not invoke Forgejo's ssh handlers), then they can disable the new security check by setting `[server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS = true` in their `app.ini`.
**This is a breaking change**: the default behaviour is to be restrictive in the contents of `authorized_keys` in order to ensure that server administrators with unexpected keys in `authorized_keys` are aware of those keys.
If `SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=false`, then the behaviour when Forgejo rewrites the `authorized_keys` file is changed to not maintain any unexpected keys in the file. If the value is `true`, then the old behaviour is retained.
The `doctor check` subcommand is updated to use the new validity routines:
```
[4] Check if OpenSSH authorized_keys file is up-to-date
- [E] Unexpected key on line 1 of /home/forgejo/.ssh/authorized_keys
- [E] Key in database is not present in /home/forgejo/.ssh/authorized_keys: ...
- [E] authorized_keys file "/home/forgejo/.ssh/authorized_keys" contains validity errors.
Regenerate it with:
"forgejo admin regenerate keys"
or
"forgejo doctor check --run authorized-keys --fix"
ERROR
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- **Documentation updates required**; pending initial reviews of this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [x] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10010
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: mfenniak <mfenniak@noreply.codeberg.org>
Co-committed-by: mfenniak <mfenniak@noreply.codeberg.org>
2025-11-09 01:06:04 +01:00
|
|
|
|
2021-07-24 11:16:34 +01:00
|
|
|
return nil
|
|
|
|
|
}
|