Skip to content

Commit 6c62789

Browse files
cfergeaucfergeau
authored
Merge pull request crc-org#195 from cfergeau/unixsocket
virtionet: Fix 'path too long' issues with virtio-net unixgram Support
2 parents b02a79d + 7e6fd83 commit 6c62789

File tree

3 files changed

+51
-42
lines changed

3 files changed

+51
-42
lines changed

‎.github/workflows/compile.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ jobs:
2323
fail-fast: false
2424
matrix:
2525
os:
26-
- macOS-12
2726
- macOS-13
2827
- macOS-14
2928
steps:

‎pkg/vf/virtionet.go‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package vf
22

33
import (
44
"fmt"
5+
"math/rand"
56
"net"
67
"os"
78
"os/signal"
@@ -19,36 +20,46 @@ type VirtioNet struct {
1920
localAddr *net.UnixAddr
2021
}
2122

22-
func localUnixSocketPath() (string, error) {
23-
homeDir, err := os.UserHomeDir()
23+
func localUnixSocketPath(dir string) (string, error) {
24+
// unix socket endpoints are filesystem paths, but their max length is
25+
// quite small (a bit over 100 bytes).
26+
// In this function we try to build a filename which is relatively
27+
// unique, not easily guessable (to prevent hostile collisions), and
28+
// short (`os.CreateTemp` filenames are a bit too long)
29+
//
30+
// os.Getpid() is unique but guessable. We append a short 16 bit random
31+
// number to it. We only use hex values to make the representation more
32+
// compact
33+
filename := filepath.Join(dir, fmt.Sprintf("vfkit-%x-%x.sock", os.Getpid(), rand.Int31n(0xffff))) //#nosec G404 -- no need for crypto/rand here
34+
35+
tmpFile, err := os.OpenFile(filename, os.O_CREATE|os.O_EXCL, 0600)
2436
if err != nil {
2537
return "", err
2638
}
27-
dir := filepath.Join(homeDir, "Library", "Application Support", "vfkit")
28-
if err := os.MkdirAll(dir, 0755); err != nil {
29-
return "", err
30-
}
31-
tmpFile, err := os.CreateTemp(dir, fmt.Sprintf("net-%d-*.sock", os.Getpid()))
32-
if err != nil {
33-
return "", err
34-
}
35-
// slightly racy, but this is in a directory only user-writable
39+
// slightly racy, but hopefully this is in a directory only user-writable
3640
defer tmpFile.Close()
3741
defer os.Remove(tmpFile.Name())
3842

3943
return tmpFile.Name(), nil
4044
}
4145

46+
// path for unixgram sockets must be less than 104 bytes on macOS
47+
const maxUnixgramPathLen = 104
48+
4249
func (dev *VirtioNet) connectUnixPath() error {
50+
4351
remoteAddr := net.UnixAddr{
4452
Name: dev.UnixSocketPath,
4553
Net: "unixgram",
4654
}
47-
localSocketPath, err := localUnixSocketPath()
55+
localSocketPath, err := localUnixSocketPath(filepath.Dir(dev.UnixSocketPath))
4856
if err != nil {
4957
return err
5058
}
51-
// FIXME: need to remove localSocketPath at process exit
59+
if len(localSocketPath) >= maxUnixgramPathLen {
60+
return fmt.Errorf("unixgram path '%s' is too long: %d >= %d bytes", localSocketPath, len(localSocketPath), maxUnixgramPathLen)
61+
}
62+
// FIXME: need to remove localSocketPath at process exit
5263
localAddr := net.UnixAddr{
5364
Name: localSocketPath,
5465
Net: "unixgram",

‎pkg/vf/virtionet_test.go‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package vf
22

33
import (
4-
"bytes"
5-
"fmt"
6-
"math/rand"
74
"net"
85
"os"
96
"path/filepath"
@@ -14,16 +11,33 @@ import (
1411
"github.com/stretchr/testify/require"
1512
)
1613

17-
func testConnectUnixgram(t *testing.T) error {
18-
unixSocketPath := filepath.Join("/tmp", fmt.Sprintf("vnet-test-%x.sock", rand.Int31n(0xffff))) //#nosec G404 -- no need for crypto/rand here
14+
func sourceSocketPath(t *testing.T, sourcePathLen int) (string, func()) {
15+
// the 't.sock' name is chosen to be shorter than what
16+
// localUnixSocketPath will generate so that the source socket path
17+
// will not exceed the 104 byte limit while the destination socket path
18+
// will, and will trigger an error
19+
const sourceSocketName = "t.sock"
20+
tmpDir := "/tmp"
21+
subDirLen := sourcePathLen - len(tmpDir) - 2*len("/") - len(sourceSocketName) - 1
22+
subDir := filepath.Join(tmpDir, strings.Repeat("a", subDirLen))
23+
err := os.Mkdir(subDir, 0700)
24+
require.NoError(t, err)
25+
unixSocketPath := filepath.Join(subDir, sourceSocketName)
26+
require.Equal(t, len(unixSocketPath), sourcePathLen-1)
27+
return unixSocketPath, func() { os.RemoveAll(subDir) }
28+
29+
}
30+
func testConnectUnixgram(t *testing.T, sourcePathLen int) error {
31+
unixSocketPath, closer := sourceSocketPath(t, sourcePathLen)
32+
defer closer()
33+
1934
addr, err := net.ResolveUnixAddr("unixgram", unixSocketPath)
2035
require.NoError(t, err)
2136

2237
l, err := net.ListenUnixgram("unixgram", addr)
2338
require.NoError(t, err)
2439

2540
defer l.Close()
26-
defer os.Remove(unixSocketPath)
2741

2842
dev := &VirtioNet{
2943
&config.VirtioNet{
@@ -37,46 +51,31 @@ func testConnectUnixgram(t *testing.T) error {
3751

3852
func TestConnectUnixPath(t *testing.T) {
3953
t.Run("Successful connection - no error", func(t *testing.T) {
40-
err := testConnectUnixgram(t)
54+
// 50 is an arbitrary number, small enough for the 104 bytes limit not to be exceeded
55+
err := testConnectUnixgram(t, 50)
4156
require.NoError(t, err)
4257
})
4358

4459
t.Run("Failed connection - End socket longer than 104 bytes", func(t *testing.T) {
45-
// Retrieve HOME env variable (used by the os.UserHomeDir)
46-
origUserHome := os.Getenv("HOME")
47-
defer func() {
48-
os.Setenv("HOME", origUserHome)
49-
}()
50-
51-
// Create a string of 100 bytes to update the user home to be sure to create a socket path > 104 bytes
52-
b := bytes.Repeat([]byte("a"), 100)
53-
subDir := string(b)
54-
55-
// Update HOME env so os.UserHomeDir returns the update path with subfolder
56-
updatedUserHome := filepath.Join(origUserHome, subDir)
57-
os.Setenv("HOME", updatedUserHome)
58-
defer os.RemoveAll(updatedUserHome)
59-
60-
err := testConnectUnixgram(t)
60+
err := testConnectUnixgram(t, maxUnixgramPathLen)
6161
// It should return an error
6262
require.Error(t, err)
63-
require.ErrorContains(t, err, "invalid argument")
63+
require.ErrorContains(t, err, "is too long")
6464
})
6565
}
6666

6767
func TestLocalUnixSocketPath(t *testing.T) {
6868
t.Run("Success case - Creates temporary socket path", func(t *testing.T) {
6969
// Retrieve HOME env variable (used by the os.UserHomeDir)
70-
userHome := os.Getenv("HOME")
70+
socketDir := t.TempDir()
7171

72-
path, err := localUnixSocketPath()
72+
path, err := localUnixSocketPath(socketDir)
7373

7474
// Assert successful execution
7575
require.NoError(t, err)
7676

7777
// Check if path starts with the expected prefix
78-
expectedPrefix := filepath.Join(userHome, "Library", "Application Support", "vfkit")
79-
require.Truef(t, strings.HasPrefix(path, expectedPrefix), "Path doesn't start with expected prefix: %v", path)
78+
require.Truef(t, strings.HasPrefix(path, socketDir), "Path doesn't start with expected prefix: %v", path)
8079

8180
// Check if path ends with a socket extension
8281
require.Equalf(t, ".sock", filepath.Ext(path), "Path doesn't end with .sock extension: %v, ext is %v", path, filepath.Ext(path))

0 commit comments

Comments
 (0)