coniks-sys / coniks-go

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

error when trying to register the name-key pair #196

Closed chesnokovilya closed 6 years ago

chesnokovilya commented 6 years ago

I am running server from master branch on local machine(CentOs 7):

[ilya@localhost]~/coniks% ~/go/bin/coniksserver run -p
2017-11-25T10:02:17.072+0700    INFO    Accepting registrations {"address": "tcp://127.0.0.1:3000"}

When I try to register name-key pair I got the following error:


coniks-client> register alice MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCqGKukO1De7z
hZj6+H0qtjTkVxwTCpvKe4eCZ0
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x70b6b2]

goroutine 1 [running]:
github.com/coniks-sys/coniks-go/protocol/auditor.(*AudState).CheckSTRAgainstVerified(0xc4200cec20, 0xc4200f8720, 0x5778d9b2ba91b117, 0x570000c4200fc840)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/protocol/auditor/auditor.go:99 +0x32
github.com/coniks-sys/coniks-go/protocol/auditor.(*AudState).AuditDirectory(0xc4200cec20, 0xc4200f79c8, 0x1, 0x1, 0x7f47658a8000, 0x0)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/protocol/auditor/auditor.go:152 +0x4a
github.com/coniks-sys/coniks-go/protocol/client.(*ConsistencyChecks).updateSTR(0xc4200cec40, 0x0, 0xc4200fc840, 0xc4200150e0, 0x50)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/protocol/client/consistencychecks.go:137 +0xa1
github.com/coniks-sys/coniks-go/protocol/client.(*ConsistencyChecks).HandleResponse(0xc4200cec40, 0x0, 0xc4200fc840, 0xc420070489, 0x5, 0xc4200150e0, 0x4c, 0x50, 0x0, 0x0)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/protocol/client/consistencychecks.go:115 +0x91
github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd.register(0xc4200cec40, 0xc420014320, 0xc420070489, 0x5, 0xc42007048f, 0x4c, 0xc420072d98, 0x3)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd/run.go:142 +0x28d
github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd.run(0x986c40)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd/run.go:98 +0x937
github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd.glob..func2(0x986c40, 0x9a6710, 0x0, 0x0)
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd/run.go:36 +0x2b
github.com/spf13/cobra.(*Command).execute(0x986c40, 0x9a6710, 0x0, 0x0, 0x986c40, 0x9a6710)
    /home/ilya/go/src/github.com/spf13/cobra/command.go:700 +0x2bd
github.com/spf13/cobra.(*Command).ExecuteC(0x986a20, 0xc420070058, 0x0, 0x5)
    /home/ilya/go/src/github.com/spf13/cobra/command.go:781 +0x349
github.com/spf13/cobra.(*Command).Execute(0x986a20, 0x986a20, 0xc42004ff50)
    /home/ilya/go/src/github.com/spf13/cobra/command.go:734 +0x2b
github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd.Execute()
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/coniksclient/cli/internal/cmd/root.go:29 +0x31
main.main()
    /home/ilya/go/src/github.com/coniks-sys/coniks-go/coniksclient/cli/coniksclient.go:10 +0x20```
vqhuy commented 6 years ago

These errors 're probably caused by recent changes of /protocol level, e.g., the client wasn't initialized with a pinned STR.

masomel commented 6 years ago

Yes, this is what this error looks like. I don't think we've implemented STR pinning in the client? This is a feature I implemented as part of #193, but I can move it to its own PR to fix this bug.

vqhuy commented 6 years ago

I don't think we've implemented STR pinning in the client?

Yes, you're right. However, I'd say we also need to rewrite a large part of client's binary when #192 is landed. I'd like to fix this bug after merging #192.

chesnokovilya commented 6 years ago

So what is best way to continue work on #180 ? I need MarshalSTRToFile function.

vqhuy commented 6 years ago

@chesnokovilya You could start by modifying https://github.com/coniks-sys/coniks-go/blob/master/crypto/util.go#L60 and /merkletree package, so the Set function also takes salt as a parameter.

masomel commented 6 years ago

I'd say we also need to rewrite a large part of client's binary when #192 is landed. I'd like to fix this bug after merging #192.

This is true, we're going to rewrite the client binary a fair amount after #192, but I don't think a bugfix for this issue should be blocked by #192 since MarshalSTRToFile() will primarily be used by the server right now to save the initial STR. Until we implement persistent storage for the client, I think we can add this function independently of how clients perform consistency checks since the client will use a pinned initial STR either way, right?

chesnokovilya commented 6 years ago

How to pin initial STR to client? Is there any docs?

vqhuy commented 6 years ago

@chesnokovilya You could print the initial STR of the server and write it to the client's config file and read from it (https://github.com/coniks-sys/coniks-go/blob/master/coniksclient/config.go#L33). The config file is written in TOML format and we currently use github.com/BurntSushi/toml to work with the format. However, I suggest you should write a test (for example, like this one: https://github.com/coniks-sys/coniks-go/blob/master/protocol/auditlog/auditlog_test.go#L21) instead of manually running the binaries.

masomel commented 6 years ago

Resolved in #204