Adding client authentication with public key fuction. #181

itouri · 2017-06-16T16:08:32Z

$ ./openvdc console -h
Connect to an instance.

Usage:
  openvdc console [Instance ID] [options] [--] [commands] [flags]

Examples:

% openvdc console i-000000001
root@i-00000001#

% echo ls | openvdc console i-000000001
% cat test.sh | openvdc console i-000000001


Flags:
  -i, --identity-file string   Selects a file from which the identity (private key) for public key authentication is read
      --show                   Show console information

Metallion · 2017-07-04T05:51:35Z

ci/citest/acceptance-test/tests/cmd_console_test.go

+
+
+	RunCmdWithTimeoutAndReportFail(t, 10, 5, "openvdc", "destroy", instance_id)
+	WaitInstance(t, 5*time.Minute, instance_id, "TERMINATED", nil)


How is this a test for console authentication? It just starts an instance and then terminates it.

Added code to test the console. And change function name to TestCmdConsole_ShowOptionAuthenticationNone.

# Conflicts: # ci/citest/acceptance-test/tests/cmd_console_test.go # model/model.pb.go # registry/bindata_assetfs.go

…arameter of LxcTemplate from json to protocol buffer.

# Conflicts: # model/model.pb.go # registry/bindata_assetfs.go

…pub_key of the VM template.

unakatsuo · 2017-08-22T02:17:41Z

handlers/vm/base.go

+		}
+	}
+
+	// Check that the key is in SECSH format.


We don't need to support SECSH format at this version.

Changed that part into a comment.

unakatsuo · 2017-08-22T02:22:44Z

handlers/vm/base.go

+	}
+
+	keyStr := string(key[:])
+	// Check that the key is in OpenSSH format.


Why do you check same thing that happens in ssh.ParsePublicKey()?

https://github.com/golang/crypto/blob/eb71ad9bd329b5ac0fd0148dd99bd62e8be8e035/ssh/keys.go#L246-L258

ssh.ParsePublicKey() can parse only RFC4253 format key. Therefore if we want to parse other type public key then we must parse by own.

unakatsuo · 2017-08-22T02:25:28Z

handlers/vm/base.go

+			return handlers.ErrInvalidTemplate(h, "ssh_public_key is not set")
+		}
+
+		isValidate := validatePublicKey([]byte(sshPubKey))


Just call ssh.ParsePublicKey() then return the error from the function. This function squash the error details from ssh.ParsePublicKey().

Changed the return type of validatePublicKey([]byte) to error.

and changed the part supporting SECSH format to comment.

Due to the change of golang.org/x/crypto/ssh . golang/crypto@e4e2799dd7aa

Metallion

I think this is almost ok but it just needs a test to confirm that openvdc console fails when the wrong private key is provided.

Metallion · 2017-10-13T08:30:36Z

ci/citest/acceptance-test/tests/cmd_console_test.go

@@ -21,6 +22,23 @@ func runConsoleCmd(instance_id string, t *testing.T) {
 	RunCmdAndExpectFail(t, "sh", "-c", fmt.Sprintf("openvdc console %s -- false", instance_id))
 }

+func runConsoleCmdWithPrivatekey(instance_id string, private_key_path string, t *testing.T) {
+	RunCmdAndReportFail(t, "openvdc", "console", instance_id, "-i", private_key_path)
+	RunCmdAndReportFail(t, "sh", "-c", fmt.Sprintf("openvdc console %s -i %s ls", instance_id, private_key_path))


Why use sh here? Why not just do this?

RunCmdAndReportFail(t,"openvdc", "console", instance_id, "-i", private_key_path, "ls")

Also you should run gofmt on this file and any other files that aren't formatted correctly. It's best to set up your editor to run it on save.

This test function integrate to TestLXCCmdConsole_AuthenticationPubkeymethod and modified to doesn't using "sh".

Metallion · 2017-10-13T08:33:13Z

ci/citest/acceptance-test/tests/cmd_console_test.go

+	RunCmdAndReportFail(t, "sh", "-c", fmt.Sprintf("openvdc console %s -i %s ls", instance_id, private_key_path))
+	RunCmdAndReportFail(t, "sh", "-c", fmt.Sprintf("openvdc console %s -i %s -- ls", instance_id, private_key_path))
+	RunCmdAndExpectFail(t, "sh", "-c", fmt.Sprintf("openvdc console %s -i %s -- false", instance_id, private_key_path))
+}


You should also confirm that the console command fails when you provide the wrong private key.

Added TestLXCCmdConsole_AuthenticationPubkeymethod testing worng key.

# Conflicts: # model/model.pb.go # model/resource_templates.go # registry/bindata_assetfs.go

Metallion

Since the ESXi feature was merged to master, the tests started failing on this one.

