Merge pull request #774 from coreos/signpocalypse

*: remove unit signing

Author: bcwaldon

Documentation/configuration.md

@@ -62,18 +62,6 @@ An Agent will be considered dead if it exceeds this amount of time to communicat
 
 Default: "30s"
 
-#### verify_units
-
-Enable payload signature verification. Payloads without verifiable signatures will not be eligible to run on the local fleet server.
-
-Default: false
-
-#### authorized_keys_file
-
-File containing public SSH keys that should be used to verify payload signatures.
-
-Default: "~/.ssh/authorized_keys"
-
 #### engine_reconcile_interval
 
 Interval at which the engine should reconcile the cluster schedule in etcd.

Documentation/security.md

@@ -4,10 +4,6 @@
 
 The default deployment of the preview release of fleet doesn't currently perform any authentication or authorization for submitted units. This means that any client that can access your etcd cluster can potentially run arbitrary code on many of your machines very easily.
 
-## Job Signing
-
-Version 0.2.0 of fleet added the ability to add signatures to Jobs to provide authorization and integrity checking to units submitted to the cluster. For more details on how it works and how to use it, see the [Signed Units](signed-units.md) documentation.
-
 ## Securing the Registry
 
 You should avoid public access to the registry (i.e etcd) and instead run fleet [from your local laptop](using-the-client.md#get-up-and-running) with the `--tunnel` flag to run commands over an SSH tunnel. You can alias this flag for easier usage: `alias fleetctl=fleetctl --tunnel 10.10.10.10` - or use the environment variable `FLEETCTL_TUNNEL`.

Documentation/signed-units.md

@@ -10,7 +10,6 @@ import (
 	"github.com/coreos/fleet/machine"
 	"github.com/coreos/fleet/pkg"
 	"github.com/coreos/fleet/registry"
-	"github.com/coreos/fleet/sign"
 )
 
 const (
@@ -44,15 +43,14 @@ func (t *task) String() string {
 	return fmt.Sprintf("{Type: %s, Job: %s, Reason: %q}", t.Type, jName, t.Reason)
 }
 
-func NewReconciler(reg registry.Registry, rStream registry.EventStream, verifier *sign.SignatureVerifier) (*AgentReconciler, error) {
-	ar := AgentReconciler{reg, rStream, verifier}
+func NewReconciler(reg registry.Registry, rStream registry.EventStream) (*AgentReconciler, error) {
+	ar := AgentReconciler{reg, rStream}
 	return &ar, nil
 }
 
 type AgentReconciler struct {
-	reg      registry.Registry
-	rStream  registry.EventStream
-	verifier *sign.SignatureVerifier
+	reg     registry.Registry
+	rStream registry.EventStream
 }
 
 // Run periodically attempts to reconcile the provided Agent until the stop
@@ -184,9 +182,8 @@ func (ar *AgentReconciler) doTask(a *Agent, t *task) (err error) {
 // Agent identified by the provided machine ID should currently be doing.
 func (ar *AgentReconciler) desiredAgentState(units []job.Unit, sUnits []job.ScheduledUnit, ms *machine.MachineState) (*AgentState, error) {
 	as := AgentState{
-		MState:     ms,
-		Jobs:       make(map[string]*job.Job),
-		verifyFunc: ar.verifyJobSignature,
+		MState: ms,
+		Jobs:   make(map[string]*job.Job),
 	}
 
 	sUnitMap := make(map[string]*job.ScheduledUnit)
@@ -223,9 +220,8 @@ func (ar *AgentReconciler) currentAgentState(a *Agent) (*AgentState, error) {
 
 	ms := a.Machine.State()
 	as := AgentState{
-		MState:     &ms,
-		Jobs:       jobs,
-		verifyFunc: ar.verifyJobSignature,
+		MState: &ms,
+		Jobs:   jobs,
 	}
 
 	return &as, nil
@@ -334,22 +330,3 @@ func (ar *AgentReconciler) calculateTasksForJob(dState, cState *AgentState, jNam
 
 	log.Errorf("Unable to determine how to reconcile Job(%s): desiredState=%#v currentState=%#V", jName, dJob, cJob)
 }
-
-// verifyJobSignature attempts to verify the integrity of the given Job by checking the
-// signature against a SignatureSet stored in the Registry
-func (ar *AgentReconciler) verifyJobSignature(j *job.Job) bool {
-	if ar.verifier == nil {
-		return true
-	}
-	ss, _ := ar.reg.JobSignatureSet(j.Name)
-	ok, err := ar.verifier.VerifyJob(j, ss)
-	if err != nil {
-		log.V(1).Infof("Error verifying signature of Job(%s): %v", j.Name, err)
-		return false
-	} else if !ok {
-		log.V(1).Infof("Job(%s) does not match signature", j.Name)
-		return false
-	}
-
-	return true
-}

agent/reconcile.go

@@ -321,7 +321,7 @@ func TestCalculateTasksForJob(t *testing.T) {
 	}
 
 	for i, tt := range tests {
-		ar, err := NewReconciler(registry.NewFakeRegistry(), nil, nil)
+		ar, err := NewReconciler(registry.NewFakeRegistry(), nil)
 		if err != nil {
 			t.Errorf("case %d: unexpected error from NewReconciler: %v", i, err)
 			continue

agent/reconcile_test.go

@@ -13,11 +13,6 @@ import (
 type AgentState struct {
 	MState *machine.MachineState
 	Jobs   map[string]*job.Job
-
-	// This is used to assert that the Agent is able to
-	// run a given job based on its signature. This feature
-	// is currently deprecated, so expect this to go away.
-	verifyFunc func(j *job.Job) bool
 }
 
 func NewAgentState(ms *machine.MachineState) *AgentState {
@@ -70,7 +65,6 @@ func globMatches(pattern, target string) bool {
 // the Agent's current state. A boolean indicating whether this is the
 // case or not is returned. The following criteria is used:
 //   - Agent must meet the Job's machine target requirement (if any)
-//   - Job must pass signature verification
 //   - Agent must have all of the Job's required metadata (if any)
 //   - Agent must have all required Peers of the Job scheduled locally (if any)
 //   - Job must not conflict with any other Jobs scheduled to the agent
@@ -79,10 +73,6 @@ func (as *AgentState) AbleToRun(j *job.Job) (bool, string) {
 		return false, fmt.Sprintf("agent ID %q does not match required %q", as.MState.ID, tgt)
 	}
 
-	if as.verifyFunc != nil && !as.verifyFunc(j) {
-		return false, "unable to verify signature"
-	}
-
 	metadata := j.RequiredTargetMetadata()
 	if len(metadata) != 0 {
 		if !machine.HasMetadata(as.MState, metadata) {

agent/state.go

@@ -3,13 +3,9 @@ package client
 import (
 	"github.com/coreos/fleet/machine"
 	"github.com/coreos/fleet/schema"
-	"github.com/coreos/fleet/sign"
 )
 
 type API interface {
-	CreateSignatureSet(*sign.SignatureSet) error
-	JobSignatureSet(string) (*sign.SignatureSet, error)
-
 	Machines() ([]machine.MachineState, error)
 
 	Unit(string) (*schema.Unit, error)

client/api.go

@@ -16,7 +16,6 @@ import (
 	"github.com/coreos/fleet/config"
 	"github.com/coreos/fleet/registry"
 	"github.com/coreos/fleet/server"
-	"github.com/coreos/fleet/sign"
 	"github.com/coreos/fleet/version"
 )
 
@@ -56,8 +55,8 @@ func main() {
 	cfgset.String("public_ip", "", "IP address that fleet machine should publish")
 	cfgset.String("metadata", "", "List of key-value metadata to assign to the fleet machine")
 	cfgset.String("agent_ttl", agent.DefaultTTL, "TTL in seconds of fleet machine state in etcd")
-	cfgset.Bool("verify_units", false, "Verify unit file signatures using local SSH identities")
-	cfgset.String("authorized_keys_file", sign.DefaultAuthorizedKeysFile, "File containing public SSH keys to be used for signature verification")
+	cfgset.Bool("verify_units", false, "DEPRECATED - This option is ignored")
+	cfgset.String("authorized_keys_file", "", "DEPRECATED - This option is ignored")
 
 	globalconf.Register("", cfgset)
 	cfg, err := getConfig(cfgset, *cfgPath)
@@ -173,7 +172,10 @@ func getConfig(flagset *flag.FlagSet, userCfgFile string) (*config.Config, error
 	}
 
 	if cfg.VerifyUnits {
-		log.Warning("WARNING: The signed/verified units feature is DEPRECATED and should not be used. It will be completely removed from fleet and fleetctl.")
+		log.Error("Config option verify_units is no longer supported - ignoring")
+	}
+	if len(cfg.AuthorizedKeysFile) > 0 {
+		log.Error("Config option authorized_keys_file is no longer supported - ignoring")
 	}
 
 	config.UpdateLoggingFlagsFromConfig(flag.CommandLine, &cfg)

fleet.go

@@ -23,7 +23,6 @@ import (
 	"github.com/coreos/fleet/pkg"
 	"github.com/coreos/fleet/registry"
 	"github.com/coreos/fleet/schema"
-	"github.com/coreos/fleet/sign"
 	"github.com/coreos/fleet/ssh"
 	"github.com/coreos/fleet/unit"
 	"github.com/coreos/fleet/version"
@@ -202,7 +201,8 @@ func main() {
 	}
 
 	if sharedFlags.Sign {
-		fmt.Fprintln(os.Stderr, "WARNING: The signed/verified units feature is DEPRECATED and should not be used. It will be completely removed from fleet and fleetctl.")
+		fmt.Fprintln(os.Stderr, "WARNING: The signed/verified units feature is DEPRECATED and cannot be used.")
+		os.Exit(2)
 	}
 
 	if cmd.Name != "help" && cmd.Name != "version" {
@@ -424,59 +424,6 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) {
 	return &u, nil
 }
 
-// signUnit signs the Unit of a Job using the public keys in the local SSH
-// agent, and pushes the resulting SignatureSet to the Registry
-func signUnit(u *schema.Unit) error {
-	sc, err := sign.NewSignatureCreatorFromSSHAgent()
-	if err != nil {
-		return fmt.Errorf("failed creating SignatureCreator: %v", err)
-	}
-
-	j := job.Job{
-		Name: u.Name,
-		Unit: *schema.MapSchemaUnitOptionsToUnitFile(u.Options),
-	}
-	ss, err := sc.SignJob(&j)
-	if err != nil {
-		return fmt.Errorf("failed signing Job(%s): %v", j.Name, err)
-	}
-
-	err = cAPI.CreateSignatureSet(ss)
-	if err != nil {
-		return fmt.Errorf("failed storing Job signature in registry: %v", err)
-	}
-
-	log.V(1).Infof("Signed Job(%s)", j.Name)
-	return nil
-}
-
-// verifyUnit attempts to verify the signature of the provided Unit's unit file using
-// the public keys in the local SSH agent
-func verifyUnit(u *schema.Unit) error {
-	sv, err := sign.NewSignatureVerifierFromSSHAgent()
-	if err != nil {
-		return fmt.Errorf("failed creating SignatureVerifier: %v", err)
-	}
-
-	ss, err := cAPI.JobSignatureSet(u.Name)
-	if err != nil {
-		return fmt.Errorf("failed attempting to retrieve SignatureSet of Job(%s): %v", u.Name, err)
-	}
-	j := &job.Job{
-		Name: u.Name,
-		Unit: *schema.MapSchemaUnitOptionsToUnitFile(u.Options),
-	}
-	verified, err := sv.VerifyJob(j, ss)
-	if err != nil {
-		return fmt.Errorf("failed attempting to verify Job(%s): %v", u.Name, err)
-	} else if !verified {
-		return fmt.Errorf("unable to verify Job(%s)", u.Name)
-	}
-
-	log.V(1).Infof("Verified signature of Job(%s)", u.Name)
-	return nil
-}
-
 // lazyCreateUnits iterates over a set of unit names and, for each, attempts to
 // ensure that a unit by that name exists in the Registry, by checking a number
 // of conditions and acting on the first one that succeeds, in order of:
@@ -485,11 +432,9 @@ func verifyUnit(u *schema.Unit) error {
 //  3. a corresponding unit template (if applicable) existing in the Registry
 //  4. a corresponding unit template (if applicable) existing on disk
 // Any error encountered during these steps is returned immediately (i.e.
-// subsequent units are not acted on). An error is also returned if none of the
-// above conditions match a given unit.
-// If signAndVerify is true, the unit will be signed (if it is created), or have
-// its signature verified if it already exists in the Registry.
-func lazyCreateUnits(args []string, signAndVerify bool) error {
+// subsequent Jobs are not acted on). An error is also returned if none of the
+// above conditions match a given Job.
+func lazyCreateUnits(args []string) error {
 	for _, arg := range args {
 		// TODO(jonboulle): this loop is getting too unwieldy; factor it out
 
@@ -503,11 +448,6 @@ func lazyCreateUnits(args []string, signAndVerify bool) error {
 		if u != nil {
 			log.V(1).Infof("Found Unit(%s) in Registry, no need to recreate it", name)
 			warnOnDifferentLocalUnit(arg, u)
-			if signAndVerify {
-				if err := verifyUnit(u); err != nil {
-					return err
-				}
-			}
 			continue
 		}
 
@@ -521,11 +461,6 @@ func lazyCreateUnits(args []string, signAndVerify bool) error {
 			if err != nil {
 				return err
 			}
-			if signAndVerify {
-				if err := signUnit(u); err != nil {
-					return err
-				}
-			}
 			continue
 		}
 
@@ -564,11 +499,6 @@ func lazyCreateUnits(args []string, signAndVerify bool) error {
 		if err != nil {
 			return err
 		}
-		if signAndVerify {
-			if err := signUnit(u); err != nil {
-				return err
-			}
-		}
 	}
 	return nil
 }

fleetctl/fleetctl.go

@@ -11,19 +11,19 @@ var (
 	cmdLoadUnits = &Command{
 		Name:    "load",
 		Summary: "Schedule one or more units in the cluster, first submitting them if necessary.",
-		Usage:   "[--sign] [--no-block|--block-attempts=N] UNIT...",
+		Usage:   "[--no-block|--block-attempts=N] UNIT...",
 		Run:     runLoadUnits,
 	}
 )
 
 func init() {
-	cmdLoadUnits.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "Sign unit file signatures and verify submitted units using local SSH identities.")
-	cmdLoadUnits.Flags.IntVar(&sharedFlags.BlockAttempts, "block-attempts", 0, "Wait until the units are loaded, performing up to N attempts before giving up. A value of 0 indicates no limit.")
-	cmdLoadUnits.Flags.BoolVar(&sharedFlags.NoBlock, "no-block", false, "Do not wait until the units have been loaded before exiting.")
+	cmdLoadUnits.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "DEPRECATED - this option cannot be used")
+	cmdLoadUnits.Flags.IntVar(&sharedFlags.BlockAttempts, "block-attempts", 0, "Wait until the jobs are loaded, performing up to N attempts before giving up. A value of 0 indicates no limit.")
+	cmdLoadUnits.Flags.BoolVar(&sharedFlags.NoBlock, "no-block", false, "Do not wait until the jobs have been loaded before exiting.")
 }
 
 func runLoadUnits(args []string) (exit int) {
-	if err := lazyCreateUnits(args, sharedFlags.Sign); err != nil {
+	if err := lazyCreateUnits(args); err != nil {
 		fmt.Fprintf(os.Stderr, "%v\n", err)
 		return 1
 	}