From f94c235b237323603820a73e6b7846204f0e15cb Mon Sep 17 00:00:00 2001 From: Dan Ballard Date: Tue, 28 Aug 2018 10:48:57 -0500 Subject: [PATCH] addressing https://review.openprivacy.ca/r/1/ comments --- app/app.go | 41 ++++++++++++++++++------------- app/cli/main.go | 13 ++++++---- storage/profile_store.go | 46 ++++++++++++++++------------------- storage/profile_store_test.go | 15 +++++++++--- 4 files changed, 65 insertions(+), 50 deletions(-) diff --git a/app/app.go b/app/app.go index 767b03c..11008b9 100644 --- a/app/app.go +++ b/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 - InitDeniableProfiles(password string) error - CreatePeer(name string) peer.CwtchPeer + InitProfiles(password string) (int, error) + InitDeniableProfiles(password string) (int, error) + 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 -func (app *application) InitProfiles(password string) error { +// 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) (int, error) { profiles, err := app.profileStore.InitializeProfileGroup(storage.GroupMaster, password) if err != nil { - return err + return 0, nil } - app.loadProfiles(profiles) - return nil + n := app.loadProfiles(profiles) + return n, nil } -// InitDeniableProfiles will attempt to load the deniable portion of profile storage with the supplied password -func (app *application) InitDeniableProfiles(password string) error { +// 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) (int, error) { profiles, err := app.profileStore.InitializeProfileGroup(storage.GroupDeniable1, password) if err != nil { - return err + return 0, err } - app.loadProfiles(profiles) - return nil + n := app.loadProfiles(profiles) + 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) { diff --git a/app/cli/main.go b/app/cli/main.go index b98a832..a913c8c 100644 --- a/app/cli/main.go +++ b/app/cli/main.go @@ -278,18 +278,21 @@ func main() { quit = true case "new-profile": if len(commands) == 2 { - p := app.CreatePeer(commands[1]) - peer = p + p, err := app.CreatePeer(commands[1]) + 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", len(profiles)) + fmt.Printf("\n%v profiles active now\n", n) fmt.Printf("You should run `select-profile` to use a profile\n") } else { fmt.Printf("\nError loading profiles: %v\n", err) diff --git a/storage/profile_store.go b/storage/profile_store.go index 7dc82f4..f04918f 100644 --- a/storage/profile_store.go +++ b/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 } diff --git a/storage/profile_store_test.go b/storage/profile_store_test.go index ed4d5c7..48fac9d 100644 --- a/storage/profile_store_test.go +++ b/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)