# github.com/axsh/openvdc/scheduler
scheduler/mesos.go:174: cannot use esxi (type *model.EsxiTemplate) as type model.InstanceResource in argument to model.IsMatchingNodeGroups:
	*model.EsxiTemplate does not implement model.InstanceResource (missing GetAuthenticationType method)

I know this is annoying since it worked before, but you'll have to update to code to work with the latest master.

Metallion · 2017-10-23T09:32:38Z

ci/citest/acceptance-test/tests/cmd_console_test.go

+		RunCmdAndExpectFail(t, "openvdc", "console", instance_id, "-i", private_key_path)
+	} else {
+		RunCmdAndReportFail(t, "openvdc", "console", instance_id, "-i", private_key_path)
+	}


This function is only called twice. Once with expect_fail = true and once with expect_fail = false.

Since only one line gets executed for each call, I think it's better to write those lines directly in the test instead of in a separate function.

Deleted runConsoleCmdWithPrivatekey and integrated to TestLXCCmdConsole_AuthenticationPubkey.

# Conflicts: # api/v1.pb.go # model/cluster.pb.go # model/model.pb.go

# Conflicts: # model/model.pb.go

# Conflicts: # api/v1.pb.go # model/model.pb.go # registry/bindata_assetfs.go # vendor/vendor.json

unakatsuo · 2018-02-23T07:36:53Z

cmd/openvdc/cmd/console.go

