Skip to content

Commit

Permalink
Fix permissions for SSH
Browse files Browse the repository at this point in the history
  • Loading branch information
thomiceli committed May 27, 2024
1 parent fa58acc commit a50a0bc
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 20 deletions.
2 changes: 1 addition & 1 deletion internal/actions/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func Run(actionType int) {
case IndexGists:
functionToRun = indexGists
default:
panic("unhandled default case")
log.Error().Msg("Unknown action type")
}

functionToRun()
Expand Down
13 changes: 6 additions & 7 deletions internal/db/sshkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,12 @@ func GetSSHKeyByID(sshKeyId uint) (*SSHKey, error) {
return sshKey, err
}

func SSHKeyDoesExists(sshKeyContent string) (*SSHKey, error) {
sshKey := new(SSHKey)
err := db.
Where("content like ?", sshKeyContent+"%").
First(&sshKey).Error

return sshKey, err
func SSHKeyDoesExists(sshKeyContent string) (bool, error) {
var count int64
err := db.Model(&SSHKey{}).
Where("content = ?", sshKeyContent).
Count(&count).Error
return count > 0, err
}

func (sshKey *SSHKey) Create() error {
Expand Down
9 changes: 9 additions & 0 deletions internal/db/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ func GetUsersFromEmails(emailsSet map[string]struct{}) (map[string]*User, error)
return userMap, nil
}

func GetUserFromSSHKey(sshKey string) (*User, error) {
user := new(User)
err := db.
Joins("JOIN ssh_keys ON users.id = ssh_keys.user_id").
Where("ssh_keys.content = ?", sshKey).
First(&user).Error
return user, err
}

func SSHKeyExistsForUser(sshKey string, userId uint) (*SSHKey, error) {
key := new(SSHKey)
err := db.
Expand Down
1 change: 1 addition & 0 deletions internal/i18n/locales/en-US.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ settings.delete-ssh-key-confirm: Confirm deletion of SSH key
settings.ssh-key-added-at: Added
settings.ssh-key-never-used: Never used
settings.ssh-key-last-used: Last used
settings.ssh-key-exists: SSH key already exists
settings.change-username: Change username
settings.create-password: Create password
settings.create-password-help: Create your password to login to Opengist via HTTP
Expand Down
11 changes: 9 additions & 2 deletions internal/ssh/git_ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,18 @@ func runGitCommand(ch ssh.Channel, gitCmd string, key string, ip string) error {
// - gist is not found (obfuscation)
// - admin setting to require login is set to true
if verb == "receive-pack" ||
gist.Private == 2 ||
gist.Private == db.PrivateVisibility ||
gist.ID == 0 ||
!allowUnauthenticated {

pubKey, err := db.SSHKeyExistsForUser(key, gist.UserID)
var userToCheckPermissions *db.User
if gist.Private != db.PrivateVisibility && verb == "upload-pack" {
userToCheckPermissions, err = db.GetUserFromSSHKey(key)

Check failure on line 59 in internal/ssh/git_ssh.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to err (ineffassign)

Check failure on line 59 in internal/ssh/git_ssh.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to err (ineffassign)
} else {
userToCheckPermissions = &gist.User
}

pubKey, err := db.SSHKeyExistsForUser(key, userToCheckPermissions.ID)
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
log.Warn().Msg("Invalid SSH authentication attempt from " + ip)
Expand Down
4 changes: 2 additions & 2 deletions internal/ssh/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ func Start() {
sshConfig := &ssh.ServerConfig{
PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
strKey := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(key)))
_, err := db.SSHKeyDoesExists(strKey)
if err != nil {
exists, err := db.SSHKeyDoesExists(strKey)
if !exists {
if !errors.Is(err, gorm.ErrRecordNotFound) {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions internal/web/git_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func gitHttp(ctx echo.Context) error {

allow, err := auth.ShouldAllowUnauthenticatedGistAccess(ContextAuthInfo{ctx}, true)
if err != nil {
panic("impossible")
log.Fatal().Err(err).Msg("Cannot check if unauthenticated access is allowed")
}

// Shows basic auth if :
Expand Down Expand Up @@ -105,7 +105,14 @@ func gitHttp(ctx echo.Context) error {
return plainText(ctx, 404, "Check your credentials or make sure you have access to the Gist")
}

if ok, err := utils.Argon2id.Verify(authPassword, gist.User.Password); !ok || gist.User.Username != authUsername {
var userToCheckPermissions *db.User
if gist.Private != db.PrivateVisibility && isPull {
userToCheckPermissions, err = db.GetUserByUsername(authUsername)

Check failure on line 110 in internal/web/git_http.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to err (ineffassign)

Check failure on line 110 in internal/web/git_http.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to err (ineffassign)
} else {
userToCheckPermissions = &gist.User
}

if ok, err := utils.Argon2id.Verify(authPassword, userToCheckPermissions.Password); !ok {
if err != nil {
return errorRes(500, "Cannot verify password", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/web/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func makeCheckRequireLogin(isSingleGistAccess bool) echo.MiddlewareFunc {

allow, err := auth.ShouldAllowUnauthenticatedGistAccess(ContextAuthInfo{ctx}, isSingleGistAccess)
if err != nil {
panic("impossible")
log.Fatal().Err(err).Msg("Failed to check if unauthenticated access is allowed")
}

if !allow {
Expand Down
8 changes: 8 additions & 0 deletions internal/web/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ func sshKeysProcess(ctx echo.Context) error {
}
key.Content = strings.TrimSpace(string(ssh.MarshalAuthorizedKey(pubKey)))

if exists, err := db.SSHKeyDoesExists(key.Content); exists {
if err != nil {
return errorRes(500, "Cannot check if SSH key exists", err)
}
addFlash(ctx, tr(ctx, "settings.ssh-key-exists"), "error")
return redirect(ctx, "/settings")
}

if err := key.Create(); err != nil {
return errorRes(500, "Cannot add SSH key", err)
}
Expand Down
6 changes: 1 addition & 5 deletions internal/web/test/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,15 @@ func TestAnonymous(t *testing.T) {

}

func TestGitClonePull(t *testing.T) {
func TestGitOperations(t *testing.T) {
setup(t)
s, err := newTestServer()
require.NoError(t, err, "Failed to create test server")
defer teardown(t, s)

admin := db.UserDTO{Username: "thomas", Password: "thomas"}
register(t, s, admin)

// err = s.request("PUT", "/admin-panel/set-config", settingSet{"require-login", "1"}, 200)
// require.NoError(t, err)
s.sessionCookie = ""

register(t, s, db.UserDTO{Username: "fujiwara", Password: "fujiwara"})
s.sessionCookie = ""
register(t, s, db.UserDTO{Username: "kaguya", Password: "kaguya"})
Expand Down

0 comments on commit a50a0bc

Please sign in to comment.