[refactor] telnet dispatcher, handle errors

12 files changed, 223 insertions, 54 deletions

diff --git a/config/CONFIG b/config/CONFIG
index 02d60cb..9d2f5c5 100755
--- a/config/CONFIG
+++ b/config/CONFIG
@@ -13,7 +13,10 @@ export VPNWEB_AUTH_SECRET=othaoPhejei8aidaeghaiVohie4du
 export VPNWEB_SIP_USER=leap
 export VPNWEB_SIP_PASS="Kohapassword1!"
 export VPNWEB_SIP_HOST="localhost"
-export VPNWEB_SIP_PORT="6001"
+export VPNWEB_SIP_PORT="7001"
 export VPNWEB_SIP_LIBR_LOCATION=testlibrary
 export VPNWEB_SIP_TERMINATOR="\r"
 
+# debug
+
+export VPNWEB_DEBUG_AUTH=yes
diff --git a/go.mod b/go.mod
index b81e92b..59d706c 100644
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/go-martini/martini v0.0.0-20170121215854-22fa46961aab // indirect
 	github.com/gorilla/mux v1.7.3 // indirect
+	github.com/linxiaozhi/go-telnet v0.0.0-20190217183315-a500ff0c2efc
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/prometheus/client_golang v1.4.0
diff --git a/go.sum b/go.sum
index 22dabc3..572ac69 100644
--- a/go.sum
+++ b/go.sum
@@ -80,6 +80,8 @@ github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFB
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/linxiaozhi/go-telnet v0.0.0-20190217183315-a500ff0c2efc h1:FQTpORWGxHvz83hOXnlbdSuraUA4fumQ0dPDdCaxN7g=
+github.com/linxiaozhi/go-telnet v0.0.0-20190217183315-a500ff0c2efc/go.mod h1:kkwxiW1i3uS0OgPQO4bkgBDeJuinEx1uR5DOGE12IQA=
 github.com/mattn/go-gtk v0.0.0-20180216084204-5a311a1830ab/go.mod h1:PwzwfeB5syFHXORC3MtPylVcjIoTDT/9cvkKpEndGVI=
 github.com/mattn/go-gtk v0.0.0-20191030024613-af2e013261f5/go.mod h1:PwzwfeB5syFHXORC3MtPylVcjIoTDT/9cvkKpEndGVI=
 github.com/mattn/go-pointer v0.0.0-20171114154726-1d30dc4b6f28/go.mod h1:2zXcozF6qYGgmsG+SeTZz3oAbFLdD3OWqnUbNvJZAlc=
diff --git a/pkg/auth/anon/auth.go b/pkg/auth/anon/auth.go
index 52d2827..52b9ffc 100644
--- a/pkg/auth/anon/auth.go
+++ b/pkg/auth/anon/auth.go
@@ -31,8 +31,8 @@ func (a *Authenticator) GetLabel() string {
 	return Label
 }
 