+			case model.AuthenticationType_PUB_KEY:
+				if indentityFile == "" {
+					log.Fatalf("Required private key but not setted")
+				} else {


no need for the else block.

Deleted it.

unakatsuo · 2018-02-23T07:38:25Z

cmd/openvdc/cmd/console.go

@@ -19,8 +21,11 @@ import (
 	"google.golang.org/grpc"
 )

+var indentityFile string


Typo and no need to be defined as a global variable. Can be refered as cmd.Flags().GetString("identity-file").

Modified typo and using to GetString()

unakatsuo · 2018-02-23T07:43:39Z

proto/v1.proto

@@ -90,6 +90,7 @@ message ConsoleReply {
  string instance_id = 1 [json_name="instance_id"];
  model.Console.Transport type = 2;
  string address = 3;
+  model.AuthenticationType authType = 4;


Breaks the convention.

model.AuthenticationType auth_type = 4 [json_name="auth_type"];

Added json_name tag for auth_type.

unakatsuo · 2018-02-23T10:11:30Z

handlers/vm/lxc/lxc.go

 	if err := flags.Parse(args); err != nil {
 		return err
 	}
 	mdst.Vcpu = int32(vcpu)
 	mdst.MemoryGb = int32(mem)
+	format := model.AuthenticationType_value[strings.ToUpper(authType)]


Missing the validation if --authtication_type gets unknown value.

Added validation of authtication_type to lxc.go and qemu.go.

itouri added 4 commits June 9, 2017 18:37

Adding parameter for authentication method.

20f480b

Adding CI test code for authentication parameters.

67629e3

Merge branch 'master' into console_auth

ba21474

Add function that validate ssh_public_key of vm/lxc template parameter

e9ce98c

itouri force-pushed the console_auth branch from 4a0e3fb to e9ce98c Compare June 16, 2017 16:27

Metallion requested a review from unakatsuo July 4, 2017 05:03

Metallion suggested changes Jul 4, 2017

View reviewed changes

itouri added 9 commits July 7, 2017 15:04

Merge branch 'master' into console_auth

8329e18

# Conflicts: # ci/citest/acceptance-test/tests/cmd_console_test.go # model/model.pb.go # registry/bindata_assetfs.go

Modify lxc.go to can convert authentication_type and ssh_public_key p…

e56737c

…arameter of LxcTemplate from json to protocol buffer.

Merge branch 'master' into console_auth

a8e918f

Remove processing read file of ssh public key.

b72ff81

Adding lxc_auth_ssh.json

7398602

Merge branch 'master' into console_auth

fd9df89

# Conflicts: # model/model.pb.go # registry/bindata_assetfs.go

Change MergeJSON to use common function of ParseTemplate.

7d92ca6

Modify validation process of LxcTemplate.

dc86ac5

Divided validation authentication_type functions into common functions.

8714d18

itouri changed the title ~~Console auth~~ Adding attribute of authentication to json schema. Aug 3, 2017

itouri added 2 commits August 10, 2017 15:05

Modify that common the validation to the authentication_type and ssh_…

11a63f7

…pub_key of the VM template.

Modify Qemu test contents

186568b

itouri changed the title ~~Adding attribute of authentication to json schema.~~ Adding client authentication with public key. Aug 10, 2017

itouri changed the title ~~Adding client authentication with public key.~~ Adding client authentication with public key fuction. Aug 10, 2017

itouri added 2 commits August 10, 2017 17:12

Made SSH server to can authentication with public key.

da2d781

Add authentication_type and ssh_public_key to MergeArgs() as parameters.

e6e0984

unakatsuo suggested changes Aug 22, 2017

View reviewed changes

itouri added 6 commits August 22, 2017 15:54

Changed the return type of validatePublicKey([]byte) to error.

2de4fcc

and changed the part supporting SECSH format to comment.

Modify the SSH server to authenticate the client.

f7b44e9

Delete extra line of code about authentification.

eb79945

Adding parameter of Auth to ClientConfig

7519521

Modify arg that is passed to NewSshConsole

fdc200e

Delete if section of parse and set indentifyFile

3171355

Masahiro Fujiwara added 6 commits September 7, 2017 18:19

Merge branch 'fix-jenkins-local-merge' into console_auth

c68c7c0

Upgrade golang.org/x/crypto/ssh

fe940a5

Merge branch 'master' into console_auth

3d94237

Host key callback became mandate parameter.

d036635

Due to the change of golang.org/x/crypto/ssh . golang/crypto@e4e2799dd7aa

Apply same ssh client host key check to acceptance test binary

ff7a886

Same ssh host key change with d036635

299e1ac

unakatsuo approved these changes Oct 4, 2017

View reviewed changes

Metallion suggested changes Oct 13, 2017

View reviewed changes

itouri and others added 3 commits October 13, 2017 18:40

Adding incorrect private key console test.

1596b50

Modify the test of console with public key.

853ffa6

Merge branch 'master' into console_auth

bb8bca2

# Conflicts: # model/model.pb.go # model/resource_templates.go # registry/bindata_assetfs.go

Metallion suggested changes Oct 23, 2017

View reviewed changes

Masahiro Fujiwara and others added 12 commits October 24, 2017 18:40

Move console auth methods to new interface

533c3d0

Cast to ConsoleAuthAttributes type instead InstanceResource type

8456dd5

Merge branch 'master' into console_auth

6554021

# Conflicts: # api/v1.pb.go # model/cluster.pb.go # model/model.pb.go

Merge branch 'fix-connection-status' into console_auth

d386fad

# Conflicts: # model/model.pb.go

Merge branch 'master' into console_auth

343c305

# Conflicts: # api/v1.pb.go # model/model.pb.go # registry/bindata_assetfs.go # vendor/vendor.json

Modified the separated function to two lines

f4782d3

Add AuthenticationType to EsxiTmplate

e4a0ecd

Modified TestLXCCmdConsole_AuthenticationPubkey

bc537e3

Added HostKeyCallback to needed place

733384b

Modified none auth type sshd.go logic

ee4c4ad

Added auth password method to ssh config

b78fbdc

Added ssh.Password("")

3f9bed5

unakatsuo suggested changes Feb 23, 2018

View reviewed changes

itouri added 2 commits February 23, 2018 17:12

Added json_name field of authType

508f170

Modified identify typo and change to local variable from global

4155a3c

unakatsuo suggested changes Feb 23, 2018

View reviewed changes

itouri added 2 commits February 23, 2018 19:23

Added validation of authtication_type to lxc.go and qemu.go

f9112cf

Modified lxc and qemu AuthenticationType validation.

6b769a9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding client authentication with public key fuction. #181

Adding client authentication with public key fuction. #181

itouri commented Jun 16, 2017 •

edited by unakatsuo

Loading

Metallion Jul 4, 2017

itouri Sep 1, 2017

unakatsuo Aug 22, 2017

itouri Aug 22, 2017

unakatsuo Aug 22, 2017 •

edited

Loading

itouri Aug 22, 2017

unakatsuo Aug 23, 2017

unakatsuo Aug 22, 2017

itouri Aug 22, 2017

Metallion left a comment

Metallion Oct 13, 2017

itouri Feb 23, 2018

Metallion Oct 13, 2017

itouri Feb 23, 2018

Metallion left a comment

Metallion Oct 23, 2017

itouri Feb 23, 2018

unakatsuo Feb 23, 2018

itouri Feb 23, 2018

unakatsuo Feb 23, 2018

itouri Feb 23, 2018

unakatsuo Feb 23, 2018

itouri Feb 23, 2018

unakatsuo Feb 23, 2018

itouri Feb 23, 2018



		RunCmdWithTimeoutAndReportFail(t, 10, 5, "openvdc", "destroy", instance_id)
		WaitInstance(t, 5*time.Minute, instance_id, "TERMINATED", nil)

Adding client authentication with public key fuction. #181

Are you sure you want to change the base?

Adding client authentication with public key fuction. #181

Conversation

itouri commented Jun 16, 2017 • edited by unakatsuo Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unakatsuo Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Metallion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Metallion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itouri commented Jun 16, 2017 •

edited by unakatsuo

Loading

unakatsuo Aug 22, 2017 •

edited

Loading