Skip to content

Commit

Permalink
Merge pull request #36 from hyperf/perm
Browse files Browse the repository at this point in the history
fix: check permission
  • Loading branch information
Reasno authored Mar 17, 2021
2 parents 6fdd702 + f44019e commit 3370649
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 31 deletions.
6 changes: 2 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ sudo: required

matrix:
include:
- php: 7.2
env: SW_VERSION="4.5.2"
- php: 7.3
env: SW_VERSION="4.5.2"
env: SW_VERSION="4.6.4"
- php: 7.4
env: SW_VERSION="4.5.2"
env: SW_VERSION="4.6.4"

services:
- mongodb
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/fatih/pool v3.0.0+incompatible
github.com/oklog/run v1.1.0
github.com/pkg/errors v0.9.1
go.mongodb.org/mongo-driver v1.3.3
github.com/spiral/goridge/v2 v2.4.4
go.mongodb.org/mongo-driver v1.3.3
golang.org/x/sys v0.0.0-20190531175056-4c3a928424d2
)
2 changes: 1 addition & 1 deletion pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func testHas(t *testing.T) {
}

func TestAll(t *testing.T) {
gotask.SetGo2PHPAddress("/tmp/test.sock")
gotask.SetGo2PHPAddress("../../tests/test.sock")
for i := 0; i < 50; i++ {
t.Run("testAll", func(t *testing.T) {
t.Parallel()
Expand Down
65 changes: 51 additions & 14 deletions pkg/gotask/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/rpc"
"os"
"os/signal"
"path"
"syscall"
"time"

Expand Down Expand Up @@ -79,14 +80,16 @@ func Run() error {

if *address != "" {
network, addr := parseAddr(*address)
if err := clearAddr(network, addr); err != nil {
return errors.Wrap(err, "Cannot remove existing unix socket")
cleanup, err := checkAddr(network, addr)
if err != nil {
return errors.Wrap(err, "cannot remove existing unix socket")
}
defer cleanup()

ln, err := net.Listen(network, addr)
if err != nil {
return errors.Wrap(err, "Unable to listen")
return errors.Wrap(err, "unable to listen")
}
defer clearAddr(network, addr)

g.Add(func() error {
for {
Expand Down Expand Up @@ -135,16 +138,6 @@ func Run() error {
return g.Run()
}

func clearAddr(network string, addr string) error {
if network != "unix" {
return nil
}
if _, err := os.Stat(addr); os.IsNotExist(err) {
return nil
}
return os.Remove(addr)
}

// Add an actor (function) to the group. Each actor must be pre-emptable by an
// interrupt function. That is, if interrupt is invoked, execute should return.
// Also, it must be safe to call interrupt even after execute has returned.
Expand All @@ -154,3 +147,47 @@ func clearAddr(network string, addr string) error {
func Add(execute func() error, interrupt func(error)) {
g.Add(execute, interrupt)
}

func checkAddr(network, addr string) (func(), error) {
if network != "unix" {
return func() {}, nil
}
if _, err := os.Stat(addr); !os.IsNotExist(err) {
return func() {}, os.Remove(addr)
}
if err := os.MkdirAll(path.Dir(addr), os.ModePerm); err != nil {
return func() {}, err
}
if ok, err := isWritable(path.Dir(addr)); err != nil || !ok {
return func() {}, errors.Wrap(err, "socket directory is not writable")
}
return func() { os.Remove(addr) }, nil
}

func isWritable(path string) (isWritable bool, err error) {
info, err := os.Stat(path)
if err != nil {
return false, err
}

if !info.IsDir() {
return false, fmt.Errorf("%s isn't a directory", path)
}

// Check if the user bit is enabled in file permission
if info.Mode().Perm()&(1<<(uint(7))) == 0 {
return false, fmt.Errorf("write permission bit is not set on this %s for user", path)
}

var stat syscall.Stat_t
if err = syscall.Stat(path, &stat); err != nil {
return false, err
}

err = nil
if uint32(os.Geteuid()) != stat.Uid {
return false, errors.Errorf("user doesn't have permission to write to %s", path)
}

return true, nil
}
24 changes: 18 additions & 6 deletions pkg/gotask/server_test.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,40 @@
package gotask

import (
"io/ioutil"
"log"
"os"
"testing"
)

func TestClearAddr(t *testing.T) {
if err := clearAddr("unix", "/tmp/non-exist.sock"); err != nil {
t.Errorf("clearAddr should not return error for non-exist files")
dir, _ := ioutil.TempDir("", "")
defer os.Remove(dir)

if _, err := checkAddr("unix", dir+"/non-exist.sock"); err != nil {
t.Errorf("checkAddr should not return error for non-exist files")
}
if err := clearAddr("tcp", "127.0.0.1:6000"); err != nil {
t.Errorf("clearAddr should not return error for tcp ports")
if _, err := checkAddr("tcp", "127.0.0.1:6000"); err != nil {
t.Errorf("checkAddr should not return error for tcp ports")
}
file, err := os.Create("/tmp/temp.sock")
if err != nil {
log.Fatal(err)
}
defer file.Close()
if err := clearAddr("unix", "/tmp/temp.sock"); err != nil {
t.Errorf("clearAddr should be able to clear unix socket")
if _, err := checkAddr("unix", "/tmp/temp.sock"); err != nil {
t.Errorf("checkAddr should be able to clear unix socket")
}
_, err = os.Stat("/tmp/temp.sock")
if !os.IsNotExist(err) {
t.Errorf("unix socket are not cleared, %v", err)
}

if _, err := checkAddr("unix", dir+"/path/to/dir/temp.sock"); err != nil {
t.Errorf("checkAddr should be able to create directory if not exist")
}

if _, err := checkAddr("unix", "/private/temp.sock"); err == nil {
t.Error("unix socket shouldn't be created")
}
}
2 changes: 1 addition & 1 deletion pkg/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func testInfo(t *testing.T) {

func TestAll(t *testing.T) {
t.Parallel()
gotask.SetGo2PHPAddress("/tmp/test.sock")
gotask.SetGo2PHPAddress("../../tests/test.sock")
for i := 0; i < 50; i++ {
t.Run("testAll", func(t *testing.T) {
t.Parallel()
Expand Down
8 changes: 7 additions & 1 deletion src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,14 @@ public function __invoke(): array

public static function address()
{
if (defined(BASE_PATH)) {
$root = BASE_PATH . '/runtime';
} else {
$root = '/tmp';
}

$appName = env('APP_NAME');
$socketName = $appName . '_' . uniqid();
return "/tmp/{$socketName}.sock";
return $root . "/{$socketName}.sock";
}
}
2 changes: 1 addition & 1 deletion tests/Cases/AbstractTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*/
abstract class AbstractTestCase extends TestCase
{
const UNIX_SOCKET = '/tmp/test.sock';
const UNIX_SOCKET = __DIR__ . '/test.sock';

public function setUp(): void
{
Expand Down
2 changes: 1 addition & 1 deletion tests/Cases/CoroutineSocketTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
class CoroutineSocketTest extends AbstractTestCase
{
const UNIX_SOCKET = '/tmp/test.sock';
const UNIX_SOCKET = __DIR__ . '/test.sock';

/**
* @var Process
Expand Down
2 changes: 1 addition & 1 deletion tests/TestServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
require __DIR__ . '/../vendor/autoload.php';
define('BASE_PATH', __DIR__);

const ADDR = '/tmp/test.sock';
const ADDR = __DIR__ . '/test.sock';
@unlink(ADDR);
$container = new Container(new DefinitionSource([], new ScanConfig()));
$container->set(ConfigInterface::class, new Config([
Expand Down

0 comments on commit 3370649

Please sign in to comment.