-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Client-auditor protocol #155
Changes from 20 commits
5a923db
d9ed76d
146e91a
90e2517
e8178c1
badbafc
3bd8653
c2d107f
13d3eb2
0d0d312
8b1eeeb
3872e32
1f609a8
477cddf
c6d106c
9843f4a
9f212b1
fcebf89
7e54087
699858a
92cee27
92c6a98
073aa70
e3cb343
e886828
3e340b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,214 @@ | ||
// This module implements a CONIKS audit log that a CONIKS auditor | ||
// maintains. | ||
// An audit log is a mirror of many CONIKS key directories' STR history, | ||
// allowing CONIKS clients to audit the CONIKS directories. | ||
|
||
package protocol | ||
|
||
import ( | ||
"encoding/hex" | ||
|
||
"github.com/coniks-sys/coniks-go/crypto" | ||
"github.com/coniks-sys/coniks-go/crypto/sign" | ||
) | ||
|
||
type directoryHistory struct { | ||
dirName string | ||
signKey sign.PublicKey | ||
snapshots map[uint64]*DirSTR | ||
latestSTR *DirSTR | ||
} | ||
|
||
// A ConiksAuditLog maintains the histories | ||
// of all CONIKS directories known to a CONIKS auditor, | ||
// indexing the histories by the hash of a directory's initial | ||
// STR (specifically, the hash of the STR's signature as a string). | ||
// Each history includes the directory's domain name as a string, its | ||
// public signing key enabling the auditor to verify the corresponding | ||
// signed tree roots, and a map with the snapshots for each observed | ||
// epoch. | ||
type ConiksAuditLog map[string]*directoryHistory | ||
|
||
// computeInitSTRHash is a wrapper for the digest function; | ||
// returns empty string if the STR isn't an initial STR (i.e. str.Epoch != 0) | ||
func computeInitSTRHash(initSTR *DirSTR) string { | ||
if initSTR.Epoch != 0 { | ||
return "" | ||
} | ||
return hex.EncodeToString(crypto.Digest(initSTR.Signature)) | ||
} | ||
|
||
// updateLatestSTR inserts a new STR into a directory history; | ||
// assumes the STR has been validated by the caller | ||
func (h *directoryHistory) updateLatestSTR(newLatest *DirSTR) { | ||
h.snapshots[newLatest.Epoch] = newLatest | ||
h.latestSTR = newLatest | ||
} | ||
|
||
// caller validates that initSTR is for epoch 0 | ||
func newDirectoryHistory(dirName string, signKey sign.PublicKey, initSTR *DirSTR) *directoryHistory { | ||
h := new(directoryHistory) | ||
h.dirName = dirName | ||
h.signKey = signKey | ||
h.snapshots = make(map[uint64]*DirSTR) | ||
h.updateLatestSTR(initSTR) | ||
return h | ||
} | ||
|
||
// NewAuditLog constructs a new ConiksAuditLog. It creates an empty | ||
// log; the auditor will add an entry for each CONIKS directory | ||
// the first time it observes an STR for that directory. | ||
func NewAuditLog() ConiksAuditLog { | ||
l := make(map[string]*directoryHistory) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return l | ||
} | ||
|
||
// IsKnownDirectory checks to see if an entry for the directory | ||
// (indexed by the hash of its initial STR dirInitHash) exists | ||
// in the audit log l. IsKnownDirectory() does not | ||
// validate the entries themselves. It returns true if an entry exists, | ||
// and false otherwise. | ||
func (l ConiksAuditLog) IsKnownDirectory(dirInitHash string) bool { | ||
h := l[dirInitHash] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still getting used to some Go idioms... |
||
if h != nil { | ||
return true | ||
} | ||
return false | ||
} | ||
|
||
// Insert creates a new directory history for the key directory addr, | ||
// verifies the consistency of the STR history so far, and inserts it | ||
// into the audit log l if the checks pass. | ||
// The directory history is initialized with the key directory's | ||
// signing key signKey, and a list of STRs representing the | ||
// directory's STR history so far (as a map of epochs to STRs). | ||
// Insert() returns an ErrAuditLog if the auditor attempts to create | ||
// a new history for a known directory, an ErrMalformedDirectoryMessage | ||
// if oldSTRs is malformed, a CheckBadSignature or CheckBadSTR if there | ||
// is an inconsistency in the history given in hist, and nil otherwise. | ||
// Insert() only creates the initial entry in the log for addr. Use Update() | ||
// to insert newly observed STRs for addr in subsequent epochs. | ||
// FIXME: pass Response message as param | ||
// masomel: will probably want to write a more generic function | ||
// for "catching up" on a history in case an auditor misses epochs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before we merge this, the comments should follow golang's documentation convention (start with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, thanks for pointing this out! |
||
func (l ConiksAuditLog) Insert(addr string, signKey sign.PublicKey, | ||
hist map[uint64]*DirSTR) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
// make sure we're getting an initial STR at the very least | ||
if len(hist) < 1 && hist[0].Epoch != 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! Can we change it to a slice? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A slice would also make sense. Then we would have to check that the epoch for the STR in each element corresponds to the index, which seems fine to me. |
||
return ErrMalformedDirectoryMessage | ||
} | ||
|
||
// compute the hash of the initial STR | ||
dirInitHash := computeInitSTRHash(hist[0]) | ||
|
||
// error if we want to create a new entry for a directory | ||
// we already know | ||
if l.IsKnownDirectory(dirInitHash) { | ||
return ErrAuditLog | ||
} | ||
|
||
// create the new directory history | ||
h := newDirectoryHistory(addr, signKey, hist[0]) | ||
|
||
// add each STR into the history | ||
// start at 1 since we've inserted the initial STR above | ||
startEp := uint64(1) | ||
endEp := uint64(len(hist)) | ||
|
||
// This loop automatically catches if hist is malformed | ||
// (i.e. hist is missing an epoch between 0 and the latest given) | ||
for ep := startEp; ep < endEp; ep++ { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There sort of already is one on L107. But that FIXME is a bit buried at the bottom of the function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thought here is we will verify the consistency of the hash chain, so it maybe better if the index starts from
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the initial STR should be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, what I was suggesting is we should insert the initial STR at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got it. It makes total sense! |
||
str := hist[ep] | ||
if str == nil { | ||
return ErrMalformedDirectoryMessage | ||
} | ||
|
||
// verify the consistency of each new STR before inserting | ||
// into the audit log | ||
err := verifySTRConsistency(signKey, h.snapshots[ep-1], str) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the comment below, but this would then become |
||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
h.snapshots[ep] = str | ||
} | ||
|
||
// Make sure to update the latestSTR | ||
// in this particular call, the latestSTR has already been | ||
// inserted into the snapshots map in the loop above | ||
h.updateLatestSTR(hist[endEp-1]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be cleared if you moved this to the last step of the loop, so that rather than
you have,
then you won't need the clarifying comment, and you won't be doing it redundantly when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was considering making this change just today when I was working on #170. I totally agree with you that this step would be clearer if we just call |
||
l[dirInitHash] = h | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe don't expose the internals of |
||
|
||
return nil | ||
} | ||
|
||
// Update verifies the consistency of a newly observed STR newSTR for | ||
// the directory addr, and inserts the newSTR into addr's directory history | ||
// if the checks (i.e. STR signature and hash chain verifications) pass. | ||
// Update() returns nil if the checks pass, and the appropriate consistency | ||
// check error otherwise. Update() assumes that Insert() has been called for | ||
// addr prior to its first call and thereby expects that an entry for addr | ||
// exists in the audit log l. | ||
// FIXME: pass Response message as param | ||
func (l ConiksAuditLog) Update(dirInitHash string, newSTR *DirSTR) error { | ||
|
||
// error if we want to update the entry for an addr we don't know | ||
if !l.IsKnownDirectory(dirInitHash) { | ||
return ErrAuditLog | ||
} | ||
|
||
h := l[dirInitHash] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing here, |
||
|
||
if err := verifySTRConsistency(h.signKey, h.latestSTR, newSTR); err != nil { | ||
return err | ||
} | ||
|
||
// update the latest STR | ||
h.updateLatestSTR(newSTR) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the refactor suggested above, these two operations ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of what I'm doing for #170, I need to see if it makes sense to refactor these operations both on the client and auditor. I'll probably hold off on that for now. |
||
return nil | ||
} | ||
|
||
// GetObservedSTRs gets a range of observed STRs for the CONIKS directory | ||
// address indicated in the AuditingRequest req received from a | ||
// CONIKS client, and returns a tuple of the form (response, error). | ||
// The response (which also includes the error code) is sent back to | ||
// the client. The returned error is used by the auditor | ||
// for logging purposes. | ||
// | ||
// A request without a directory address, with a StartEpoch or EndEpoch | ||
// greater than the latest observed epoch of this directory, or with | ||
// at StartEpoch > EndEpoch is considered | ||
// malformed and causes GetObservedSTRs() to return a | ||
// message.NewErrorResponse(ErrMalformedClientMessage) tuple. | ||
// GetObservedSTRs() returns a message.NewSTRHistoryRange(strs) tuple. | ||
// strs is a list of STRs for the epoch range [StartEpoch, EndEpoch]; | ||
// if StartEpoch == EndEpoch, the list returned is of length 1. | ||
// If the auditor doesn't have any history entries for the requested CONIKS | ||
// directory, GetObservedSTRs() returns a | ||
// message.NewErrorResponse(ReqUnknownDirectory) tuple. | ||
func (l ConiksAuditLog) GetObservedSTRs(req *AuditingRequest) (*Response, | ||
ErrorCode) { | ||
|
||
// make sure we have a history for the requested directory in the log | ||
if !l.IsKnownDirectory(req.DirInitSTRHash) { | ||
return NewErrorResponse(ReqUnknownDirectory), ReqUnknownDirectory | ||
} | ||
|
||
h := l[req.DirInitSTRHash] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or, maybe we should embrace that it's a map and always just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
// make sure the request is well-formed | ||
if req.EndEpoch > h.latestSTR.Epoch || req.StartEpoch > req.EndEpoch { | ||
return NewErrorResponse(ErrMalformedClientMessage), | ||
ErrMalformedClientMessage | ||
} | ||
|
||
var strs []*DirSTR | ||
for ep := req.StartEpoch; ep <= req.EndEpoch; ep++ { | ||
str := h.snapshots[ep] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is problematic if latestSTR isn't in the snapshots. See the discussion above. |
||
strs = append(strs, str) | ||
} | ||
|
||
return NewSTRHistoryRange(strs) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should either remove this check or panic here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this function in 92cee27 (it would be used by the clients, too).
Feel free to revert if you don't like it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactoring LGTM.