mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
fix: don't clobber authorized_keys file during installation (#10948)
This PR makes two changes. **First**, it removes the ability for Forgejo to rewrite ssh authorized_keys during startup. Forgejo previously checked if the application path or config file path had changed from that which is recorded in its database. If either has changed, it would rewrite the SSH `authorized_keys` file, as those paths are embedded into that file. The problem is that if a new installation runs the app and goes through a standard init procedure, it will clobber ~/.ssh/authorized_keys which is a disruptive mistake. Instead, Forgejo will proceed to ssh initialization, and due to the change in #10010 the incorrect application path or config file path will result in a fatal server error that the administrator must resolve. Disabling SSH is added as a plausible option for how to resolve that fatal error. **Second**, the interactive install UI has been modified to detect this error before the installation proceeds. If a user is attempting to install Forgejo with SSH, and they have an existing `~/.ssh/authorized_keys` file with keys present in it, the installation will fail with an error advising them to use a separate Forgejo user or to disable SSH. (More options are possible to fix this problem, but these are the obvious solutions.)  Fixes #10942. Manually tested. Without the install process change, Forgejo behaves like this: - Configure a typical end-user `~/.ssh/authorized_keys` file with normal keys - Run through a Forgejo initialization process on a new database; run with SQLite, add a new administrator account during the init process. - After initialization, Forgejo will restart and encounter a fatal error: ``` 2026/01/20 10:11:24 routers/init.go:84:syncAppConfForGit() [I] AppPath changed from '' to '/home/mfenniak/Dev/forgejo/forgejo' 2026/01/20 10:11:24 routers/init.go:89:syncAppConfForGit() [I] CustomConf changed from '' to '/home/mfenniak/Dev/forgejo/custom/conf/app.ini' 2026/01/20 10:11:24 routers/init.go:95:syncAppConfForGit() [I] re-sync repository hooks ... 2026/01/20 10:11:24 ...er/issues/indexer.go:155:func2() [I] Issue Indexer Initialization took 9.858336ms 2026/01/20 10:11:24 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/mfenniak/.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. Option 3: If unused, disable SSH support by setting [server].DISABLE_SSH=true in Forgejo's config file. Unexpected key on line 1 of /home/mfenniak/.ssh/authorized_keys Unexpected key on line 2 of /home/mfenniak/.ssh/authorized_keys Unexpected key on line 3 of /home/mfenniak/.ssh/authorized_keys Unexpected key on line 4 of /home/mfenniak/.ssh/authorized_keys ``` With the install process change, the above error screenshot occurs instead. ## 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... - [ ] 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 - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. <!--start release-notes-assistant--> ## Release notes <!--URL:https://codeberg.org/forgejo/forgejo--> - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/10948): <!--number 10948 --><!--line 0 --><!--description ZG9uJ3QgY2xvYmJlciBhdXRob3JpemVkX2tleXMgZmlsZSBkdXJpbmcgaW5zdGFsbGF0aW9u-->don't clobber authorized_keys file during installation<!--description--> <!--end release-notes-assistant--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10948 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
Mathieu Fenniak
Mathieu Fenniak
parent
0dbd533e42
commit
c52ecd2258
4 changed files with 25 additions and 6 deletions
|
|
@ -85,7 +85,9 @@ func Init(ctx context.Context) error {
|
|||
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"+
|
||||
"Option 2: Permit unexpected keys by setting [server].SSH_ALLOW_UNEXPECTED_AUTHORIZED_KEYS=true in Forgejo's config file.\n"+
|
||||
"Option 3: If unused, disable SSH support by setting [server].DISABLE_SSH=true in Forgejo's config file.\n"+
|
||||
"\t"+
|
||||
detailConcat, filepath.Join(setting.SSH.RootPath, "authorized_keys"))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,6 +115,8 @@
|
|||
"alert.asset_load_failed": "Failed to load asset files from {path}. Please make sure the asset files can be accessed.",
|
||||
"alert.range_error": " must be a number between %[1]s and %[2]s.",
|
||||
"install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s",
|
||||
"install.ssh_authorized_keys_inspection_error": "Failed to inspect existing authorized_keys file: %v",
|
||||
"install.ssh_authorized_keys_unexpected_key": "Enabling SSH for Forgejo conflicts with the file located at %s that contains existing SSH keys. Suggestions: use a dedicated system user for Forgejo, or disable SSH.",
|
||||
"profile.actions.tooltip": "More actions",
|
||||
"profile.edit.link": "Edit profile",
|
||||
"feed.atom.link": "Atom feed",
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ import (
|
|||
"runtime"
|
||||
|
||||
"forgejo.org/models"
|
||||
asymkey_model "forgejo.org/models/asymkey"
|
||||
auth_model "forgejo.org/models/auth"
|
||||
"forgejo.org/modules/cache"
|
||||
"forgejo.org/modules/eventsource"
|
||||
|
|
@ -95,10 +94,6 @@ func syncAppConfForGit(ctx context.Context) error {
|
|||
if updated {
|
||||
log.Info("re-sync repository hooks ...")
|
||||
mustInitCtx(ctx, repo_service.SyncRepositoryHooks)
|
||||
|
||||
log.Info("re-write ssh public keys ...")
|
||||
mustInitCtx(ctx, asymkey_model.RewriteAllPublicKeys)
|
||||
|
||||
return system.AppState.Set(ctx, runtimeState)
|
||||
}
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import (
|
|||
"strings"
|
||||
"time"
|
||||
|
||||
"forgejo.org/models/asymkey"
|
||||
"forgejo.org/models/db"
|
||||
db_install "forgejo.org/models/db/install"
|
||||
"forgejo.org/models/gitea_migrations"
|
||||
|
|
@ -403,6 +404,25 @@ func SubmitInstall(ctx *context.Context) {
|
|||
} else {
|
||||
cfg.Section("server").Key("DISABLE_SSH").SetValue("false")
|
||||
cfg.Section("server").Key("SSH_PORT").SetValue(fmt.Sprint(form.SSHPort))
|
||||
|
||||
sshKeyErrors, err := asymkey.InspectPublicKeys(ctx)
|
||||
if err != nil {
|
||||
ctx.RenderWithErr(ctx.Tr("install.ssh_authorized_keys_inspection_error", err), tplInstall, &form)
|
||||
return
|
||||
}
|
||||
|
||||
var authorizedKeysWillCauseFatalError bool
|
||||
for _, finding := range sshKeyErrors {
|
||||
if finding.Type == asymkey.InspectionResultUnexpectedKey {
|
||||
// Any single finding of this type would cause `ssh.Init` to have a fatal error on Forgejo startup, so
|
||||
// let's note it here while the install page is still usable and allow users to deal with it.
|
||||
authorizedKeysWillCauseFatalError = true
|
||||
}
|
||||
}
|
||||
if authorizedKeysWillCauseFatalError {
|
||||
ctx.RenderWithErr(ctx.Tr("install.ssh_authorized_keys_unexpected_key", filepath.Join(setting.SSH.RootPath, "authorized_keys")), tplInstall, &form)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if form.LFSRootPath != "" {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue