mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-02-07 02:10:12 +00:00
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>
94 lines
3.4 KiB
Go
94 lines
3.4 KiB
Go
// Copyright 2022 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package ssh
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"net"
|
|
"os"
|
|
"path/filepath"
|
|
"strconv"
|
|
"strings"
|
|
|
|
"forgejo.org/models/asymkey"
|
|
"forgejo.org/modules/log"
|
|
"forgejo.org/modules/setting"
|
|
)
|
|
|
|
var logger = log.GetManager().GetLogger("ssh")
|
|
|
|
func Init(ctx context.Context) error {
|
|
if setting.SSH.Disabled {
|
|
builtinUnused()
|
|
return nil
|
|
}
|
|
|
|
if setting.SSH.StartBuiltinServer {
|
|
Listen(setting.SSH.ListenHost, setting.SSH.ListenPort, setting.SSH.ServerCiphers, setting.SSH.ServerKeyExchanges, setting.SSH.ServerMACs)
|
|
log.Info("SSH server started on %s. Cipher list (%v), key exchange algorithms (%v), MACs (%v)",
|
|
net.JoinHostPort(setting.SSH.ListenHost, strconv.Itoa(setting.SSH.ListenPort)),
|
|
setting.SSH.ServerCiphers, setting.SSH.ServerKeyExchanges, setting.SSH.ServerMACs,
|
|
)
|
|
return nil
|
|
}
|
|
|
|
builtinUnused()
|
|
|
|
// FIXME: why 0o644 for a directory .....
|
|
if err := os.MkdirAll(setting.SSH.KeyTestPath, 0o644); err != nil {
|
|
return fmt.Errorf("failed to create directory %q for ssh key test: %w", setting.SSH.KeyTestPath, err)
|
|
}
|
|
|
|
if len(setting.SSH.TrustedUserCAKeys) > 0 && setting.SSH.AuthorizedPrincipalsEnabled {
|
|
caKeysFileName := setting.SSH.TrustedUserCAKeysFile
|
|
caKeysFileDir := filepath.Dir(caKeysFileName)
|
|
|
|
err := os.MkdirAll(caKeysFileDir, 0o700) // SSH.RootPath by default (That is `~/.ssh` in most cases)
|
|
if err != nil {
|
|
return fmt.Errorf("failed to create directory %q for ssh trusted ca keys: %w", caKeysFileDir, err)
|
|
}
|
|
|
|
if err := os.WriteFile(caKeysFileName, []byte(strings.Join(setting.SSH.TrustedUserCAKeys, "\n")), 0o600); err != nil {
|
|
return fmt.Errorf("failed to write ssh trusted ca keys to %q: %w", caKeysFileName, err)
|
|
}
|
|
}
|
|
|
|
if !setting.SSH.AllowUnexpectedAuthorizedKeys {
|
|
findings, err := asymkey.InspectPublicKeys(ctx)
|
|
if err != nil {
|
|
return fmt.Errorf("inspect authorized_keys failed: %w", err)
|
|
}
|
|
|
|
unexpectedKeys := []string{}
|
|
for _, finding := range findings {
|
|
switch finding.Type {
|
|
case asymkey.InspectionResultFileMissing:
|
|
err := asymkey.RewriteAllPublicKeys(ctx)
|
|
if err != nil {
|
|
return fmt.Errorf("rewrite authorized_keys failed: %w", err)
|
|
}
|
|
case asymkey.InspectionResultUnexpectedKey:
|
|
unexpectedKeys = append(unexpectedKeys, finding.Comment)
|
|
|
|
case asymkey.InspectionResultMissingExpectedKey:
|
|
// MissingExpectingKey can happen at the same time as UnexpectedKey -- so while it might seem to make
|
|
// sense to regenerate the key file automatically in this case, it could overwrite keys that someone
|
|
// wants present there when they want SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true but haven't set it yet.
|
|
// So, just warn as this isn't an insecure situation.
|
|
log.Warn("authorized_keys is missing a key from the database; regenerate authorized_keys from the admin panel to repair this")
|
|
}
|
|
}
|
|
|
|
if len(unexpectedKeys) > 0 {
|
|
detailConcat := strings.Join(unexpectedKeys, "\n\t")
|
|
log.Fatal("An unexpected ssh public key was discovered. Forgejo will shutdown to require this to be fixed. Fix by either:\n"+
|
|
"Option 1: Delete the file %s, and Forgejo will recreate it with only expected ssh public keys.\n"+
|
|
"Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.\n\t"+
|
|
detailConcat, filepath.Join(setting.SSH.RootPath, "authorized_keys"))
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|