Skip to content
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

Autoupdate osquery #34

Merged
merged 1 commit into from
Aug 4, 2017
Merged

Autoupdate osquery #34

merged 1 commit into from
Aug 4, 2017

Conversation

groob
Copy link
Contributor

@groob groob commented Jun 30, 2017

This PR is mostly complete. Needs some work adding tests and using docker compose instead of some hacky code to run a local test mirror.

groob added a commit that referenced this pull request Jul 6, 2017
The upstream package was refactored and doesn't exist anymore.
The functionality is actually being implemented in #34 so the correct
dependencies will be added there.

Because the updater package is missing, the dependency manager is corrupting the
dependency cache for various pull requests.

Closes #39

// bootstraps local TUF metadata from bindata assets.
func (u *Updater) createLocalTufRepo() error {
if err := os.MkdirAll(u.settings.LocalRepoPath, 0755); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't take the creation of the directory structure for delegate roles into account.

    ├── root.json
    ├── snapshot.json
    ├── targets
    │   ├── bar.json
    │   ├── role
    │   │   └── foo.json
    │   └── role.json
    ├── targets.json
    └── timestamp.json

Copy link
Contributor Author

@groob groob Aug 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to move this to an issue

#66

Makefile Outdated
INSECURE ?= false
PLATFORM ?= darwin
generate:
go run $(shell pwd)/autoupdate/generate_tuf.go \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't this be go run ./autoupdate/generate_tuf.go or even using GOPATH instead of pwd?

@marpaia marpaia changed the title WIP: autoupdate osquery Autoupdate osquery Aug 2, 2017
@marpaia
Copy link
Contributor

marpaia commented Aug 2, 2017

Should we consider merging this before too many merge conflicts arise? It looks like the debug logging changes maybe have caused some merge conflicts in cmd/launcger/launcher.go.

@groob
Copy link
Contributor Author

groob commented Aug 2, 2017

No, I'm in the process of rebasing this locally. Started yesterday.


func (u *Updater) binary() string {
bin := filepath.Base(string(u.destination))
if bin == "osqueryd" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marpaia right now we have the binary named osqueryd but the download url uses osquery for the binary/url path.
Would you rather have this conditional in the code, or use osqueryd in the url on the mirror?

Let's 🚲 🏠

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I follow. I think the binary should be called osqueryd in GCS.

@@ -59,6 +66,11 @@ func parseOptions() (*options, error) {
false,
"print launcher version and exit",
)
flTLS = flag.Bool(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's rename this to something like flInsecureTLS as right now false indicates "use TLS" and that's a bit confusing.

Makefile Outdated


test: generate
go test -cover -v $(shell go list ./... | grep -v /vendor/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-race also?

) (*Updater, error) {
gun, err := d.gun()
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to be consistent with the use of errors.Wrap (as in line 78). I'd prefer using it everywhere, but I'm open to argument.

localRepo := filepath.Base(u.settings.LocalRepoPath)
roles := []string{"root.json", "snapshot.json", "timestamp.json", "targets.json"}
for _, role := range roles {
asset, err := Asset(path.Join("autoupdate/assets", localRepo, role))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split "autoupdate/assets" into two strings so path.Join can use the appropriate separator

func (u *Updater) Run(opts ...tuf.Option) (stop func(), err error) {
updaterOpts := []tuf.Option{
tuf.WithHTTPClient(u.client),
tuf.WithFrequency(10 * time.Second), //TODO leave default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a TODO for this PR or later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question. I might just up this to every 5 minutes an issue for now while we're working out the details.

or i could expose it in a flag..
The default interval is 1 hour, which is ok for a customer's environment, but a hell of a time to wait while testing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you refer to "testing" are you talking about manual testing, or automated tests? If it's manual testing then I think I agree with the idea of putting it behind a flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, manual testing with the launcher running and actually updating the artifact in the remote mirror.

I'll add the flag.

flag.Parse()

gun := fmt.Sprintf("kolide/%s", *flBinary)
localRepo := filepath.Join("./autoupdate/assets/", fmt.Sprintf("%s-tuf", *flBinary))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split "./autoupdate/assets/"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what you mean by split?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, I see

)
flag.Parse()

gun := fmt.Sprintf("kolide/%s", *flBinary)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a path being created? Should it use filepath.Join?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a URL path.

I suppose path.Join can work.


// call this method to restart the launcher when autoupdate completes.
launcherFinalizer := func() error {
if err := syscall.Exec(os.Args[0], os.Args, os.Environ()); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

logFatal(logger, errors.Wrap(err, "launching osquery instance"))
}

// TODO delete this block and use docker compose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this was just a shim so I can test with a local mirror

Copy link
Contributor

@marpaia marpaia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Makefile Outdated
go run ./autoupdate/generate_tuf.go \
-binary=osqueryd -notary=${NOTARY_URL} -insecure=${INSECURE}
# go run ./autoupdate/generate_tuf.go \
# -binary=launcher -notary=${NOTARY_URL} -insecure=${INSECURE}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be uncommented?

updater.bootstrapFn = updater.createLocalTufRepo

for _, opt := range opts {
opt(&updater)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘

launcherUpdaterOpts...,
)
if err != nil {
logFatal(logger, "err", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that enableAutoUpdate should return (func(), err) and func main should be the one doing the logFataling.

Adds the autoupdate package which wraps github.com/kolide/updater/tuf
to provide secure autoupdates from a remote mirror.
If enabled, both osquery and launcher itself are updated when
new versions are released.
@groob groob merged commit ecb0a2d into kolide:master Aug 4, 2017
@groob groob deleted the auto_update_osqueryd branch August 4, 2017 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants