Merge pull request '[FIX] Don't allow SSH authentication without ssh executable' (#5123) from gusted/forgejo-prevent-no-ssh into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5123
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2024-08-26 08:03:52 +00:00
commit 190b5a3859
7 changed files with 80 additions and 0 deletions

View file

@ -38,6 +38,8 @@ var (
InvertedGitFlushEnv bool // 2.43.1 InvertedGitFlushEnv bool // 2.43.1
SupportCheckAttrOnBare bool // >= 2.40 SupportCheckAttrOnBare bool // >= 2.40
HasSSHExecutable bool
gitVersion *version.Version gitVersion *version.Version
) )
@ -203,6 +205,10 @@ func InitFull(ctx context.Context) (err error) {
globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=")
} }
// Detect the presence of the ssh executable in $PATH.
_, err = exec.LookPath("ssh")
HasSSHExecutable = err == nil
return syncGitConfig() return syncGitConfig()
} }

View file

@ -1105,6 +1105,7 @@ mirror_interval_invalid = The mirror interval is not valid.
mirror_public_key = Public SSH key mirror_public_key = Public SSH key
mirror_use_ssh.text = Use SSH authentication mirror_use_ssh.text = Use SSH authentication
mirror_use_ssh.helper = Forgejo will mirror the repository via Git over SSH and create a keypair for you when you select this option. You must ensure that the generated public key is authorized to push to the destination repository. You cannot use password-based authorization when selecting this. mirror_use_ssh.helper = Forgejo will mirror the repository via Git over SSH and create a keypair for you when you select this option. You must ensure that the generated public key is authorized to push to the destination repository. You cannot use password-based authorization when selecting this.
mirror_use_ssh.not_available = SSH authentication isn't available.
mirror_denied_combination = Cannot use public key and password based authentication in combination. mirror_denied_combination = Cannot use public key and password based authentication in combination.
mirror_sync = synced mirror_sync = synced
mirror_sync_on_commit = Sync when commits are pushed mirror_sync_on_commit = Sync when commits are pushed

View file