-func (a *Authenticator) CheckCredentials(cred *creds.Credentials) bool {
-	return true
+func (a *Authenticator) CheckCredentials(cred *creds.Credentials) (bool, error) {
+	return true, nil
 }
 
 func (a *Authenticator) NeedsCredentials() bool {
diff --git a/pkg/auth/interfaces.go b/pkg/auth/interfaces.go
index 619eff8..e5a24aa 100644
--- a/pkg/auth/interfaces.go
+++ b/pkg/auth/interfaces.go
@@ -22,5 +22,5 @@ import (
 type Authenticator interface {
 	GetLabel() string
 	NeedsCredentials() bool
-	CheckCredentials(*creds.Credentials) bool
+	CheckCredentials(*creds.Credentials) (bool, error)
 }
diff --git a/pkg/auth/sip2/auth.go b/pkg/auth/sip2/auth.go
index 47733c2..0fe596b 100644
--- a/pkg/auth/sip2/auth.go
+++ b/pkg/auth/sip2/auth.go
@@ -16,7 +16,6 @@
 package sip2
 
 import (
-	"errors"
 	"log"
 	"os"
 
@@ -72,14 +71,11 @@ func initializeSipConnection(skipConnect bool) (sipClient, error) {
 		return sip, nil
 	}
 
-	ok, err := sip.Connect()
+	sip.setCredentials(user, pass)
+	_, err := sip.doConnect()
 	if err != nil {
 		return sip, err
 	}
-	ok = sip.Login(user, pass)
-	if !ok {
-		return sip, errors.New("SIP login error")
-	}
 	return sip, nil
 }
 
diff --git a/pkg/auth/sip2/client.go b/pkg/auth/sip2/client.go
index 9adf218..9686e4b 100644
--- a/pkg/auth/sip2/client.go
+++ b/pkg/auth/sip2/client.go
@@ -17,8 +17,9 @@ package sip2
 
 import (
 	"0xacab.org/leap/vpnweb/pkg/auth/creds"
+	"errors"
 	"fmt"
-	"github.com/reiver/go-telnet"
+	"github.com/linxiaozhi/go-telnet"
 	"log"
 	"time"
 )
@@ -33,43 +34,121 @@ type sipClient struct {
 	host     string
 	port     string
 	location string
-	conn     *telnet.Conn
+	user     string
+	pass     string
+	conn     gote.Connection
+	reqQueue chan request
 	parser   *Parser
 }
 
+type request struct {
+	msg      string
+	respChan chan response
+}
+
+type response struct {
+	msg string
+	err error
+}
+
 func newClient(host, port, location string) sipClient {
-	c := sipClient{host, port, location, nil, nil}
-	c.parser = getParser()
+	reqQ := make(chan request)
+	parser := getParser()
+	c := sipClient{host, port, location, "", "", nil, reqQ, parser}
 	return c
 }
 
-func (c *sipClient) Connect() (bool, error) {
-	conn, err := telnet.DialTo(c.host + ":" + c.port)
+func (c *sipClient) startDispatcher() {
+	go func() {
+		for {
+			req := <-c.reqQueue
+			resp, err := c.handleRequest(req.msg)
+			req.respChan <- response{resp, err}
+		}
+	}()
+}
+
+func (c *sipClient) sendRequest(msg string) (string, error) {
+	respChan := make(chan response)
+	c.reqQueue <- request{msg, respChan}
+	resp := <-respChan
+	return resp.msg, resp.err
+}
+
+func (c *sipClient) handleRequest(msg string) (string, error) {
+	err := telnetSend(c.conn, msg)
+	if err != nil {
+		return "", err
+	}
+	resp, err := telnetRead(c.conn)
+	return resp, err
+}
+
+func (c *sipClient) doConnect() (bool, error) {
+	_, err := c.connect()
+	if err != nil {
+		return false, err
+	}
+	_, err = c.login()
+	if err != nil {
+		return false, err
+	}
+	c.startDispatcher()
+	return true, nil
+
+}
+
+func (c *sipClient) setCredentials(user, pass string) {
+	c.user = user
+	c.pass = pass
+}
+
+/* TODO heartbeat function -------------------------------------------------- */
+
+func (c *sipClient) connect() (bool, error) {
+	conn, err := gote.DialTimeout("tcp", c.host+":"+c.port, time.Second*2)
 	if nil != err {
-		log.Println("error", err)
+		log.Println("connect error:", err)
 		return false, err
 	}
 	c.conn = conn
 	return true, nil
 }
 
-func (c *sipClient) Login(user, pass string) bool {
-	loginStr := fmt.Sprintf(loginRequestTemplate, user, pass, c.location)
+func (c *sipClient) login() (bool, error) {
+	loginStr := fmt.Sprintf(loginRequestTemplate, c.user, c.pass, c.location)
 	if nil == c.conn {
 		fmt.Println("error! null connection")
+		return false, errors.New("null connection received")
+	}
+
+	err := telnetSend(c.conn, loginStr)
+	if err != nil {
+		log.Println("Error while sending request")
+		return false, err
+	}
+
+	loginResp, err := telnetRead(c.conn)
+	if err != nil {
+		log.Println("login error on read: ", err)
+		return false, err
+	}
+
+	msg, err := c.parseResponse(loginResp)
+	if err != nil {
+		log.Println("login error:", err)
+		return false, err
 	}
-	telnetSend(c.conn, loginStr)
-	loginResp := telnetRead(c.conn)
-	msg := c.parseResponse(loginResp)
 	if value, ok := c.parser.getFixedFieldValue(msg, okVal); ok && value == trueVal {
-		return true
+		log.Println("SIP admin login OK")
+		return true, nil
 	}
-	return false
+	return false, nil
 }
 
-func (c *sipClient) parseResponse(txt string) *message {
-	msg := c.parser.parseMessage(txt)
-	return msg
+func (c *sipClient) parseResponse(txt string) (*message, error) {
+	msg, err := c.parser.parseMessage(txt)
+	return msg, err
 }
 
 /* Authenticator interface */
@@ -82,24 +161,35 @@ func (c *sipClient) NeedsCredentials() bool {
 	return true
 }
 
-func (c *sipClient) CheckCredentials(credentials *creds.Credentials) bool {
+func (c *sipClient) CheckCredentials(credentials *creds.Credentials) (bool, error) {
+
 	currentTime := time.Now()
 	user := credentials.User
 	passwd := credentials.Password
+
 	statusRequest := fmt.Sprintf(
 		statusRequestTemplate,
 		currentTime.Format("20060102"),
 		currentTime.Format("150102"),
 		c.location, user, passwd)
-	telnetSend(c.conn, statusRequest)
 
-	statusMsg := c.parseResponse(telnetRead(c.conn))
+	statusMsg := &message{}
+	resp, err := c.sendRequest(statusRequest)
+	if err != nil {
+		return false, err
+	}
+
+	statusMsg, err = c.parseResponse(resp)
+	if err != nil {
+		log.Println("Error while parsing response")
+		return false, err
+	}
 	if value, ok := c.parser.getFieldValue(statusMsg, validPatron); ok && value == yes {
 		if value, ok := c.parser.getFieldValue(statusMsg, validPatronPassword); ok && value == yes {
-			return true
+			return true, nil
 		}
+		// TODO log whatever error we can find (AF, Screen Message, for instance)
 	}
 
-	// TODO log whatever error we can find (AF, Screen Message, for instance)
-	return false
+	return false, errors.New("unknown error while checking credentials")
 }
diff --git a/pkg/auth/sip2/spec.go b/pkg/auth/sip2/spec.go
index 09561e6..80fbe7a 100644
--- a/pkg/auth/sip2/spec.go
+++ b/pkg/auth/sip2/spec.go
@@ -16,6 +16,7 @@
 package sip2
 
 import (
+	"errors"
 	"log"
 	"strconv"
 	"strings"
@@ -127,11 +128,22 @@ func (p *Parser) getFieldValue(msg *message, field string) (string, bool) {
 	return "", false
 }
 
-func (p *Parser) parseMessage(msg string) *message {
+func (p *Parser) parseMessage(msg string) (*message, error) {
+	/* FIXME */
+	/*
+		http: panic serving 186.26.116.7:1292: runtime error: slice bounds out of range
+	*/
+	if len(msg) == 0 {
+		return &message{}, errors.New("empty message")
+	}
+	if len(msg) < (2 + len(telnetTerminator)) {
+		return &message{}, errors.New("parseMessage: message too short")
+	}
 	txt := msg[:len(msg)-len(telnetTerminator)]
 	code, err := strconv.Atoi(txt[:2])
 	if nil != err {
 		log.Printf("Error parsing integer: %s\n", txt[:2])
+		return &message{}, errors.New("parseMessage: error parsing integer")
 	}
 	spec := p.getMessageSpecByCode(code)
 	txt = txt[2:]
@@ -143,7 +155,7 @@ func (p *Parser) parseMessage(msg string) *message {
 		message.fixedFields = append(message.fixedFields, fixedField{sp, value})
 	}
 	if len(txt) == 0 {
-		return message
+		return message, nil
 	}
 	for _, part := range strings.Split(txt, "|") {
 		if len(part) > 0 {
@@ -152,5 +164,5 @@ func (p *Parser) parseMessage(msg string) *message {
 			message.fields = append(message.fields, variableField{partSpec, value})
 		}
 	}
-	return message
+	return message, nil
 }
diff --git a/pkg/auth/sip2/telnet.go b/pkg/auth/sip2/telnet.go
index 7d8c4fa..469f980 100644
--- a/pkg/auth/sip2/telnet.go
+++ b/pkg/auth/sip2/telnet.go
@@ -16,7 +16,12 @@
 package sip2
 
 import (
-	"github.com/reiver/go-telnet"
+	//"github.com/reiver/go-telnet"
+	/* TODO this one exposes DialTimeout */
+	"github.com/linxiaozhi/go-telnet"
+	"log"
+	"net"
+	"time"
 )
 
 // The terminator can be configured differently for different SIP endpoints.
@@ -24,16 +29,29 @@ import (
 
 var telnetTerminator string
 
-func telnetRead(conn *telnet.Conn) (out string) {
+var telnetTimeout string = "2000ms"
+
+func telnetRead(conn gote.Connection) (string, error) {
 	var buffer [1]byte
 	recvData := buffer[:]
 	var n int
 	var err error
+	var out string
 
 	for {
+		setDeadline(conn)
 		n, err = conn.Read(recvData)
 		if n <= 0 || err != nil {
-			break
+			if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
+				log.Println("telnet: read timeout:", err)
+				return "", err
+			} else {
+				// some other error, do something else, for example create new conn
+				/* TODO -- should modify attributes of the error, or raise a non-recoverable one */
+				/* ie, broken pipe errors will not be solved by retries */
+				log.Println("telnet: read error:", err)
+				return "", err
+			}
 		} else {
 			out += string(recvData)
 		}
@@ -41,11 +59,14 @@ func telnetRead(conn *telnet.Conn) (out string) {
 			break
 		}
 	}
-	return out
+	return out, nil
 }
 
-func telnetSend(conn *telnet.Conn, command string) {
+func telnetSend(conn gote.Connection, command string) error {
 	var commandBuffer []byte
+
+	setDeadline(conn)
+
 	for _, char := range command {
 		commandBuffer = append(commandBuffer, byte(char))
 	}
@@ -53,6 +74,29 @@ func telnetSend(conn *telnet.Conn, command string) {
 	var crlfBuffer [2]byte = [2]byte{'\r', '\n'}
 	crlf := crlfBuffer[:]
 
-	conn.Write(commandBuffer)
-	conn.Write(crlf)
+	_, err := conn.Write(commandBuffer)
+	if err != nil {
+		return err
+	}
+
+	_, err = conn.Write(crlf)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func setDeadline(conn gote.Connection) error {
+	/* TODO do we need to set deadline explicitely here? */
+	t, err := time.ParseDuration(telnetTimeout)
+	if err != nil {
+		log.Println("telnet: error parsing duration")
+		return err
+	}
+	err = conn.SetDeadline(time.Now().Add(t * time.Second))
+	if err != nil {
+		log.Println("telnet: error setting deadline")
+		return err
+	}
+	return nil
 }
diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
index 8b4bdbb..e7db26b 100644
--- a/pkg/metrics/metrics.go
+++ b/pkg/metrics/metrics.go
@@ -6,6 +6,11 @@ import (
 )
 
 var (
+	UnavailableLogins = promauto.NewCounter(prometheus.CounterOpts{
+		Name: "vpnweb_unavailable_service_logins_total",
+		Help: "The total number of failed logins (server error)",
+	})
+
 	FailedLogins = promauto.NewCounter(prometheus.CounterOpts{
 		Name: "vpnweb_failed_logins_total",
 		Help: "The total number of failed logins",
diff --git a/pkg/web/middleware.go b/pkg/web/middleware.go
index fbbdbaf..ed137d6 100644
--- a/pkg/web/middleware.go
+++ b/pkg/web/middleware.go
@@ -31,10 +31,17 @@ import (
 
 const debugAuth string = "VPNWEB_DEBUG_AUTH"
 
-func AuthMiddleware(authenticationFunc func(*creds.Credentials) bool, opts *config.Opts) http.HandlerFunc {
-	debugAuth, exists := os.LookupEnv(debugAuth)
+func isDebugAuthEnabled(s string) bool {
+	if strings.ToLower(s) == "yes" || strings.ToLower(s) == "true" {
+		return true
+	}
+	return false
+}
+
+func AuthMiddleware(authenticationFunc func(*creds.Credentials) (bool, error), opts *config.Opts) http.HandlerFunc {
+	debugFlag, exists := os.LookupEnv(debugAuth)
 	if !exists {
-		debugAuth = "false"
+		debugFlag = "false"
 	}
 	var authHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var c creds.Credentials
@@ -51,18 +58,27 @@ func AuthMiddleware(authenticationFunc func(*creds.Credentials) bool, opts *conf
 			return
 		}
 
-		valid := authenticationFunc(&c)
+		valid, err := authenticationFunc(&c)
 
 		if !valid {
-			metrics.FailedLogins.Inc()
-			log.Println("Wrong auth for user", c.User)
-			http.Error(w, "Wrong user and/or password", http.StatusUnauthorized)
-			return
+			if err != nil {
+				metrics.UnavailableLogins.Inc()
+				log.Println("Error while checking credentials: ", err)
+				http.Error(w, "Auth service unavailable", http.StatusServiceUnavailable)
+				return
+			} else {
+				metrics.FailedLogins.Inc()
+				if isDebugAuthEnabled(debugFlag) {
+					log.Println("Wrong credentials for user", c.User)
+				}
+				http.Error(w, "Wrong user and/or password", http.StatusUnauthorized)
+				return
+			}
 		}
 
 		metrics.SuccessfulLogins.Inc()
 
-		if strings.ToLower(debugAuth) == "yes" {
+		if isDebugAuthEnabled(debugFlag) {
 			log.Println("Valid auth for user", c.User)
 		}
 		token := jwt.New(jwt.SigningMethodHS256)
diff --git a/test/integration/sipcli/main.go b/test/integration/sipcli/main.go
index 1e05938..8393a37 100644
--- a/test/integration/sipcli/main.go
+++ b/test/integration/sipcli/main.go
@@ -12,8 +12,8 @@ import (
 	"strings"
 )
 
-const authURI string = "https://%s:%s/3/auth"
-const certURI string = "https://%s:%s/3/cert"
+const authURI string = "%s:%s/3/auth"
+const certURI string = "%s:%s/3/cert"
 
 func formatCredentials(user, pass string) (string, error) {
 	c := creds.Credentials{User: user, Password: pass}