Browse Source

Add outer base64 encoding for authorizations

- Ensures inner JSON encoding is not exposed
  for accidental transformation/reformatting
  when in transport; eases handling of
  authorizations embedded in other JSON in
  transport.

- Increases length of authorization with short
  access type name from ~300 bytes to ~400
  bytes.
Rod Hynes 8 years ago
parent
commit
b16db52553

+ 21 - 12
psiphon/common/accesscontrol/accesscontrol.go

@@ -30,7 +30,8 @@
 // entities which are distinct from service providers. Only verification
 // keys will be deployed to service providers.
 //
-// An authorization is encoded in JSON:
+// An authorization is represented in JSON, which is then base64-encoded
+// for transport:
 //
 // {
 //   "Authorization" : {
@@ -48,6 +49,7 @@ import (
 	"crypto/rand"
 	"crypto/sha256"
 	"crypto/subtle"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"io"
@@ -151,25 +153,25 @@ type signedAuthorization struct {
 // from the seed without revealing the original value. The authorization
 // ID is to be used to mitigate malicious authorization reuse/sharing.
 //
-// The return value is a serialized JSON representation of the
-// signed authorization that can be passed to VerifyAuthorization.
+// The return value is a base64-encoded, serialized JSON representation
+// of the signed authorization that can be passed to VerifyAuthorization.
 func IssueAuthorization(
 	signingKey *SigningKey,
 	seedAuthorizationID []byte,
-	expires time.Time) ([]byte, error) {
+	expires time.Time) (string, error) {
 
 	if len(signingKey.ID) != keyIDLength ||
 		len(signingKey.AccessType) < 1 ||
 		len(signingKey.AuthorizationIDKey) != authorizationIDKeyLength ||
 		len(signingKey.PrivateKey) != ed25519.PrivateKeySize {
-		return nil, common.ContextError(errors.New("invalid signing key"))
+		return "", common.ContextError(errors.New("invalid signing key"))
 	}
 
 	hkdf := hkdf.New(sha256.New, signingKey.AuthorizationIDKey, nil, seedAuthorizationID)
 	ID := make([]byte, authorizationIDLength)
 	_, err := io.ReadFull(hkdf, ID)
 	if err != nil {
-		return nil, common.ContextError(err)
+		return "", common.ContextError(err)
 	}
 
 	auth := Authorization{
@@ -180,7 +182,7 @@ func IssueAuthorization(
 
 	authJSON, err := json.Marshal(auth)
 	if err != nil {
-		return nil, common.ContextError(err)
+		return "", common.ContextError(err)
 	}
 
 	signature := ed25519.Sign(signingKey.PrivateKey, authJSON)
@@ -193,10 +195,12 @@ func IssueAuthorization(
 
 	signedAuthJSON, err := json.Marshal(signedAuth)
 	if err != nil {
-		return nil, common.ContextError(err)
+		return "", common.ContextError(err)
 	}
 
-	return signedAuthJSON, nil
+	encodedSignedAuth := base64.StdEncoding.EncodeToString(signedAuthJSON)
+
+	return encodedSignedAuth, nil
 }
 
 // VerificationKeyRing is a set of verification keys to be deployed
@@ -228,11 +232,16 @@ func ValidateKeyRing(keyRing *VerificationKeyRing) error {
 // Assumes that ValidateKeyRing has been called.
 func VerifyAuthorization(
 	keyRing *VerificationKeyRing,
-	signedAuthorizationJSON []byte) (*Authorization, error) {
+	encodedSignedAuthorization string) (*Authorization, error) {
 
-	var signedAuth signedAuthorization
+	signedAuthorizationJSON, err := base64.StdEncoding.DecodeString(
+		encodedSignedAuthorization)
+	if err != nil {
+		return nil, common.ContextError(err)
+	}
 
-	err := json.Unmarshal(signedAuthorizationJSON, &signedAuth)
+	var signedAuth signedAuthorization
+	err = json.Unmarshal(signedAuthorizationJSON, &signedAuth)
 	if err != nil {
 		return nil, common.ContextError(err)
 	}

+ 16 - 5
psiphon/common/accesscontrol/accesscontrol_test.go

@@ -20,7 +20,9 @@
 package accesscontrol
 
 import (
+	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 )
@@ -78,6 +80,8 @@ func TestAuthorization(t *testing.T) {
 		t.Fatalf("IssueAuthorization failed: %s", err)
 	}
 
+	fmt.Printf("encoded authorization length: %d\n", len(auth))
+
 	verifiedAuth, err := VerifyAuthorization(keyRing, auth)
 	if err != nil {
 		t.Fatalf("VerifyAuthorization failed: %s", err)
@@ -126,8 +130,13 @@ func TestAuthorization(t *testing.T) {
 		t.Fatalf("IssueAuthorization failed: %s", err)
 	}
 
+	decodedAuth, err := base64.StdEncoding.DecodeString(auth)
+	if err != nil {
+		t.Fatalf("DecodeString failed: %s", err)
+	}
+
 	var hackSignedAuth signedAuthorization
-	err = json.Unmarshal(auth, &hackSignedAuth)
+	err = json.Unmarshal(decodedAuth, &hackSignedAuth)
 	if err != nil {
 		t.Fatalf("Unmarshal failed: %s", err)
 	}
@@ -140,19 +149,21 @@ func TestAuthorization(t *testing.T) {
 
 	hackAuth.AccessType = correctAccess
 
-	auth, err = json.Marshal(hackAuth)
+	marshaledAuth, err := json.Marshal(hackAuth)
 	if err != nil {
 		t.Fatalf("Marshall failed: %s", err)
 	}
 
-	hackSignedAuth.Authorization = auth
+	hackSignedAuth.Authorization = marshaledAuth
 
-	signedAuth, err := json.Marshal(hackSignedAuth)
+	marshaledSignedAuth, err := json.Marshal(hackSignedAuth)
 	if err != nil {
 		t.Fatalf("Marshall failed: %s", err)
 	}
 
-	verifiedAuth, err = VerifyAuthorization(keyRing, signedAuth)
+	encodedSignedAuth := base64.StdEncoding.EncodeToString(marshaledSignedAuth)
+
+	verifiedAuth, err = VerifyAuthorization(keyRing, encodedSignedAuth)
 	// TODO: check error message?
 	if err == nil {
 		t.Fatalf("VerifyAuthorization unexpected success")

+ 1 - 1
psiphon/config.go

@@ -502,7 +502,7 @@ type Config struct {
 
 	// Authorizations is a list of encoded, signed access control authorizations that
 	// the client has obtained and will present to the server.
-	Authorizations []json.RawMessage
+	Authorizations []string
 }
 
 // DownloadURL specifies a URL for downloading resources along with parameters

+ 31 - 99
psiphon/server/api.go

@@ -20,7 +20,6 @@
 package server
 
 import (
-	"bytes"
 	"crypto/subtle"
 	"encoding/json"
 	"errors"
@@ -48,6 +47,8 @@ const (
 
 var CLIENT_VERIFICATION_REQUIRED = false
 
+type requestJSONObject map[string]interface{}
+
 // sshAPIRequestHandler routes Psiphon API requests transported as
 // JSON objects via the SSH request mechanism.
 //
@@ -66,10 +67,20 @@ func sshAPIRequestHandler(
 	name string,
 	requestPayload []byte) ([]byte, error) {
 
-	// Note: for SSH requests, MAX_API_PARAMS_SIZE is implicitly enforced
-	// by max SSH request packet size.
-
-	params, err := requestJSONUnmarshal(requestPayload)
+	// Notes:
+	//
+	// - For SSH requests, MAX_API_PARAMS_SIZE is implicitly enforced
+	//   by max SSH request packet size.
+	//
+	// - The param protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS is an
+	//   array of base64-encoded strings; the base64 representation should
+	//   not be decoded to []byte values. The default behavior of
+	//   https://golang.org/pkg/encoding/json/#Unmarshal for a target of
+	//   type map[string]interface{} will unmarshal a base64-encoded string
+	//   to a string, not a decoded []byte, as required.
+
+	var params requestJSONObject
+	err := json.Unmarshal(requestPayload, &params)
 	if err != nil {
 		return nil, common.ContextError(
 			fmt.Errorf("invalid payload for request name: %s: %s", name, err))
@@ -190,16 +201,12 @@ func handshakeAPIRequestHandler(
 	isMobile := isMobileClientPlatform(clientPlatform)
 	normalizedPlatform := normalizeClientPlatform(clientPlatform)
 
-	var authorizations [][]byte
+	var authorizations []string
 	if params[protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS] != nil {
-		authorizationsRawJSON, err := getRawJSONArrayRequestParam(params, protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS)
+		authorizations, err = getStringArrayRequestParam(params, protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS)
 		if err != nil {
 			return nil, common.ContextError(err)
 		}
-		authorizations = make([][]byte, len(authorizationsRawJSON))
-		for i := 0; i < len(authorizationsRawJSON); i++ {
-			authorizations[i] = authorizationsRawJSON[i]
-		}
 	}
 
 	// Flag the SSH client as having completed its handshake. This
@@ -569,91 +576,6 @@ func clientVerificationAPIRequestHandler(
 	}
 }
 
-type requestJSONObject map[string]interface{}
-
-// requestJSONUnmarshal is equivilent to:
-//
-//   var params requestJSONObject
-//   json.Unmarshal(jsonPayload, &params)
-//
-// ...with the one exception that when the field name is
-// protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS, the value is
-// not fully unmarshaled but instead treated as []json.RawMessage.
-// This leaves the authentications in PSIPHON_API_HANDSHAKE_AUTHORIZATIONS
-// as raw JSON to be unmarshaled in accesscontrol.VerifyAuthorization.
-func requestJSONUnmarshal(jsonPayload []byte) (requestJSONObject, error) {
-
-	expectJSONDelimiter := func(jsonDecoder *json.Decoder, delimiter string) error {
-
-		token, err := jsonDecoder.Token()
-		if err != nil {
-			return err
-		}
-
-		delim, ok := token.(json.Delim)
-		if !ok {
-			return fmt.Errorf("unexpected token type: %T", token)
-		}
-
-		if delim.String() != delimiter {
-			return fmt.Errorf("unexpected delimiter: %s", delim.String())
-		}
-
-		return nil
-	}
-
-	params := make(requestJSONObject)
-
-	jsonDecoder := json.NewDecoder(bytes.NewReader(jsonPayload))
-
-	err := expectJSONDelimiter(jsonDecoder, "{")
-	if err != nil {
-		return nil, common.ContextError(err)
-	}
-
-	for jsonDecoder.More() {
-
-		token, err := jsonDecoder.Token()
-		if err != nil {
-			return nil, common.ContextError(err)
-		}
-
-		name, ok := token.(string)
-		if !ok {
-			return nil, common.ContextError(
-				fmt.Errorf("unexpected token type: %T", token))
-		}
-
-		var value interface{}
-
-		if name == protocol.PSIPHON_API_HANDSHAKE_AUTHORIZATIONS {
-
-			var rawJSONArray []json.RawMessage
-			err = jsonDecoder.Decode(&rawJSONArray)
-			if err != nil {
-				return nil, common.ContextError(err)
-			}
-			value = rawJSONArray
-
-		} else {
-
-			err = jsonDecoder.Decode(&value)
-			if err != nil {
-				return nil, common.ContextError(err)
-			}
-		}
-
-		params[name] = value
-	}
-
-	err = expectJSONDelimiter(jsonDecoder, "}")
-	if err != nil {
-		return nil, common.ContextError(err)
-	}
-
-	return params, nil
-}
-
 type requestParamSpec struct {
 	name      string
 	validator func(*SupportServices, string) bool
@@ -950,15 +872,25 @@ func getMapStringInt64RequestParam(params requestJSONObject, name string) (map[s
 	return result, nil
 }
 
-func getRawJSONArrayRequestParam(params requestJSONObject, name string) ([]json.RawMessage, error) {
+func getStringArrayRequestParam(params requestJSONObject, name string) ([]string, error) {
 	if params[name] == nil {
 		return nil, common.ContextError(fmt.Errorf("missing param: %s", name))
 	}
-	value, ok := params[name].([]json.RawMessage)
+	value, ok := params[name].([]interface{})
 	if !ok {
 		return nil, common.ContextError(fmt.Errorf("invalid param: %s", name))
 	}
-	return value, nil
+
+	result := make([]string, len(value))
+	for i, v := range value {
+		strValue, ok := v.(string)
+		if !ok {
+			return nil, common.ContextError(fmt.Errorf("invalid param: %s", name))
+		}
+		result[i] = strValue
+	}
+
+	return result, nil
 }
 
 // Normalize reported client platform. Android clients, for example, report

+ 1 - 1
psiphon/server/server_test.go

@@ -552,7 +552,7 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 	clientConfig.EmitSLOKs = true
 
 	if !runConfig.omitAuthorization {
-		clientConfig.Authorizations = []json.RawMessage{json.RawMessage(clientAuthorization)}
+		clientConfig.Authorizations = []string{clientAuthorization}
 	}
 
 	if runConfig.doClientVerification {

+ 3 - 3
psiphon/server/tunnelServer.go

@@ -233,7 +233,7 @@ func (server *TunnelServer) ResetAllClientOSLConfigs() {
 func (server *TunnelServer) SetClientHandshakeState(
 	sessionID string,
 	state handshakeState,
-	authorizations [][]byte) ([]string, []string, error) {
+	authorizations []string) ([]string, []string, error) {
 
 	return server.sshServer.setClientHandshakeState(sessionID, state, authorizations)
 }
@@ -683,7 +683,7 @@ func (sshServer *sshServer) resetAllClientOSLConfigs() {
 func (sshServer *sshServer) setClientHandshakeState(
 	sessionID string,
 	state handshakeState,
-	authorizations [][]byte) ([]string, []string, error) {
+	authorizations []string) ([]string, []string, error) {
 
 	sshServer.clientsMutex.Lock()
 	client := sshServer.clients[sessionID]
@@ -1728,7 +1728,7 @@ func (sshClient *sshClient) rejectNewChannel(newChannel ssh.NewChannel, reason s
 // sshClient.stop().
 func (sshClient *sshClient) setHandshakeState(
 	state handshakeState,
-	authorizations [][]byte) ([]string, []string, error) {
+	authorizations []string) ([]string, []string, error) {
 
 	sshClient.Lock()
 	completed := sshClient.handshakeState.completed