diff --git a/cmd/minivpn/main.go b/cmd/minivpn/main.go index 88885f9..b9b0fdb 100644 --- a/cmd/minivpn/main.go +++ b/cmd/minivpn/main.go @@ -94,7 +94,10 @@ func main() { defer cancel() // create config from the passed options - vpncfg := config.NewConfig(opts...) + vpncfg, err := config.NewConfig(opts...) + if err != nil { + log.WithError(err).Fatal("cannot parse config") + } // create a vpn tun Device tun, err := tunnel.Start(ctx, &net.Dialer{}, vpncfg) diff --git a/internal/datachannel/common_test.go b/internal/datachannel/common_test.go index d277f6b..7400027 100644 --- a/internal/datachannel/common_test.go +++ b/internal/datachannel/common_test.go @@ -14,7 +14,8 @@ import ( ) func makeTestingSession() *session.Manager { - manager, err := session.NewManager(config.NewConfig()) + cfg, _ := config.NewConfig() + manager, err := session.NewManager(cfg) runtimex.PanicOnError(err, "could not get session manager") manager.SetRemoteSessionID(model.SessionID{0x01}) return manager diff --git a/internal/datachannel/service_test.go b/internal/datachannel/service_test.go index 96d9ba1..639d337 100644 --- a/internal/datachannel/service_test.go +++ b/internal/datachannel/service_test.go @@ -27,7 +27,8 @@ func TestService_StartWorkers(t *testing.T) { session := makeTestingSession() opts := makeTestingOptions(t, "AES-128-GCM", "sha512") - s.StartWorkers(config.NewConfig(config.WithOpenVPNOptions(opts)), workers, session) + cfg, _ := config.NewConfig(config.WithOpenVPNOptions(opts)) + s.StartWorkers(cfg, workers, session) keyReady <- makeTestingDataChannelKey() <-session.Ready diff --git a/internal/networkio/service_test.go b/internal/networkio/service_test.go index b43665c..a5328c4 100644 --- a/internal/networkio/service_test.go +++ b/internal/networkio/service_test.go @@ -42,10 +42,10 @@ func TestService_StartStopWorkers(t *testing.T) { NetworkToMuxer: &networkToMuxer, } - s.StartWorkers(config.NewConfig(config.WithLogger(log.Log)), workersManager, framingConn) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + s.StartWorkers(cfg, workersManager, framingConn) got := <-networkToMuxer - //time.Sleep(time.Millisecond * 10) workersManager.StartShutdown() workersManager.WaitWorkersShutdown() diff --git a/internal/reliabletransport/common_test.go b/internal/reliabletransport/common_test.go index 75698e2..ac58b46 100644 --- a/internal/reliabletransport/common_test.go +++ b/internal/reliabletransport/common_test.go @@ -18,7 +18,8 @@ import ( // initManagers initializes a workers manager and a session manager. func initManagers() (*workers.Manager, *session.Manager) { w := workers.NewManager(log.Log) - s, err := session.NewManager(config.NewConfig(config.WithLogger(log.Log))) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + s, err := session.NewManager(cfg) runtimex.PanicOnError(err, "cannot create session manager") return w, s } diff --git a/internal/reliabletransport/reliable_ack_test.go b/internal/reliabletransport/reliable_ack_test.go index 56cf3fe..2d65c40 100644 --- a/internal/reliabletransport/reliable_ack_test.go +++ b/internal/reliabletransport/reliable_ack_test.go @@ -19,9 +19,8 @@ import ( └─┬──┘ └──▲─┘ │ │ │ │ - │ │ - ▼ send - ack + ▼ | + ack send */ func TestReliable_ACK(t *testing.T) { if testing.Verbose() { @@ -129,7 +128,8 @@ func TestReliable_ACK(t *testing.T) { t0 := time.Now() // let the workers pump up the jam! - s.StartWorkers(config.NewConfig(config.WithLogger(log.Log)), workers, session) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + s.StartWorkers(cfg, workers, session) writer := vpntest.NewPacketWriter(dataIn) diff --git a/internal/reliabletransport/reliable_loss_test.go b/internal/reliabletransport/reliable_loss_test.go index 0a69cda..03ce556 100644 --- a/internal/reliabletransport/reliable_loss_test.go +++ b/internal/reliabletransport/reliable_loss_test.go @@ -251,7 +251,8 @@ func TestReliable_WithLoss(t *testing.T) { t0 := time.Now() // let the workers pump up the jam! - s.StartWorkers(config.NewConfig(config.WithLogger(log.Log)), workers, session) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + s.StartWorkers(cfg, workers, session) writer := vpntest.NewPacketWriter(dataIn) go writer.WriteSequenceWithFixedPayload(tt.args.inputSequence, tt.args.inputPayload, 3) diff --git a/internal/reliabletransport/reliable_reorder_test.go b/internal/reliabletransport/reliable_reorder_test.go index 0678d51..6c4fc0e 100644 --- a/internal/reliabletransport/reliable_reorder_test.go +++ b/internal/reliabletransport/reliable_reorder_test.go @@ -124,7 +124,8 @@ func TestReliable_Reordering_UP(t *testing.T) { t0 := time.Now() // let the workers pump up the jam! - s.StartWorkers(config.NewConfig(config.WithLogger(log.Log)), workers, session) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + s.StartWorkers(cfg, workers, session) writer := vpntest.NewPacketWriter(dataIn) initializeSessionIDForWriter(writer, session) diff --git a/internal/reliabletransport/service_test.go b/internal/reliabletransport/service_test.go index a173ead..b6f811c 100644 --- a/internal/reliabletransport/service_test.go +++ b/internal/reliabletransport/service_test.go @@ -43,10 +43,14 @@ func TestService_StartWorkers(t *testing.T) { }(), }, args: args{ - config: config.NewConfig(config.WithLogger(log.Log)), + config: func() *config.Config { + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + return cfg + }(), workersManager: workers.NewManager(log.Log), sessionManager: func() *session.Manager { - m, _ := session.NewManager(config.NewConfig(config.WithLogger(log.Log))) + cfg, _ := config.NewConfig(config.WithLogger(log.Log)) + m, _ := session.NewManager(cfg) return m }(), }, diff --git a/internal/tlssession/common_test.go b/internal/tlssession/common_test.go index c2f029c..24d79ad 100644 --- a/internal/tlssession/common_test.go +++ b/internal/tlssession/common_test.go @@ -8,7 +8,8 @@ import ( ) func makeTestingSession() *session.Manager { - manager, err := session.NewManager(config.NewConfig()) + cfg, _ := config.NewConfig() + manager, err := session.NewManager(cfg) runtimex.PanicOnError(err, "could not get session manager") manager.SetRemoteSessionID(model.SessionID{0x01}) return manager diff --git a/pkg/config/config.go b/pkg/config/config.go index 61930dc..bf4c4cd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,11 +1,11 @@ package config import ( + "fmt" "net" "github.com/apex/log" "github.com/ooni/minivpn/internal/model" - "github.com/ooni/minivpn/internal/runtimex" ) // Config contains options to initialize the OpenVPN tunnel. @@ -20,8 +20,9 @@ type Config struct { tracer model.HandshakeTracer } -// NewConfig returns a Config ready to intialize a vpn tunnel. -func NewConfig(options ...Option) *Config { +// NewConfig returns a Config ready to intialize a vpn tunnel. In case any error is raised during the +// initialization, it will be returned too. +func NewConfig(options ...Option) (*Config, error) { cfg := &Config{ openvpnOptions: &OpenVPNOptions{}, logger: log.Log, @@ -30,16 +31,18 @@ func NewConfig(options ...Option) *Config { for _, opt := range options { opt(cfg) } - return cfg + return cfg, nil } -// Option is an option you can pass to initialize minivpn. -type Option func(config *Config) +// Option is an option you can pass to initialize minivpn. It will return any error +// that should be propagated by the constructor. +type Option func(config *Config) error // WithLogger configures the passed [Logger]. func WithLogger(logger model.Logger) Option { - return func(config *Config) { + return func(config *Config) error { config.logger = logger + return nil } } @@ -50,8 +53,9 @@ func (c *Config) Logger() model.Logger { // WithHandshakeTracer configures the passed [HandshakeTracer]. func WithHandshakeTracer(tracer model.HandshakeTracer) Option { - return func(config *Config) { + return func(config *Config) error { config.tracer = tracer + return nil } } @@ -62,18 +66,24 @@ func (c *Config) Tracer() model.HandshakeTracer { // WithConfigFile configures OpenVPNOptions parsed from the given file. func WithConfigFile(configPath string) Option { - return func(config *Config) { + return func(config *Config) error { openvpnOpts, err := ReadConfigFile(configPath) - runtimex.PanicOnError(err, "cannot parse config file") - runtimex.PanicIfFalse(openvpnOpts.HasAuthInfo(), "missing auth info") + if err != nil { + return err + } + if !openvpnOpts.HasAuthInfo() { + return fmt.Errorf("%w: %s", ErrBadConfig, "missing auth info") + } config.openvpnOptions = openvpnOpts + return nil } } // WithOpenVPNOptions configures the passed OpenVPN options. func WithOpenVPNOptions(openvpnOptions *OpenVPNOptions) Option { - return func(config *Config) { + return func(config *Config) error { config.openvpnOptions = openvpnOptions + return nil } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 79ea154..b6279be 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -11,7 +11,7 @@ import ( func TestNewConfig(t *testing.T) { t.Run("default constructor does not fail", func(t *testing.T) { - c := NewConfig() + c, _ := NewConfig() if c.logger == nil { t.Errorf("logger should not be nil") } @@ -21,14 +21,14 @@ func TestNewConfig(t *testing.T) { }) t.Run("WithLogger sets the logger", func(t *testing.T) { testLogger := model.NewTestLogger() - c := NewConfig(WithLogger(testLogger)) + c, _ := NewConfig(WithLogger(testLogger)) if c.Logger() != testLogger { t.Errorf("expected logger to be set to the configured one") } }) t.Run("WithTracer sets the tracer", func(t *testing.T) { testTracer := model.HandshakeTracer(model.DummyTracer{}) - c := NewConfig(WithHandshakeTracer(testTracer)) + c, _ := NewConfig(WithHandshakeTracer(testTracer)) if c.Tracer() != testTracer { t.Errorf("expected tracer to be set to the configured one") } @@ -36,7 +36,7 @@ func TestNewConfig(t *testing.T) { t.Run("WithConfigFile sets OpenVPNOptions after parsing the configured file", func(t *testing.T) { configFile := writeValidConfigFile(t.TempDir()) - c := NewConfig(WithConfigFile(configFile)) + c, _ := NewConfig(WithConfigFile(configFile)) opts := c.OpenVPNOptions() if opts.Proto.String() != "udp" { t.Error("expected proto udp") diff --git a/tests/integration/main.go b/tests/integration/main.go index 1d9e15a..2bdf0d1 100644 --- a/tests/integration/main.go +++ b/tests/integration/main.go @@ -17,6 +17,7 @@ import ( "github.com/ooni/minivpn/extras/ping" "github.com/ooni/minivpn/internal/networkio" "github.com/ooni/minivpn/internal/tun" + "github.com/ooni/minivpn/pkg/config" ) const ( @@ -117,7 +118,7 @@ func main() { defer os.RemoveAll(tmp) // clean up fmt.Println("launching docker") - config, pool, resource, err := launchDocker("AES-256-GCM", "SHA256") + configInfo, pool, resource, err := launchDocker("AES-256-GCM", "SHA256") if err != nil { log.WithError(err).Fatal("cannot start docker") } @@ -131,12 +132,12 @@ func main() { defer cfgFile.Close() fmt.Println("Config written to: " + cfgFile.Name()) - if _, err = cfgFile.Write(config); err != nil { + if _, err = cfgFile.Write(configInfo); err != nil { log.WithError(err).Fatal("Failed to write config to temporary file") } // actual test begins - vpnConfig := config.NewConfig(config.WithConfigFile(cfgFile.Name())) + vpnConfig, err := config.NewConfig(config.WithConfigFile(cfgFile.Name())) dialer := networkio.NewDialer(log.Log, &net.Dialer{}) conn, err := dialer.DialContext(context.TODO(), vpnConfig.Remote().Protocol, vpnConfig.Remote().Endpoint)