@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
@ -350,6 +351,11 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro
return return
} }
if mirrorOption.UseSSH && !git.HasSSHExecutable {
ctx.Error(http.StatusBadRequest, "CreatePushMirror", "SSH authentication not available.")
return
}
if mirrorOption.UseSSH && (mirrorOption.RemoteUsername != "" || mirrorOption.RemotePassword != "") { if mirrorOption.UseSSH && (mirrorOption.RemoteUsername != "" || mirrorOption.RemotePassword != "") {
ctx.Error(http.StatusBadRequest, "CreatePushMirror", "'use_ssh' is mutually exclusive with 'remote_username' and 'remote_passoword'") ctx.Error(http.StatusBadRequest, "CreatePushMirror", "'use_ssh' is mutually exclusive with 'remote_username' and 'remote_passoword'")
return return

View file

@ -92,6 +92,7 @@ func SettingsCtxData(ctx *context.Context) {
return return
} }
ctx.Data["PushMirrors"] = pushMirrors ctx.Data["PushMirrors"] = pushMirrors
ctx.Data["CanUseSSHMirroring"] = git.HasSSHExecutable
} }
// Units show a repositorys unit settings page // Units show a repositorys unit settings page
@ -643,6 +644,11 @@ func SettingsPost(ctx *context.Context) {
return return
} }
if form.PushMirrorUseSSH && !git.HasSSHExecutable {
ctx.RenderWithErr(ctx.Tr("repo.mirror_use_ssh.not_available"), tplSettingsOptions, &form)
return
}
address, err := forms.ParseRemoteAddr(form.PushMirrorAddress, form.PushMirrorUsername, form.PushMirrorPassword) address, err := forms.ParseRemoteAddr(form.PushMirrorAddress, form.PushMirrorUsername, form.PushMirrorPassword)
if err == nil { if err == nil {
err = migrations.IsMigrateURLAllowed(address, ctx.Doer) err = migrations.IsMigrateURLAllowed(address, ctx.Doer)

View file

@ -300,6 +300,7 @@
<label for="push_mirror_password">{{ctx.Locale.Tr "password"}}</label> <label for="push_mirror_password">{{ctx.Locale.Tr "password"}}</label>
<input id="push_mirror_password" name="push_mirror_password" type="password" value="{{.push_mirror_password}}" autocomplete="off"> <input id="push_mirror_password" name="push_mirror_password" type="password" value="{{.push_mirror_password}}" autocomplete="off">
</div> </div>
{{if .CanUseSSHMirroring}}
<div class="inline field {{if .Err_PushMirrorUseSSH}}error{{end}}"> <div class="inline field {{if .Err_PushMirrorUseSSH}}error{{end}}">
<div class="ui checkbox df ac"> <div class="ui checkbox df ac">
<input id="push_mirror_use_ssh" name="push_mirror_use_ssh" type="checkbox" {{if .push_mirror_use_ssh}}checked{{end}}> <input id="push_mirror_use_ssh" name="push_mirror_use_ssh" type="checkbox" {{if .push_mirror_use_ssh}}checked{{end}}>
@ -307,6 +308,7 @@
<span class="help tw-block">{{ctx.Locale.Tr "repo.mirror_use_ssh.helper"}} <span class="help tw-block">{{ctx.Locale.Tr "repo.mirror_use_ssh.helper"}}
</div> </div>
</div> </div>
{{end}}
</div> </div>
</details> </details>
<div class="field"> <div class="field">

View file

@ -11,6 +11,7 @@ import (
"net/http" "net/http"
"net/url" "net/url"
"os" "os"
"os/exec"
"path/filepath" "path/filepath"
"strconv" "strconv"
"testing" "testing"
@ -23,6 +24,7 @@ import (
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
@ -141,6 +143,11 @@ func testAPIPushMirror(t *testing.T, u *url.URL) {
} }
func TestAPIPushMirrorSSH(t *testing.T) { func TestAPIPushMirrorSSH(t *testing.T) {
_, err := exec.LookPath("ssh")
if err != nil {
t.Skip("SSH executable not present")
}
onGiteaRun(t, func(t *testing.T, _ *url.URL) { onGiteaRun(t, func(t *testing.T, _ *url.URL) {
defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)() defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)()
defer test.MockVariableValue(&setting.Mirror.Enabled, true)() defer test.MockVariableValue(&setting.Mirror.Enabled, true)()
@ -178,6 +185,22 @@ func TestAPIPushMirrorSSH(t *testing.T) {
assert.EqualValues(t, "'use_ssh' is mutually exclusive with 'remote_username' and 'remote_passoword'", apiError.Message) assert.EqualValues(t, "'use_ssh' is mutually exclusive with 'remote_username' and 'remote_passoword'", apiError.Message)
}) })
t.Run("SSH not available", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
defer test.MockVariableValue(&git.HasSSHExecutable, false)()
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/push_mirrors", srcRepo.FullName()), &api.CreatePushMirrorOption{
RemoteAddress: sshURL,
Interval: "8h",
UseSSH: true,
}).AddTokenAuth(token)
resp := MakeRequest(t, req, http.StatusBadRequest)
var apiError api.APIError
DecodeJSON(t, resp, &apiError)
assert.EqualValues(t, "SSH authentication not available.", apiError.Message)
})
t.Run("Normal", func(t *testing.T) { t.Run("Normal", func(t *testing.T) {
var pushMirror *repo_model.PushMirror var pushMirror *repo_model.PushMirror
t.Run("Adding", func(t *testing.T) { t.Run("Adding", func(t *testing.T) {

View file

@ -11,6 +11,7 @@ import (
"net/http" "net/http"
"net/url" "net/url"
"os" "os"
"os/exec"
"path/filepath" "path/filepath"
"strconv" "strconv"
"testing" "testing"
@ -157,6 +158,11 @@ func doRemovePushMirror(ctx APITestContext, address, username, password string,
} }
func TestSSHPushMirror(t *testing.T) { func TestSSHPushMirror(t *testing.T) {
_, err := exec.LookPath("ssh")
if err != nil {
t.Skip("SSH executable not present")
}
onGiteaRun(t, func(t *testing.T, _ *url.URL) { onGiteaRun(t, func(t *testing.T, _ *url.URL) {
defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)() defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)()
defer test.MockVariableValue(&setting.Mirror.Enabled, true)() defer test.MockVariableValue(&setting.Mirror.Enabled, true)()
@ -194,6 +200,36 @@ func TestSSHPushMirror(t *testing.T) {
assert.Contains(t, errMsg, "Cannot use public key and password based authentication in combination.") assert.Contains(t, errMsg, "Cannot use public key and password based authentication in combination.")
}) })
inputSelector := `input[id="push_mirror_use_ssh"]`
t.Run("SSH not available", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
defer test.MockVariableValue(&git.HasSSHExecutable, false)()
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/settings", srcRepo.FullName()), map[string]string{
"_csrf": GetCSRF(t, sess, fmt.Sprintf("/%s/settings", srcRepo.FullName())),
"action": "push-mirror-add",
"push_mirror_address": sshURL,
"push_mirror_use_ssh": "true",
"push_mirror_interval": "0",
})
resp := sess.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
errMsg := htmlDoc.Find(".ui.negative.message").Text()
assert.Contains(t, errMsg, "SSH authentication isn't available.")
htmlDoc.AssertElement(t, inputSelector, false)
})
t.Run("SSH available", func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings", srcRepo.FullName()))
resp := sess.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
htmlDoc.AssertElement(t, inputSelector, true)
})
t.Run("Normal", func(t *testing.T) { t.Run("Normal", func(t *testing.T) {
var pushMirror *repo_model.PushMirror var pushMirror *repo_model.PushMirror
t.Run("Adding", func(t *testing.T) { t.Run("Adding", func(t *testing.T) {