Bug fixes

* nil reference due to using uninitialized tunnel.serverContext
* panic due to using unsupported regex syntax
* used wrong SSH request name values

psiphon/server/api.go

@@ -61,17 +61,17 @@ func sshAPIRequestHandler(
 	}
 
 	switch name {
-	case "handshake":
+	case psiphon.SERVER_API_HANDSHAKE_REQUEST_NAME:
 		return handshakeAPIRequestHandler(config, geoIPData, params)
-	case "connected":
+	case psiphon.SERVER_API_CONNECTED_REQUEST_NAME:
 		return connectedAPIRequestHandler(config, geoIPData, params)
-	case "status":
+	case psiphon.SERVER_API_STATUS_REQUEST_NAME:
 		return statusAPIRequestHandler(config, geoIPData, params)
-	case "client_verification":
+	case psiphon.SERVER_API_CLIENT_VERIFICATION_REQUEST_NAME:
 		return clientVerificationAPIRequestHandler(config, geoIPData, params)
 	}
 
-	return nil, psiphon.ContextError(errors.New("invalid request name"))
+	return nil, psiphon.ContextError(fmt.Errorf("invalid request name: %s", name))
 }
 
 // handshakeAPIRequestHandler implements the "handshake" API request.
@@ -528,7 +528,7 @@ func isIPAddress(_ *Config, value string) bool {
 	return net.ParseIP(value) != nil
 }
 
