addressing https://review.openprivacy.ca/r/1/ comments

app/app.go

@@ -29,9 +29,9 @@ type application struct {
 
 // Application is a full cwtch peer application. It allows management, usage and storage of multiple peers
 type Application interface {
-	InitProfiles(password string) error
+	InitProfiles(password string) (int, error)
-	InitDeniableProfiles(password string) error
+	InitDeniableProfiles(password string) (int, error)
-	CreatePeer(name string) peer.CwtchPeer
+	CreatePeer(name string) (peer.CwtchPeer, error)
 
 	GetPeer(onion string) peer.CwtchPeer
 	ListPeers() map[string]string
@@ -77,50 +77,57 @@ func (app *application) startTor(torPath string) error {
 	return nil
 }
 
-// InitProfiles will attempt to load the normal portion of profile storage with the supplied password
+// InitProfiles will attempt to load the normal portion of profile storage with the supplied password, returning number of profiles loaded
-func (app *application) InitProfiles(password string) error {
+func (app *application) InitProfiles(password string) (int, error) {
 	profiles, err := app.profileStore.InitializeProfileGroup(storage.GroupMaster, password)
 	if err != nil {
-		return err
+		return 0, nil
 	}
 
-	app.loadProfiles(profiles)
+	n := app.loadProfiles(profiles)
-	return nil
+	return n, nil
 }
 
-// InitDeniableProfiles will attempt to load the deniable portion of profile storage with the supplied password
+// InitDeniableProfiles will attempt to load the deniable portion of profile storage with the supplied password, returning number of profiles loaded
-func (app *application) InitDeniableProfiles(password string) error {
+func (app *application) InitDeniableProfiles(password string) (int, error) {
 	profiles, err := app.profileStore.InitializeProfileGroup(storage.GroupDeniable1, password)
 	if err != nil {
-		return err
+		return 0, err
 	}
 
-	app.loadProfiles(profiles)
+	n := app.loadProfiles(profiles)
-	return nil
+	return n, nil
 }
 
-func (app *application) loadProfiles(profiles []*model.Profile) {
+func (app *application) loadProfiles(profiles []*model.Profile) int {
 	app.mutex.Lock()
+	count := 0
 	for _, profile := range profiles {
 		fmt.Printf("init from profile: %v\n", profile.Name)
 		peer := peer.InitFromProfile(profile)
 		app.peers[peer.GetProfile().Onion] = peer
 		app.startPeer(peer)
+		count++
 	}
 
 	app.mutex.Unlock()
+	return count
 }
 
 // NewProfile creates a new cwtchPeer with a given name.
-func (app *application) CreatePeer(name string) peer.CwtchPeer {
+func (app *application) CreatePeer(name string) (peer.CwtchPeer, error) {
 	app.mutex.Lock()
 	p := peer.NewCwtchPeer(name)
 
-	app.profileStore.AddProfile(storage.GroupMaster, p.GetProfile())
+	err := app.profileStore.AddProfile(storage.GroupMaster, p.GetProfile())
+	if err != nil {
+		log.Printf("Could not add profile to profile store: %v", err)
+		return nil, err
+	}
 	app.peers[p.GetProfile().Onion] = p
 
 	app.startPeer(p)
-	return p
+	return p, nil
 }
 
 func (app *application) startPeer(peer peer.CwtchPeer) {

app/cli/main.go

@@ -278,18 +278,21 @@ func main() {
 			quit = true
 		case "new-profile":
 			if len(commands) == 2 {
-				p := app.CreatePeer(commands[1])
+				p, err := app.CreatePeer(commands[1])
-				peer = p
+				if err != nil {
+					fmt.Printf("Error creating profile: %v\n", err)
+				} else {
+					peer = p
+				}
 			} else {
 				fmt.Printf("Error creating NewProfile, usage: %s\n", usages["new-profile"])
 			}
 		case "load-profiles":
 			fmt.Print("Enter a password to decrypt the profiles: ")
 			bytePassword, err := terminal.ReadPassword(int(syscall.Stdin))
-			err = app.InitProfiles(string(bytePassword))
+			n, err := app.InitProfiles(string(bytePassword))
 			if err == nil {
-				profiles := app.ListPeers()
+				fmt.Printf("\n%v profiles active now\n", n)
-				fmt.Printf("\n%v profiles active now\n", len(profiles))
 				fmt.Printf("You should run `select-profile` to use a profile\n")
 			} else {
 				fmt.Printf("\nError loading profiles: %v\n", err)

storage/profile_store.go

@@ -10,7 +10,6 @@ import (
 	"golang.org/x/crypto/sha3"
 	"io"
 	"log"
-	mrand "math/rand"
 	"os"
 	"sync"
 )
@@ -57,10 +56,6 @@ const (
 	NumProfileBlocks    = 32 // means each small store is about 2.2 MB, medium is 10 MB, and large is 40 MB
 )
 
-const (
-	letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"
-)
-
 // Usable names for profileStore divisions
 const (
 	GroupMaster ProfileGroupID = iota
@@ -93,7 +88,7 @@ type profileStore struct {
 // it should offer no way to prove that those divisions are or are not in use
 type ProfileStore interface {
 	InitializeProfileGroup(groupid ProfileGroupID, password string) ([]*model.Profile, error)
-	AddProfile(groupid ProfileGroupID, profile *model.Profile)
+	AddProfile(groupid ProfileGroupID, profile *model.Profile) error
 }
 
 // NewProfileStore creates a new storage class and file for storing profile data
@@ -119,14 +114,6 @@ func NewProfileStore(filepath string, blocksize int, numBlocks int, numGroups in
 	return ps
 }
 
-func randStringBytes(n int) string {
-	b := make([]byte, n)
-	for i := range b {
-		b[i] = letterBytes[mrand.Intn(len(letterBytes))]
-	}
-	return string(b)
-}
-
 // generateFile creates a storage backing for a profileStore in a file
 // format is [rand*blockSize]*numBlocks
 // which yeilds a file of numBlocks blocks of initially random data of blockSize
@@ -134,23 +121,26 @@ func randStringBytes(n int) string {
 // (the fact the file exists indicates at least 1 profile, but no way beyond that to determine exactly how many)
 func (ps *profileStore) generateFile() error {
 	ps.mutex.Lock()
+	defer ps.mutex.Unlock()
 
 	file, err := os.OpenFile(ps.filename, os.O_CREATE|os.O_WRONLY, 0600)
 	if err != nil {
 		log.Printf("Error opening profile file for writting: %v", err)
-		ps.mutex.Unlock()
 		return err
 	}
 
 	// Generate and write NumProfileBlocks blocks
 	for i := 0; i < ps.numBlocks; i++ {
 		padding := make([]byte, ps.fullBlocksize)
-		rand.Read(padding)
+		_, err := rand.Read(padding)
+		if err != nil {
+			log.Printf("Error: could not read from Rand to fill profile padding: %v", err)
+			return err
+		}
 		file.Write(padding[:])
 	}
 
 	file.Close()
-	ps.mutex.Unlock()
 	return nil
 }
 
@@ -162,6 +152,7 @@ func (ps *profileStore) InitializeProfileGroup(groupid ProfileGroupID, password
 	pg := &ps.profileGroups[groupid]
 
 	if len(pg.profiles) != 0 {
+		ps.mutex.Unlock()
 		return nil, errors.New("ProfileGroup already has loaded profiles, cannot reinitialize")
 	}
 
@@ -207,6 +198,7 @@ func (ps *profileStore) attemptLoad(groupid ProfileGroupID) error {
 		log.Printf("Error opening profile store file: %v", err)
 		return err
 	}
+	defer file.Close()
 
 	file.Seek(int64(ps.fullBlocksize*(int(groupid)*(ps.numBlocks/ps.numGroups))), 0)
 	for i := 0; i < ps.numBlocks/ps.numGroups; i++ {
@@ -214,7 +206,6 @@ func (ps *profileStore) attemptLoad(groupid ProfileGroupID) error {
 		_, err := file.Read(encryptedBytes[:])
 		if err != nil {
 			log.Printf("Error reading from profile store file: %v", err)
-			file.Close()
 			return err
 		}
 		profile := model.AttemptLoadProfile(encryptedBytes[:], ps.profileGroups[groupid].password, ps.getSaveFn(groupid, i))
@@ -225,30 +216,35 @@ func (ps *profileStore) attemptLoad(groupid ProfileGroupID) error {
 		ps.profileGroups[groupid].profiles = append(ps.profileGroups[groupid].profiles, profile)
 	}
 
-	file.Close()
 	return nil
 }
 
 // createKey derives a key and salt for use in encrypting cwtchPeers
-func createKey(password string) ([32]byte, [128]byte) {
+func createKey(password string) ([32]byte, [128]byte, error) {
 	var salt [128]byte
+	var dkr [32]byte
+
 	if _, err := io.ReadFull(rand.Reader, salt[:]); err != nil {
-		panic(err)
+		fmt.Printf("Error: Could not read Rand for salt: %v", err)
+		return dkr, salt, err
 	}
 	dk := pbkdf2.Key([]byte(password), salt[:], 4096, 32, sha3.New512)
 
-	var dkr [32]byte
 	copy(dkr[:], dk)
-	return dkr, salt
+	return dkr, salt, nil
 }
 
 // AddProfile adds a profile to a group, and wires it in to be Save()able: gives it a key, salt and save function
 // then it is saved
-func (ps *profileStore) AddProfile(groupid ProfileGroupID, profile *model.Profile) {
+func (ps *profileStore) AddProfile(groupid ProfileGroupID, profile *model.Profile) error {
 	ps.mutex.Lock()
-	key, salt := createKey(ps.profileGroups[groupid].password)
+	key, salt, err := createKey(ps.profileGroups[groupid].password)
+	if err != nil {
+		return err
+	}
 	profile.OnProfileStoreAdd(key, salt, ps.getSaveFn(groupid, len(ps.profileGroups[groupid].profiles)))
 	ps.profileGroups[groupid].profiles = append(ps.profileGroups[groupid].profiles, profile)
 	ps.mutex.Unlock()
 	profile.Save()
+	return nil
 }

storage/profile_store_test.go

@@ -49,7 +49,10 @@ func TestProfileStore(t *testing.T) {
 		t.Error("Expected error on trying to save profile before added to ProfileStore")
 	}
 
-	ps.AddProfile(GroupMaster, alice)
+	err = ps.AddProfile(GroupMaster, alice)
+	if err != nil {
+		t.Errorf("Error adding profile to profile store: %v", err)
+	}
 
 	// Redundtant, but testing path
 	err = alice.Save()
@@ -60,11 +63,17 @@ func TestProfileStore(t *testing.T) {
 	statFileTest(t)
 
 	carol := model.GenerateNewProfile("Carol")
-	ps.AddProfile(GroupMaster, carol)
+	err = ps.AddProfile(GroupMaster, carol)
+	if err != nil {
+		t.Errorf("Error adding profile to profile store: %v", err)
+	}
 
 	ps.InitializeProfileGroup(GroupDeniable1, DeniablePassword)
 	secretBob := model.GenerateNewProfile("Secret Bob")
-	ps.AddProfile(GroupDeniable1, secretBob)
+	err = ps.AddProfile(GroupDeniable1, secretBob)
+	if err != nil {
+		t.Errorf("Error adding profile to profile store: %v", err)
+	}
 
 	// Confirm loading works
 	psL := NewProfileStore(ProfilesFile, DataBlockSizeSmall, 6, 2)