diff --git a/models/asymkey/ssh_key_authorized_keys.go b/models/asymkey/ssh_key_authorized_keys.go index db4730f00a152..cb2943e31dfd8 100644 --- a/models/asymkey/ssh_key_authorized_keys.go +++ b/models/asymkey/ssh_key_authorized_keys.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "regexp" "strings" "sync" @@ -50,12 +51,42 @@ func WriteAuthorizedStringForValidKey(key *PublicKey, w io.Writer) error { return err } +var globalVars = sync.OnceValue(func() (ret struct { + principalRegexp *regexp.Regexp +}, +) { + // principalRegexp expresses whether a principal is considered valid. + // This reverse engineers how sshd parses the authorized keys file, + // see e.g. https://github.com/openssh/openssh-portable/blob/32deb00b38b4ee2b3302f261ea1e68c04e020a08/auth2-pubkeyfile.c#L221-L256 + // Any newline or # comment will be stripped when parsing, so don't allow + // those. Also, if any space or tab is present in the principal, the part + // proceeding this would be parsed as an option, so just avoid any whitespace + // altogether. + ret.principalRegexp = regexp.MustCompile(`^[^\s#]+$`) + return ret +}) + func writeAuthorizedStringForKey(key *PublicKey, w io.Writer) (keyValid bool, err error) { - const tpl = AuthorizedStringCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s %s` + "\n" - pubKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key.Content)) - if err != nil { - return false, err + const tpl = AuthorizedStringCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s` + "\n" + + var sshKey string + + if key.Type == KeyTypePrincipal { + // TODO: actually using PublicKey to store "principal" is an abuse + if !globalVars().principalRegexp.MatchString(key.Content) { + return false, fmt.Errorf("invalid principal key: %s", key.Content) + } + sshKey = fmt.Sprintf("%s # user-%d", key.Content, key.OwnerID) + } else { + pubKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key.Content)) + if err != nil { + return false, err + } + + sshKeyMarshalled := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(pubKey))) + sshKey = fmt.Sprintf("%s user-%d", sshKeyMarshalled, key.OwnerID) } + // now the key is valid, the code below could only return template/IO related errors sbCmd := &strings.Builder{} err = setting.SSH.AuthorizedKeysCommandTemplateTemplate.Execute(sbCmd, map[string]any{ @@ -69,9 +100,7 @@ func writeAuthorizedStringForKey(key *PublicKey, w io.Writer) (keyValid bool, er return true, err } sshCommandEscaped := util.ShellEscape(sbCmd.String()) - sshKeyMarshalled := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(pubKey))) - sshKeyComment := fmt.Sprintf("user-%d", key.OwnerID) - _, err = fmt.Fprintf(w, tpl, sshCommandEscaped, sshKeyMarshalled, sshKeyComment) + _, err = fmt.Fprintf(w, tpl, sshCommandEscaped, sshKey) return true, err } diff --git a/models/asymkey/ssh_key_authorized_keys_test.go b/models/asymkey/ssh_key_authorized_keys_test.go new file mode 100644 index 0000000000000..36ed57a653786 --- /dev/null +++ b/models/asymkey/ssh_key_authorized_keys_test.go @@ -0,0 +1,90 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package asymkey + +import ( + "strings" + "testing" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" +) + +func TestWriteAuthorizedStringForKey(t *testing.T) { + defer test.MockVariableValue(&setting.AppPath, "/tmp/gitea")() + defer test.MockVariableValue(&setting.CustomConf, "/tmp/app.ini")() + writeKey := func(t *testing.T, key *PublicKey) (bool, string, error) { + sb := &strings.Builder{} + valid, err := writeAuthorizedStringForKey(key, sb) + return valid, sb.String(), err + } + const validKeyContent = `ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf` + + testValid := func(t *testing.T, key *PublicKey, expected string) { + valid, content, err := writeKey(t, key) + assert.True(t, valid) + assert.Equal(t, expected, content) + assert.NoError(t, err) + } + + testInvalid := func(t *testing.T, key *PublicKey) { + valid, content, err := writeKey(t, key) + assert.False(t, valid) + assert.Empty(t, content) + assert.Error(t, err) + } + + t.Run("PublicKey", func(t *testing.T) { + testValid(t, &PublicKey{ + OwnerID: 123, + Content: validKeyContent + " any-comment", + Type: KeyTypeUser, + }, `# gitea public key +command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf user-123 +`) + }) + + t.Run("PublicKeyWithNewLine", func(t *testing.T) { + testValid(t, &PublicKey{ + OwnerID: 123, + Content: validKeyContent + "\nany-more", // the new line should be ignored + Type: KeyTypeUser, + }, `# gitea public key +command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf user-123 +`) + }) + + t.Run("PublicKeyInvalid", func(t *testing.T) { + testInvalid(t, &PublicKey{ + OwnerID: 123, + Content: validKeyContent + "any-more", + Type: KeyTypeUser, + }) + }) + + t.Run("Principal", func(t *testing.T) { + testValid(t, &PublicKey{ + OwnerID: 123, + Content: "any-content", + Type: KeyTypePrincipal, + }, `# gitea public key +command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict any-content # user-123 +`) + }) + + t.Run("PrincipalInvalid", func(t *testing.T) { + testInvalid(t, &PublicKey{ + OwnerID: 123, + Content: "a b", + Type: KeyTypePrincipal, + }) + testInvalid(t, &PublicKey{ + OwnerID: 123, + Content: "a\nb", + Type: KeyTypePrincipal, + }) + }) +}