cmd/principal: Usability tweaks.
Prior to this commit, the command printed by "recvblessings" was
would always fail because of lack of caveats and would also
have to be edited to give a meaningful extension.
This commit changes it so that the exact command can be run successfully
- though it will generate prompts for the user to confirm the lack of
caveats and to add an extension.
For non-interactive uses, the caveat flags and the extension can
still be specified in the command-line.
Change-Id: I0c4fea11883878d776dfcc8a7dd19167e7e73a52
diff --git a/cmd/principal/doc.go b/cmd/principal/doc.go
index 4910fb7..167de0c 100644
--- a/cmd/principal/doc.go
+++ b/cmd/principal/doc.go
@@ -283,7 +283,7 @@
all other arguments will be extracted from the specified file.
Usage:
- principal bless [flags] [<principal to bless>] <extension>
+ principal bless [flags] [<principal to bless>] [<extension>]
<principal to bless> represents the principal to be blessed (i.e., whose public
key will be provided with a name). This can be either: (a) The directory
diff --git a/cmd/principal/main.go b/cmd/principal/main.go
index 72dbd94..c18bb1b 100644
--- a/cmd/principal/main.go
+++ b/cmd/principal/main.go
@@ -222,7 +222,7 @@
When --remote-arg-file is specified, only the blessing extension is required, as all other
arguments will be extracted from the specified file.
`,
- ArgsName: "[<principal to bless>] <extension>",
+ ArgsName: "[<principal to bless>] [<extension>]",
ArgsLong: `
<principal to bless> represents the principal to be blessed (i.e., whose public
key will be provided with a name). This can be either:
@@ -243,10 +243,17 @@
`,
Runner: v23cmd.RunnerFunc(func(ctx *context.T, env *cmdline.Env, args []string) error {
- if len(flagRemoteArgFile) > 0 && len(args) != 1 {
- return fmt.Errorf("when --remote-arg-file is provided, only <extension> is expected, provided %d", len(args))
- } else if len(flagRemoteArgFile) == 0 && len(args) != 2 {
- return fmt.Errorf("require exactly two arguments when --remote-arg-file is not provided, provided %d", len(args))
+ if len(flagRemoteArgFile) > 0 {
+ if len(args) > 1 {
+ return fmt.Errorf("when --remote-arg-file is provided, only <extension> is expected, provided %d", len(args))
+ }
+ if (len(flagBlessRemoteKey) + len(flagBlessRemoteToken)) > 0 {
+ return fmt.Errorf("--remote-key and --remote-token should not be specified when --remote-arg-file is")
+ }
+ } else if len(args) > 2 {
+ return fmt.Errorf("got %d arguments, require at most 2", len(args))
+ } else if (len(flagBlessRemoteKey) == 0) != (len(flagBlessRemoteToken) == 0) {
+ return fmt.Errorf("either both --remote-key and --remote-token should be set, or neither should")
}
p := v23.GetPrincipal(ctx)
@@ -265,14 +272,19 @@
if err != nil {
return err
}
- if !flagBlessRequireCaveats && len(caveats) == 0 {
+ if len(caveats) == 0 {
+ if flagBlessRequireCaveats {
+ if err := confirmNoCaveats(env); err != nil {
+ return err
+ }
+ }
caveats = []security.Caveat{security.UnconstrainedUse()}
}
if len(caveats) == 0 {
return errNoCaveats
}
- tobless, extension, remoteKey, remoteToken, err := blessArgs(args)
+ tobless, extension, remoteKey, remoteToken, err := blessArgs(env, args)
if err != nil {
return err
}
@@ -769,20 +781,18 @@
if err := server.Serve("", service, security.AllowEveryone()); err != nil {
return fmt.Errorf("failed to setup service: %v", err)
}
- // Proposed name:
- extension := fmt.Sprintf("extension%d", int(token[0])<<16|int(token[1])<<8|int(token[2]))
fmt.Println("Run the following command on behalf of the principal that will send blessings:")
fmt.Println("You may want to adjust flags affecting the caveats on this blessing, for example using")
- fmt.Println("the --for flag, or change the extension to something more meaningful")
+ fmt.Println("the --for flag")
fmt.Println()
if len(flagRemoteArgFile) > 0 {
if err := writeRecvBlessingsInfo(flagRemoteArgFile, p.PublicKey().String(), service.token, eps[0].Name()); err != nil {
return fmt.Errorf("failed to write recvblessings info to %v: %v", flagRemoteArgFile, err)
}
fmt.Printf("make %q accessible to the blesser, possibly by copying the file over and then run:\n", flagRemoteArgFile)
- fmt.Printf("principal bless --remote-arg-file=%v %v", flagRemoteArgFile, extension)
+ fmt.Printf("principal bless --remote-arg-file=%v", flagRemoteArgFile)
} else {
- fmt.Printf("principal bless --remote-key=%v --remote-token=%v %v %v\n", p.PublicKey(), service.token, eps[0].Name(), extension)
+ fmt.Printf("principal bless --remote-key=%v --remote-token=%v %v\n", p.PublicKey(), service.token, eps[0].Name())
}
fmt.Println()
fmt.Println("...waiting for sender..")
@@ -800,25 +810,65 @@
return fmt.Sprintf("%v%s", b, expiredMessage)
}
-func blessArgs(args []string) (tobless, extension, remoteKey, remoteToken string, err error) {
- if len(flagRemoteArgFile) > 0 && (len(flagBlessRemoteKey)+len(flagBlessRemoteToken) > 0) {
- return "", "", "", "", fmt.Errorf("--remote-key and --remote-token cannot be provided with --remote-arg-file")
- }
- if (len(flagBlessRemoteKey) == 0) != (len(flagBlessRemoteToken) == 0) {
- return "", "", "", "", fmt.Errorf("either both --remote-key and --remote-token should be set, or neither should")
- }
-
+func blessArgs(env *cmdline.Env, args []string) (tobless, extension, remoteKey, remoteToken string, err error) {
+ extensionInArgs := false
if len(flagRemoteArgFile) == 0 {
- tobless, extension = args[0], args[1]
+ tobless = args[0]
remoteKey = flagBlessRemoteKey
remoteToken = flagBlessRemoteToken
+ extensionInArgs = len(args) > 1
} else if len(flagRemoteArgFile) > 0 {
- extension = args[0]
remoteKey, remoteToken, tobless, err = blessArgsFromFile(flagRemoteArgFile)
+ extensionInArgs = len(args) > 0
+ }
+ if extensionInArgs {
+ extension = args[len(args)-1]
+ } else {
+ extension, err = readFromStdin(env, "Extension to use for blessing:")
}
return
}
+func confirmNoCaveats(env *cmdline.Env) error {
+ text, err := readFromStdin(env, `WARNING: No caveats provided
+It is generally dangerous to bless another principal without any caveats as
+that gives them unrestricted access to the blesser's credentials.
+
+Caveats can be specified with the --for or --caveat flags.
+
+Do you really wish to bless without caveats? (YES to confirm)`)
+ if err != nil || strings.ToUpper(text) != "YES" {
+ return errNoCaveats
+ }
+ return nil
+}
+
+func readFromStdin(env *cmdline.Env, prompt string) (string, error) {
+ fmt.Fprintf(env.Stdout, "%v ", prompt)
+ os.Stdout.Sync()
+ // Cannot use bufio because that may "lose" data beyond the line (the
+ // remainder in the buffer).
+ // Do the inefficient byte-by-byte scan for now - shouldn't be a problem
+ // given the common use case. If that becomes a problem, switch to bufio
+ // and share the bufio.Reader between multiple calls to readFromStdin.
+ buf := make([]byte, 0, 100)
+ r := make([]byte, 1)
+ for {
+ n, err := env.Stdin.Read(r)
+ if n == 1 && r[0] == '\n' {
+ break
+ }
+ if n == 1 {
+ buf = append(buf, r[0])
+ continue
+ }
+ if err != nil {
+ return "", err
+ }
+ }
+ return strings.TrimSpace(string(buf)), nil
+}
+
func blessOverFileSystem(p security.Principal, tobless string, with security.Blessings, extension string, caveats []security.Caveat) (security.Blessings, error) {
var key security.PublicKey
if finfo, err := os.Stat(tobless); err == nil && finfo.IsDir() {
diff --git a/cmd/principal/principal_v23_test.go b/cmd/principal/principal_v23_test.go
index bd8fde3..6186649 100644
--- a/cmd/principal/principal_v23_test.go
+++ b/cmd/principal/principal_v23_test.go
@@ -248,7 +248,7 @@
bobBlessFile = filepath.Join(outputDir, "bobBlessInfo")
)
- // Generate principals for alice and carol.
+ // Generate principals
bin.Start("create", aliceDir, "alice").WaitOrDie(os.Stdout, os.Stderr)
bin.Start("create", bobDir, "bob").WaitOrDie(os.Stdout, os.Stderr)
bin.Start("create", carolDir, "carol").WaitOrDie(os.Stdout, os.Stderr)
@@ -259,8 +259,8 @@
{
inv := bin.Start("--v23.credentials="+carolDir, "--v23.tcp.address=127.0.0.1:0", "recvblessings")
args = append([]string{"bless", "--require-caveats=false"}, blessArgsFromRecvBlessings(inv)...)
- // Replace the random extension suggested by recvblessings with "friend/carol"
- args[len(args)-1] = "friend/carol"
+ // Use the "friend/carol" extension
+ args = append(args, "friend/carol")
}
bin.WithEnv(credEnv(aliceDir)).Start(args...).WaitOrDie(os.Stdout, os.Stderr)
@@ -270,7 +270,7 @@
inv := bin.Start("--v23.credentials="+carolDir, "--v23.tcp.address=127.0.0.1:0", "recvblessings", "--for-peer=alice", "--set-default=false")
// recvblessings suggests a random extension, find the extension and replace it with friend/carol/foralice.
args = append([]string{"bless", "--require-caveats=false"}, blessArgsFromRecvBlessings(inv)...)
- args[len(args)-1] = "friend/carol/foralice"
+ args = append(args, "friend/carol/foralice")
}
bin.WithEnv(credEnv(aliceDir)).Start(args...).WaitOrDie(os.Stdout, os.Stderr)
@@ -279,13 +279,14 @@
inv := bin.Start("--v23.credentials="+carolDir, "--v23.tcp.address=127.0.0.1:0", "recvblessings", "--for-peer=bob", "--set-default=false", "--remote-arg-file="+bobBlessFile)
// recvblessings suggests a random extension, use friend/carol/forbob instead.
args = append([]string{"bless", "--require-caveats=false"}, blessArgsFromRecvBlessings(inv)...)
- args[len(args)-1] = "friend/carol/forbob"
+ args = append(args, "friend/carol/forbob")
}
bin.WithEnv(credEnv(bobDir)).Start(args...).WaitOrDie(os.Stdout, os.Stderr)
listenerInv := bin.Start("--v23.credentials="+carolDir, "--v23.tcp.address=127.0.0.1:0", "recvblessings", "--for-peer=alice/...", "--set-default=false", "--vmodule=*=2", "--logtostderr")
args = append([]string{"bless", "--require-caveats=false"}, blessArgsFromRecvBlessings(listenerInv)...)
+ args = append(args, "willfail")
{
// Mucking around with remote-key should fail.
@@ -335,6 +336,70 @@
}
}
+func V23TestRecvBlessingsInteractive(t *v23tests.T) {
+ var (
+ outputDir = t.NewTempDir("")
+ bin = t.BuildGoPkg("v.io/x/ref/cmd/principal")
+ aliceDir = filepath.Join(outputDir, "alice")
+ bobDir = filepath.Join(outputDir, "bob")
+ aliceBin = bin.WithEnv(credEnv(aliceDir))
+ )
+
+ // Generate principals
+ bin.Start("create", aliceDir, "alice").WaitOrDie(os.Stdout, os.Stderr)
+ bin.Start("create", bobDir, "bob").WaitOrDie(os.Stdout, os.Stderr)
+
+ // Run recvblessings on bob
+ recv := bin.Start("--v23.credentials="+bobDir, "--v23.tcp.address=127.0.0.1:0", "recvblessings")
+ args := blessArgsFromRecvBlessings(recv)
+
+ // When running the exact command, must be prompted about caveats.
+ {
+ inv := aliceBin.Start(append([]string{"bless"}, args...)...)
+ inv.Expect("WARNING: No caveats provided")
+ // Saying something other than "yes" or "YES"
+ // should fail.
+ fmt.Fprintln(inv.Stdin(), "yeah")
+ if err := inv.Wait(os.Stdout, os.Stderr); err == nil {
+ t.Fatalf("Expected principal bless to fail because the wrong input was provided")
+ }
+ }
+ // When agreeing to have no caveats, must specify an extension
+ {
+ inv := aliceBin.Start(append([]string{"bless"}, args...)...)
+ inv.Expect("WARNING: No caveats provided")
+ fmt.Fprintln(inv.Stdin(), "yes")
+ inv.CloseStdin()
+ if err := inv.Wait(os.Stdout, os.Stderr); err == nil {
+ t.Fatalf("Expected principal bless to fail because no extension was provided")
+ }
+ }
+ // When providing both, the bless command should succeed.
+ {
+ inv := aliceBin.Start(append([]string{"bless"}, args...)...)
+ fmt.Fprintln(inv.Stdin(), "YES")
+ fmt.Fprintln(inv.Stdin(), "friend/bobby")
+ if err := inv.Wait(os.Stdout, os.Stderr); err != nil {
+ t.Fatal(err)
+ }
+ }
+ got := removePublicKeys(bin.Start("--v23.credentials="+bobDir, "dump").Output())
+ want := `Public key : XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX
+Default Blessings : alice/friend/bobby
+---------------- BlessingStore ----------------
+Default Blessings alice/friend/bobby
+Peer pattern Blessings
+... alice/friend/bobby
+---------------- BlessingRoots ----------------
+Public key Pattern
+XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX [alice]
+XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX [bob]
+`
+ if want != got {
+ t.Fatalf("unexpected output, got\n%s, wanted\n%s", got, want)
+ }
+}
+
func V23TestFork(t *v23tests.T) {
var (
outputDir = t.NewTempDir("")
diff --git a/cmd/principal/v23_test.go b/cmd/principal/v23_test.go
index 842bc1f..3aa5c29 100644
--- a/cmd/principal/v23_test.go
+++ b/cmd/principal/v23_test.go
@@ -49,6 +49,10 @@
v23tests.RunTest(t, V23TestRecvBlessings)
}
+func TestV23RecvBlessingsInteractive(t *testing.T) {
+ v23tests.RunTest(t, V23TestRecvBlessingsInteractive)
+}
+
func TestV23Fork(t *testing.T) {
v23tests.RunTest(t, V23TestFork)
}
