constant-time auth verification

secrethandshake/conn_test.go

@@ -3,11 +3,15 @@
 package secrethandshake
 
 import (
+	"bytes"
 	"io"
 	"log"
+	"math/rand"
 	"os"
 	"reflect"
 	"testing"
+
+	"github.com/pkg/errors"
 )
 
 // StupidRandom always reads itself. Goal is determinism.
@@ -88,3 +92,113 @@ func TestAuth(t *testing.T) {
 		t.Error("secrets not equal")
 	}
 }
+
+func TestAuthWrong(t *testing.T) {
+	log.SetOutput(os.Stdout)
+
+	keySrv, err := GenEdKeyPair(StupidRandom(0))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	keyClient, err := GenEdKeyPair(StupidRandom(1))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	appKey := make([]byte, 32)
+	io.ReadFull(StupidRandom(255), appKey)
+
+	rServer, wClient := io.Pipe()
+	rClient, wServer := io.Pipe()
+
+	rwServer := rw{rServer, wServer}
+	rwClient := rw{rClient, wClient}
+
+	serverState, err := NewServerState(appKey, *keySrv)
+	if err != nil {
+		t.Error("error making server state:", err)
+	}
+
+	clientState, err := NewClientState(appKey, *keyClient, keySrv.Public)
+	if err != nil {
+		t.Error("error making client state:", err)
+	}
+
+	// buffered channel
+	ch := make(chan error, 2)
+
+	go func() {
+		err := Server(serverState, rwServer)
+		expErr := ErrProtocol{1}
+		if err != expErr {
+			ch <- errors.Wrap(err, "server failed differntly then expected")
+		} else {
+			ch <- nil
+		}
+		wServer.Close()
+	}()
+
+	go func() {
+		err := wrongClient(clientState, rwClient)
+		ch <- errors.Wrap(err, "client failed")
+		wClient.Close()
+	}()
+
+	// t.Error may only be called from this goroutine :/
+	if err = <-ch; err != nil {
+		t.Error(err)
+	}
+	if err = <-ch; err != nil {
+		t.Error(err)
+	}
+
+	if !reflect.DeepEqual(clientState.secret, serverState.secret) {
+		t.Error("secrets not equal")
+	}
+}
+
+// wrong client is a version of Client() that messes up the client auth and expects the connection to be shut down afterwards
+func wrongClient(state *State, conn io.ReadWriter) (err error) {
+	_, err = io.Copy(conn, bytes.NewReader(state.createChallenge()))
+	if err != nil {
+		return errors.Wrapf(err, "secrethandshake: sending challenge failed.")
+	}
+
+	// recv challenge
+	chalResp := make([]byte, ChallengeLength)
+	_, err = io.ReadFull(conn, chalResp)
+	if err != nil {
+		return errors.Wrapf(err, "secrethandshake: receiving challenge failed.")
+	}
+
+	// verify challenge
+	if !state.verifyChallenge(chalResp) {
+		return errors.Errorf("secrethandshake: challenge didn't verify")
+	}
+
+	// prepare authentication vector
+	cauth := state.createClientAuth()
+
+	// flip two bytes
+	n := len(cauth) - 1
+	i := rand.Intn(n)
+	if i == n {
+		i = n - 1
+	}
+	cauth[i], cauth[n] = cauth[n], cauth[i]
+
+	_, err = io.Copy(conn, bytes.NewReader(cauth))
+	if err != nil {
+		return errors.Wrapf(err, "secrethandshake: sending client auth failed.")
+	}
+
+	// recv authentication vector? shouldn't get it
+	boxedSig := make([]byte, ServerAuthLength)
+	n, err = io.ReadFull(conn, boxedSig)
+	if err != io.EOF || n != 0 {
+		return errors.Errorf("wrongClient: expected unepexcted EOF, got %s %d", err, n)
+	}
+
+	return nil
+}

secrethandshake/state.go

