Merge DirectoryProof and DirectoryProofs into one
#179
Conversation
protocol/consistencychecks.go
Outdated
| @@ -76,14 +76,10 @@ func (cc *ConsistencyChecks) HandleResponse(requestType int, msg *Response, | |||
| return err.(ErrorCode) | |||
| } | |||
| switch requestType { | |||
| case RegistrationType, KeyLookupType: | |||
| case RegistrationType, KeyLookupType, MonitoringType, KeyLookupInEpochType: | |||
There was a problem hiding this comment.
Nit: Keep order of types in case statement consistent (e.g. L38 in client/encoding.go is different).
| AP: ap, | ||
| STR: str, | ||
| AP: append([]*m.AuthenticationPath{}, ap), | ||
| STR: append([]*DirSTR{}, str), |
There was a problem hiding this comment.
We should update the documentation to specify that the length of AP and STR is expected to be equal to 1. Same goes for NewKeyLookupProof below.
protocol/consistencychecks.go
Outdated
| ap := df.AP | ||
| str := df.STR | ||
| ap := df.AP[0] | ||
| str := df.STR[0] |
There was a problem hiding this comment.
I think we need to validate dfat this point and fail early if len(df.AP) != 1 || len(df.STR) != 1. For now, we should do the same in verifyRegistration although this will probably change when we verify the prior history on a registration.
There was a problem hiding this comment.
It is validated in
coniks-go/protocol/consistencychecks.go
Line 75 in 667a6e0
There was a problem hiding this comment.
message.validate() only checks that the lengths are > 0. I was asking about an additional registration- and lookups-specific validation that checks the lengths are == 1.
There was a problem hiding this comment.
I got it. I think even a malicious server has no reasons to do such an invalid response. I will add a TODO here, since I fixed it in the next pulls.
| @@ -186,6 +175,7 @@ func NewRegistrationProof(ap *m.AuthenticationPath, str *DirSTR, | |||
| // NewKeyLookupProof creates the response message a CONIKS directory | |||
| // sends to a client upon a KeyLookupRequest, | |||
| // and returns a Response containing a DirectoryProof struct. | |||
| // The length of `AP` and `STR` must to be equal to 1. | |||
There was a problem hiding this comment.
This is only the case for registrations and key lookups. Monitoring and LookupInEpochs return more than one auth path and/or STR. And even registrations are going to return multiple auth paths and STRs when we implement prior history verification.
|
Can we merge this now? |
|
Please open new issue if you have any further comments. |
A series of pulls towards #173.