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

Feature/gh 201 use ssh agent #207

Merged
merged 15 commits into from
Dec 7, 2018
Merged

Conversation

stefbenoist
Copy link
Contributor

@stefbenoist stefbenoist commented Nov 23, 2018

Pull Request description

Use ssh-agent to not write ssh private keys on disk
As: An ops
I want: to ensure that connection credentials are not left non-encrypted on disk
So that: I can relax and sleep this night

AC1: Allow to either specify a key path or a key content into the credential part of a Compute
AC2: before running Ansible command store key content into an ssh-agent for a limited amount of time
AC3: check if it also works with paramiko (anyways document what works or not)
AC4: support key content for terraform provisioning
AC5: Keys content should never be store unencrypted on disk

Description of the change

  • Add use_ssh_agent config option set to true by default.
  • Allow using private key file if use_ssh_agent=false
  • Allow provide private key content for Terraform check connection
  • Update doc.

What I did

Add ssh_agent API to:

  • create new ssh-agent, add/remove keys, stop...

Update Ansible to :

  • use ssh-agent if use_ssh_agent=true
  • use private key file if use_ssh_agent=false (check if private key is a valid path)

Update Terraform to:

  • Allow providing private key content for Terraform check connection
  • Use terraform env var to store private key content

How to verify it

  • Deploy a topology with SSH private key content set in Compute credentials (On demand resource configuration within the A4C Location configuration)
  • Config Yorc with use_ssh_agent=false and deploy the same topology with private key file path.
  • Test with valid and not valid cerficates. When using a valid certificate, you can use our home made yorc key pair, or another one created by your own, but in this latter case, you should use a tool that generates the private key in DER format (BER format not suuported)

Description for the changelog

Applicable Issues

fixes #201

stefbenoist and others added 7 commits November 20, 2018 16:56
Allow to provide private key content to Terraform instead of file
Allow to use ssh-agent for Ansible ssh connections
Check private key is valid path when use_ssh_agent is false
Fix concurrency issue on sshAgent use with googleGenerator
@laurentganne
Copy link
Contributor

Seeing a failure in unit tests:

        --- FAIL: TestRunConsulGooglePackageTests/googleProvider/simpleComputeInstanceWithPersistentDisk (4.43s)
	Error Trace:	
	Error:		Received unexpected error "\"~/.ssh/yorc.pem\" is not a valid path"
	Messages:	Unexpected error attempting to generate compute instance for simpleComputeInstanceWithPersistentDisk

Looks like the testdata yaml file describing the compute should be updated to provide a private key content ?

@stefbenoist
Copy link
Contributor Author

The file ~/.ssh/yorc.pem is not present in Travis build vm. I fix it with a test key pem file

@codecov
Copy link

codecov bot commented Nov 27, 2018

Codecov Report

Merging #207 into develop will decrease coverage by 0.42%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #207      +/-   ##
===========================================
- Coverage    39.79%   39.37%   -0.43%     
===========================================
  Files          159      162       +3     
  Lines        14574    14851     +277     
===========================================
+ Hits          5800     5847      +47     
- Misses        7872     8077     +205     
- Partials       902      927      +25
Impacted Files Coverage Δ
config/config.go 79.66% <ø> (ø) ⬆️
prov/ansible/execution.go 24.89% <0%> (-1.35%) ⬇️
prov/terraform/aws/generator.go 0% <0%> (ø) ⬆️
prov/terraform/openstack/generator.go 3.4% <0%> (ø) ⬆️
prov/terraform/google/generator.go 0% <0%> (ø) ⬆️
commands/server.go 76.33% <100%> (+0.31%) ⬆️
helper/stringutil/stringutil.go 84.84% <100%> (+4.07%) ⬆️
prov/terraform/google/compute_instance.go 64.47% <40%> (-4.81%) ⬇️
prov/terraform/openstack/osinstance.go 39.17% <50%> (-2.54%) ⬇️
prov/terraform/aws/aws_instance.go 72.58% <50%> (-2.62%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 807af37...8048bc1. Read the comment docs.

helper/sshutil/sshutil.go Show resolved Hide resolved
// Truncate truncates a string if it's longer than defined length
// Add ... after the cut
func Truncate(str string, l int) string {
if len(str) > l {
Copy link
Member

Choose a reason for hiding this comment

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

What if l < 0
or l < 3 ?

if !e.cfg.UseSSHAgent && sshCredentials.privateKey != "" {
// check privateKey's a valid path
if is, err := pathutil.IsValidPath(sshCredentials.privateKey); err != nil || !is {
return errors.Errorf("%q is not a valid path", stringutil.Truncate(sshCredentials.privateKey, 20))
Copy link
Member

Choose a reason for hiding this comment

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

Why truncate where?
We want to will typically want to get the invalid path and make ls /my/deep/deep/path/to/key.pem ; ls /my/deep/deep/path/to; ls /my/deep/deep/path/; ls /my/deep/deep/ and so on to find the missing part.

@@ -37,11 +38,24 @@ const infrastructureName = "google"
type googleGenerator struct {
}

func (g *googleGenerator) GenerateTerraformInfraForNode(ctx context.Context, cfg config.Configuration, deploymentID, nodeName, infrastructurePath string) (bool, map[string]string, []string, error) {
func getSSHAgent(ctx context.Context, privateKey string) (*sshutil.SSHAgent, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not in the commons package?

prov/terraform/commons/generator.go Outdated Show resolved Hide resolved
Copy link
Contributor

@adidanes adidanes left a comment

Choose a reason for hiding this comment

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

Its OK for me

@stefbenoist stefbenoist merged commit fde03f7 into develop Dec 7, 2018
@stefbenoist stefbenoist deleted the feature/gh-201-use-ssh-agent branch December 7, 2018 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ssh-agent to not write ssh private keys on disk
4 participants