@@ -11,11 +11,11 @@ package secrethandshake
 
 import (
 	"bytes"
-
 	"crypto/hmac"
 	"crypto/rand"
 	"crypto/sha256"
 	"crypto/sha512"
+	"crypto/subtle"
 
 	"go.cryptoscope.co/secretstream/internal/lo25519"
 
@@ -26,6 +26,13 @@ import (
 	"golang.org/x/crypto/nacl/box"
 )
 
+func init() {
+	err := testMemoryLayoutAssumption()
+	if err != nil {
+		panic(err)
+	}
+}
+
 // State is the state each peer holds during the handshake
 type State struct {
 	appKey, secHash           []byte
@@ -165,10 +172,13 @@ func (s *State) createClientAuth() []byte {
 	return out
 }
 
-var nullHello [ed25519.SignatureSize + ed25519.PublicKeySize]byte
-
 // verifyClientAuth returns whether a buffer contains a valid clientAuth message
 func (s *State) verifyClientAuth(data []byte) bool {
+	// this branch is okay because there are no secrets involved
+	if len(data) != ed25519.SignatureSize+ed25519.PublicKeySize+box.Overhead {
+		return false
+	}
+
 	var cvSec, aBob [32]byte
 	extra25519.PrivateKeyToCurve25519(&cvSec, &s.local.Secret)
 	curve25519.ScalarMult(&aBob, &cvSec, &s.remoteExchange.Public)
@@ -180,35 +190,22 @@ func (s *State) verifyClientAuth(data []byte) bool {
 	secHasher.Write(s.aBob[:])
 	copy(s.secret2[:], secHasher.Sum(nil))
 
-	s.hello = make([]byte, 0, len(data)-16)
+	s.hello = make([]byte, len(data)-16)
 
-	var nonce [24]byte // always 0?
-	var openOk bool
-	s.hello, openOk = box.OpenAfterPrecomputation(s.hello, data, &nonce, &s.secret2)
+	var (
+		nonce  [24]byte // always 0?
+		sig    [ed25519.SignatureSize]byte
+		public [ed25519.PublicKeySize]byte
+		openOk bool
+	)
 
-	var sig [ed25519.SignatureSize]byte
-	var public [ed25519.PublicKeySize]byte
-	/* TODO: is this const time!?!
-
-	   this is definetly not:
-	   if !openOK {
-	   	s.hello = nullHello
-	   }
-	   copy(sig, ...)
-	   copy(pub, ...)
-	*/
-	if openOk {
-		copy(sig[:], s.hello[:ed25519.SignatureSize])
-		copy(public[:], s.hello[ed25519.SignatureSize:])
-
-	} else {
-		copy(sig[:], nullHello[:ed25519.SignatureSize])
-		copy(public[:], nullHello[ed25519.SignatureSize:])
-	}
+	_, openOk = box.OpenAfterPrecomputation(s.hello[:0], data, &nonce, &s.secret2)
 
-	if lo25519.IsEdLowOrder(sig[:32]) {
-		openOk = false
-	}
+	// see unsafecast_test.go
+	okInt := castBoolToInt(&openOk)
+
+	subtle.ConstantTimeCopy(okInt, sig[:], s.hello[:ed25519.SignatureSize])
+	subtle.ConstantTimeCopy(okInt, public[:], s.hello[ed25519.SignatureSize:ed25519.SignatureSize+ed25519.PublicKeySize])
 
 	var sigMsg bytes.Buffer
 	sigMsg.Write(s.appKey)
@@ -217,7 +214,9 @@ func (s *State) verifyClientAuth(data []byte) bool {
 	verifyOk := ed25519.Verify(&public, sigMsg.Bytes(), &sig)
 
 	copy(s.remotePublic[:], public[:])
-	return openOk && verifyOk
+
+	keyOk := !lo25519.IsEdLowOrder(sig[:32])
+	return openOk && keyOk && verifyOk
 }
 
 // createServerAccept returns a buffer containing a serverAccept message

secrethandshake/unsafecast.go

@@ -0,0 +1,31 @@
+package secrethandshake
+
+import (
+	"errors"
+	"unsafe" // ☢ ☣
+)
+
+// the idea here is that the memory backed by bool
+// will be on byte and no other values than 0x00 and 0x01
+func castBoolToInt(bptr *bool) int {
+	uptr := unsafe.Pointer(bptr)
+	bytePtr := (*byte)(uptr)
+
+	return int(*bytePtr)
+}
+
+func testMemoryLayoutAssumption() error {
+	var boolTrue, boolFalse bool
+	boolTrue = true
+
+	okTrue := castBoolToInt(&boolTrue)
+	if okTrue != 1 {
+		return errors.New("expected bool to int cast failed on true")
+	}
+
+	okFalse := castBoolToInt(&boolFalse)
+	if okFalse != 0 {
+		return errors.New("expected bool to int cast failed on false")
+	}
+	return nil
+}

secrethandshake/unsafecast_test.go

@@ -0,0 +1,11 @@
+package secrethandshake
+
+import (
+	"testing"
+)
+
+func TestUnsafeBoolCase(t *testing.T) {
+	if err := testMemoryLayoutAssumption(); err != nil {
+		t.Fatal(err)
+	}
+}