coniks-sys / coniks-go

A CONIKS implementation in Golang
http://coniks.org
Other
116 stars 30 forks source link

SaveSTR function should use `ioutil.writeFile` instead of `utils.WriteFile`. #209

Closed weikengchen closed 5 years ago

weikengchen commented 5 years ago

In the current version of CONIKS, the server's first epoch's signed Merkle tree root will be stored on a file named init.str. A client will load this file as the initial state.

When the server restarts, it is expected that init.str is refreshed with the new Merkle tree root. But, the current version uses utils.writeFile, which causes a problem. Let me now describe in detail.

Detail

On coniks-go/application/config.go,

SaveSTR has a line:

if err := utils.WriteFile(file, strBytes, 0600); err != nil {

where utils.WriteFile only writes file when the file doesn't exist.

This causes a problem. When we restart the server, the new initial STR is not written to init.str because utils.WriteFile skips it.

As a result, the client keeps outputting an error saying the hash chain doesn't match.

To fix this, a simple workaround is to change utils.WriteFile in SaveSTR into ioutil.writeFile.

Question

Does that look good? Should I submit a PR for this?

vqhuy commented 5 years ago

When the server restarts, it is expected that init.str is refreshed with the new Merkle tree root

In the real world, we would like the server's tree root to be consistent even if it restarts. This means there should be only one tree root for each server, and thus the init.str should be written only once. If you want to have a "refresh" server run, you should remove all old materials including private keys, init.str file etc.

weikengchen commented 5 years ago

Got it! That makes sense.

Closed.