-var isDomainRegex = regexp.MustCompile("(?!-)[a-zA-Z\\d-]{1,63}(?<!-)$")
+var isDomainRegex = regexp.MustCompile("[a-zA-Z\\d-]{1,63}$")
 
 func isDomain(_ *Config, value string) bool {
 
@@ -545,6 +545,11 @@ func isDomain(_ *Config, value string) bool {
 	}
 	value = strings.TrimSuffix(value, ".")
 	for _, part := range strings.Split(value, ".") {
+		// Note: regexp doesn't support the following Perl expression which
+		// would check for '-' prefix/suffix: "(?!-)[a-zA-Z\\d-]{1,63}(?<!-)$"
+		if strings.HasPrefix(part, "-") || strings.HasSuffix(part, "-") {
+			return false
+		}
 		if !isDomainRegex.Match([]byte(part)) {
 			return false
 		}

psiphon/serverApi.go

@@ -126,7 +126,7 @@ func NewServerContext(tunnel *Tunnel, sessionId string) (*ServerContext, error)
 // stored -- and sponsor info (home pages, stat regexes).
 func (serverContext *ServerContext) doHandshakeRequest() error {
 
-	params := getBaseParams(serverContext.tunnel)
+	params := serverContext.getBaseParams()
 
 	// *TODO*: this is obsolete?
 	/*
@@ -270,7 +270,7 @@ func (serverContext *ServerContext) doHandshakeRequest() error {
 // a unique user for a time period.
 func (serverContext *ServerContext) DoConnectedRequest() error {
 
-	params := getBaseParams(serverContext.tunnel)
+	params := serverContext.getBaseParams()
 
 	const DATA_STORE_LAST_CONNECTED_KEY = "lastConnected"
 	lastConnected, err := GetKeyValue(DATA_STORE_LAST_CONNECTED_KEY)
@@ -332,7 +332,7 @@ func (serverContext *ServerContext) StatsRegexps() *transferstats.Regexps {
 // DoStatusRequest makes a "status" API request to the server, sending session stats.
 func (serverContext *ServerContext) DoStatusRequest(tunnel *Tunnel) error {
 
-	params := getStatusParams(serverContext.tunnel, true)
+	params := serverContext.getStatusParams(true)
 
 	// Note: ensure putBackStatusRequestPayload is called, to replace
 	// payload for future attempt, in all failure cases.
@@ -379,9 +379,9 @@ func (serverContext *ServerContext) DoStatusRequest(tunnel *Tunnel) error {
 	return nil
 }
 
-func getStatusParams(tunnel *Tunnel, isTunneled bool) requestJSONObject {
+func (serverContext *ServerContext) getStatusParams(isTunneled bool) requestJSONObject {
 
-	params := getBaseParams(tunnel)
+	params := serverContext.getBaseParams()
 
 	// Add a random amount of padding to help prevent stats updates from being
 	// a predictable size (which often happens when the connection is quiet).
@@ -488,23 +488,25 @@ func confirmStatusRequestPayload(payloadInfo *statusRequestPayloadInfo) {
 // The tunnel is assumed to be closed, but its config, protocol, and
 // context values must still be valid.
 // TryUntunneledStatusRequest emits notices detailing failed attempts.
-func TryUntunneledStatusRequest(tunnel *Tunnel, isShutdown bool) error {
+func (serverContext *ServerContext) TryUntunneledStatusRequest(isShutdown bool) error {
 
-	for _, port := range tunnel.serverEntry.GetUntunneledWebRequestPorts() {
-		err := doUntunneledStatusRequest(tunnel, port, isShutdown)
+	for _, port := range serverContext.tunnel.serverEntry.GetUntunneledWebRequestPorts() {
+		err := serverContext.doUntunneledStatusRequest(port, isShutdown)
 		if err == nil {
 			return nil
 		}
 		NoticeAlert("doUntunneledStatusRequest failed for %s:%s: %s",
-			tunnel.serverEntry.IpAddress, port, err)
+			serverContext.tunnel.serverEntry.IpAddress, port, err)
 	}
 
 	return errors.New("all attempts failed")
 }
 
 // doUntunneledStatusRequest attempts an untunneled status request.
-func doUntunneledStatusRequest(
-	tunnel *Tunnel, port string, isShutdown bool) error {
+func (serverContext *ServerContext) doUntunneledStatusRequest(
+	port string, isShutdown bool) error {
+
+	tunnel := serverContext.tunnel
 
 	certificate, err := DecodeCertificate(tunnel.serverEntry.WebServerCertificate)
 	if err != nil {
@@ -525,7 +527,7 @@ func doUntunneledStatusRequest(
 		*dialConfig = *tunnel.untunneledDialConfig
 	}
 
-	url := makeRequestUrl(tunnel, port, "status", getStatusParams(tunnel, false))
+	url := makeRequestUrl(tunnel, port, "status", serverContext.getStatusParams(false))
 
 	httpClient, url, err := MakeUntunneledHttpsClient(
 		dialConfig,
@@ -649,7 +651,7 @@ func RecordTunnelStats(
 func (serverContext *ServerContext) DoClientVerificationRequest(
 	verificationPayload string) error {
 
-	params := getBaseParams(serverContext.tunnel)
+	params := serverContext.getBaseParams()
 
 	if serverContext.psiphonHttpsClient == nil {
 
@@ -724,12 +726,14 @@ type requestJSONObject map[string]interface{}
 // getBaseParams returns all the common API parameters that are included
 // with each Psiphon API request. These common parameters are used for
 // statistics.
-func getBaseParams(tunnel *Tunnel) requestJSONObject {
+func (serverContext *ServerContext) getBaseParams() requestJSONObject {
 
 	params := make(requestJSONObject)
 
-	params["session_id"] = tunnel.serverContext.sessionId
-	params["client_session_id"] = tunnel.serverContext.sessionId
+	tunnel := serverContext.tunnel
+
+	params["session_id"] = serverContext.sessionId
+	params["client_session_id"] = serverContext.sessionId
 	params["server_secret"] = tunnel.serverEntry.WebServerSecret
 	params["propagation_channel_id"] = tunnel.config.PropagationChannelId
 	params["sponsor_id"] = tunnel.config.SponsorId

psiphon/tunnel.go

@@ -1043,7 +1043,7 @@ func sendUntunneledStats(tunnel *Tunnel, isShutdown bool) {
 		return
 	}
 
-	err := TryUntunneledStatusRequest(tunnel, isShutdown)
+	err := tunnel.serverContext.TryUntunneledStatusRequest(isShutdown)
 	if err != nil {
 		NoticeAlert("TryUntunneledStatusRequest failed for %s: %s", tunnel.serverEntry.IpAddress, err)
 	}