kolide / updater Goto Github PK

The repo interface has a series of methods which accept a variadic interface{} and then assert on that type.

This code is a bit hard to read and reduces some benefits we get with a type safe approach.
We could refactor the parameters to take a type repoOption func(*repoconfig)
In #2 I made a similar change to the public interface for creating an Updater.

But I'm not certain that it makes sense to coerce a "local" and "remote" repository into the same function signature. The two repository types have a somewhat different purpose.

Not a high priority, just bringing it up for discussion/ marking it for future work.

For each step in the tuf code add text from the tuf spec in comments.

in tuf/tuf.go

This code returns an empty string every single time because I have a misconfiguration in my notary setup.
If this is not a valid state, the updater should blow up instead.

	var stagedPath string
	if current.Signed.Version > previous.Signed.Version {
		stagedPath, err = rs.stageTarget(current.Signed.Targets)
		if err != nil {
			return nil, "", errors.Wrap(err, "staging targets")
		}
	}
	return current, stagedPath, nil

Notary server optionally supports authentication from clients using JWT tokens. This requires an authorization server that manages access controls, and a cert bundle from this authorization server containing the public key it uses to sign tokens. If token authentication is enabled on Notary server, then any connecting client that does not have a token will be redirected to the authorization server. Please see the docs for Docker Registry v2 authentication for more information.

Right now we have the following structure:

// FileIntegrityMeta hashes and length of a file based resource to help ensure
// the binary footprint of the file hasn't been tampered with
type FileIntegrityMeta struct {
	Hashes map[hashingMethod]string `json:"hashes"`
	Length int                      `json:"length"`
}

The hashes are represented as a dictionary of strings, but it would be more accurate ( and easier to work with ) if we used a byte slice for each checksum.

Relevant from the notary types https://github.com/docker/notary/blob/c04e3e6d05881045def11167c51d4a8baa34899a/tuf/data/types.go#L169-L184

Update documentation so that it is clear the encryption and hashing standards are supported and also describes the updater capabilities.

From Spec.

3.1.2.1 Metadata files for targets delegation

When the targets role delegates trust to other roles, each delegated role
provides one signed metadata file. As is the case with the directory
structure of top-level metadata, the delegated files are relative to the
base URL of metadata available from a given repository mirror.

A delegated role file is located at:

/DELEGATED_ROLE.json

where DELEGATED_ROLE is the name of the delegated role that has been
specified in targets.json. If this role further delegates trust to a role
named ANOTHER_ROLE, that role's signed metadata file is made available at:

/ANOTHER_ROLE.json

I'm working on #13 to add a Download method which downloads a specific target.
In order for the download method to work, it must call refresh() first to update all the local metadata from Notary. The refresh method works by updating pointers on a struct, and is also called by the updater loop which runs on a timer.
This new way of using the code will result in race conditions.

	root, err := rs.refreshRoot()
	if err != nil {
		return "", errors.Wrap(err, "refreshing root")
	}
	// cache the current root
	rs.root = root
	timestamp, err := rs.refreshTimestamp(root)
	if err != nil {
		return "", errors.Wrap(err, "refreshing timestamp")
	}
	rs.timestamp = timestamp

based on slack discussion:

If we merge #20, we're exposing the tuf client to the outside world, which increases the complexity of the implementation and state management somewhat.

We should merge the updater package into tuf, making the client private but allowing the caller to:

a) continuously monitor a specific target for changes
b) download a specific target based on current local repo metadata

This repo makes use of the path package for working with file and directory paths. However the path package is only meant for / separated paths (like in urls). For working with paths on a filesystem, we should use the path/filepath sub-package.

From 3.3.3

any targets metadata filename that was listed in the trusted snapshot metadata file, if any, MUST continue to be listed in the new snapshot metadata file.

We use bytes.Equal to checksums several times, including when we're actually checksumming payloads.
example:

Using bytes.Equal can potentially open up the updater code to a timing attack.

Go provides the subtle package and also the hmac.Equals method for constant time comparisons.

I'm far from an expert but it's probably best to replace all uses of bytes.Equal with hmac.Equals when comparing checksums.

Right now, the only way the updater will get a new file is if an update has been issued. There does not appear for a way to download a file on demand.

We need an equivalent of the go-tuf Download method on the Updater.

See my comment here

the downloadTarget method copies the file to a memory buffer before saving to disk. In case of a large file that's not going to work out well.

The downloadTarget method should take an io.Writer and write to it with a LimitReader