tests: adapt integration tests to run against the refactored code (#65)

As part of a previous commit ($64), I reverted a recent "fix" that was
inverting the local-remote session id check - the error in that commit
was to change the check to match the description in the comment. In
reality, it was the comment that was wrong.

This commit enables integration tests for the new internal path to avoid
this kind of errors in the future.

Some test code was removed, because the script to parse certs etc was
written before minivpn could parse inline credentials. not needed
anymore.

For more context: the reason why we cannot check remote session id in
all packets is because remote-session-id only present if the packet has
an ack array. The handshake was broken by the previous change, but I did
not catch it because the integration tests were not running for the
refactored codebase.

.github/workflows/build-refactor.yml

@@ -62,5 +62,5 @@ jobs:
         with:
           go-version: '1.21'
       - name: run integration tests
-        run: go test -v ./tests/integration
+        run: go run ./tests/integration
 

Makefile

@@ -57,7 +57,6 @@ integration-server:
 test-fetch-config:
 	rm -rf data/tests
 	mkdir -p data/tests && curl 172.17.0.2:8080/ > data/tests/config
-	cd data/tests && ../../tests/integration/extract.sh config
 
 test-ping-local:
 	# run the integration-server first
@@ -72,10 +71,12 @@ qa:
 	@rm -rf data/tests
 	@mkdir -p data/tests && curl 172.17.0.2:8080/ > data/tests/config
 	@sleep 1
-	@cd data/tests && ../../tests/integration/extract.sh config
 	./minivpn -c data/tests/config -t 172.17.0.1 -n ${COUNT} ping
 	@docker stop ovpn1
 
+integration:
+	go run ./tests/integration
+
 filternet-qa:
 	cd tests/qa && ./run-filternet.sh remote-block-all
 

tests/integration/openvpn_test.go → tests/integration/main.go

@@ -1,29 +1,26 @@
-//build: +integration
 package main
 
 import (
 	"bufio"
-	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
-	"log"
+	"io"
+	"net"
 	"net/http"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"testing"
-	"time"
 
+	"github.com/apex/log"
 	"github.com/ory/dockertest/v3"
 	dc "github.com/ory/dockertest/v3/docker"
 
 	"github.com/ooni/minivpn/extras/ping"
-	"github.com/ooni/minivpn/vpn"
+	"github.com/ooni/minivpn/internal/model"
+	"github.com/ooni/minivpn/internal/networkio"
+	"github.com/ooni/minivpn/internal/tun"
 )
 
 const (
-	parseConfig = "extract.sh"
 	dockerImage = "ainghazal/openvpn"
 	dockerTag   = "latest"
 )
@@ -34,14 +31,14 @@ var (
 )
 
 func copyFile(src, dst string) error {
-	input, err := ioutil.ReadFile(src)
+	input, err := os.ReadFile(src)
 	if err != nil {
 		fmt.Println(err)
 		return nil
 	}
 
 	dstFile := filepath.Join(dst, src)
-	err = ioutil.WriteFile(dstFile, input, 0744)
+	err = os.WriteFile(dstFile, input, 0744)
 	if err != nil {
 		fmt.Println("Error creating", dstFile)
 		return err
@@ -73,15 +70,15 @@ func launchDocker(cipher, auth string) ([]byte, *dockertest.Pool, *dockertest.Re
 	// the minio client does not do service discovery for you (i.e. it does not check if connection can be established), so we have to use the health check
 	var config []byte
 	if err := pool.Retry(func() error {
-		url := fmt.Sprintf("http://localhost:8080/")
+		url := "http://localhost:8080/"
 		resp, err := http.Get(url)
 		if err != nil {
 			return err
 		}
 		if resp.StatusCode != http.StatusOK {
 			return fmt.Errorf("status code not OK")
 		}
-		config, err = ioutil.ReadAll(resp.Body)
+		config, err = io.ReadAll(resp.Body)
 		if err != nil {
 			panic(err)
 		}
@@ -94,94 +91,75 @@ func launchDocker(cipher, auth string) ([]byte, *dockertest.Pool, *dockertest.Re
 }
 
 func stopContainer(p *dockertest.Pool, res *dockertest.Resource) {
-	fmt.Println("Stopping container")
+	fmt.Println("[!] Stopping container")
 	if err := p.Purge(res); err != nil {
-		log.Printf("Could not purge resource: %s\n", err)
+		log.Warnf("Could not purge resource: %s\n", err)
 	}
 }
 
-func TestClientAES256GCM(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping test in short mode.")
-	}
-	tmp := t.TempDir()
-
-	copyFile(parseConfig, tmp)
-	os.Chdir(tmp)
-	err := os.Chmod(parseConfig, 0700)
+func readLines(f string) ([]string, error) {
+	var ll []string
+	rf, err := os.Open(f)
 	if err != nil {
-		log.Fatal(err)
+		return ll, err
+	}
+	defer rf.Close()
+	fs := bufio.NewScanner(rf)
+	fs.Split(bufio.ScanLines)
+	for fs.Scan() {
+		ll = append(ll, fs.Text())
 	}
+	return ll, nil
+}
 
-	config, pool, resource, err := launchDocker("AES-256-GCM", "SHA256")
+// This main function exercises AES256GCM
+func main() {
+	tmp, err := os.MkdirTemp("", "minivpn-integration-test")
+	defer os.RemoveAll(tmp) // clean up
 
+	fmt.Println("launching docker")
+	config, pool, resource, err := launchDocker("AES-256-GCM", "SHA256")
 	if err != nil {
-		log.Fatal(err)
+		log.WithError(err).Fatal("cannot start docker")
 	}
 	// when all test done, time to kill and remove the container
 	defer stopContainer(pool, resource)
 
-	cfgFile, err := ioutil.TempFile(tmp, "minivpn-e2e-")
-	defer cfgFile.Close()
+	cfgFile, err := os.CreateTemp(tmp, "minivpn-e2e-")
 	if err != nil {
-		log.Fatal("Cannot create temporary file", err)
+		log.WithError(err).Fatal("Cannot create temporary file")
 	}
+	defer cfgFile.Close()
 	fmt.Println("Config written to: " + cfgFile.Name())
 
 	if _, err = cfgFile.Write(config); err != nil {
-		log.Fatal("Failed to write config to temporary file", err)
+		log.WithError(err).Fatal("Failed to write config to temporary file")
 	}
 
-	// execute the extract.sh shell script, to process key blocks piecewise
-	cmd := exec.Command("./"+parseConfig, cfgFile.Name())
-	cmd.Dir = tmp
-	var out bytes.Buffer
-	cmd.Stdout = &out
-	cmd.Run()
+	// actual test begins
+	vpnConfig := model.NewConfig(model.WithConfigFile(cfgFile.Name()))
 
+	dialer := networkio.NewDialer(log.Log, &net.Dialer{})
+	conn, err := dialer.DialContext(context.TODO(), vpnConfig.Remote().Protocol, vpnConfig.Remote().Endpoint)
 	if err != nil {
-		log.Fatal(err)
+		log.WithError(err).Fatal("dial error")
 	}
 
-	c, err := readLines("config")
-	fmt.Println("Remote:", c[len(c)-1])
-	// can assert that this is a remote line
-
-	// actual test begins
-	opt, err := vpn.NewOptionsFromFilePath(filepath.Join(tmp, "config"))
+	tunnel, err := tun.StartTUN(context.Background(), conn, vpnConfig)
 	if err != nil {
-		log.Fatalf("Could not parse file: %s", err)
+		log.WithError(err).Fatal("cannot start tunnel")
 	}
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
-	defer cancel()
 
-	tunnel := vpn.NewClientFromOptions(opt)
-	tunnel.Start(ctx)
 	pinger := ping.New(target, tunnel)
 	pinger.Count = count
-	err = pinger.Run(ctx)
+	err = pinger.Run(context.Background())
 	defer pinger.Stop()
 	if err != nil {
-		log.Fatalf("VPN Error: %s", err)
+		log.WithError(err).Fatalf("VPN Error")
 	}
 	if pinger.PacketLoss() != 0 {
 		log.Fatalf("packet loss is not zero")
 	}
 	// let's assert something wise about the pings
 	// can we parse the logs? get initialization etc
 }
-
-func readLines(f string) ([]string, error) {
-	var ll []string
-	rf, err := os.Open(f)
-	defer rf.Close()
-	if err != nil {
-		return ll, err
-	}
-	fs := bufio.NewScanner(rf)
-	fs.Split(bufio.ScanLines)
-	for fs.Scan() {
-		ll = append(ll, fs.Text())
-	}
-	return ll, nil
-}