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

Switch to Travis settings env var to store private key #243

Merged
merged 16 commits into from
Jul 9, 2019

Conversation

dhimmel
Copy link
Member

@dhimmel dhimmel commented Jul 2, 2019

Closes #91
Closes #246

Ready for review

Using Travis environment variables set in the web settings GUI succeeded for manubot/catalog.

Removes the need for the travis ruby gem during setup, potentially allowing setup on Windows Bash.

In progress.

bash "-v varname" to test whether variable assinged
True if the shell variable varname is set (has been assigned a value).
https://stackoverflow.com/a/17538964/4651668

bash here string to input key to ssh-add
https://stackoverflow.com/a/46253163/4651668

Storing private keys using environment varaible in Travis settings
wp-cli/wp-cli#3798
https://github.com/Automattic/vip-go-mu-plugins/blob/5f985910c849b1eb104c3b4b19776e9be0fb5b87/ci/deploy.sh#L26-L31
Assumes newline characters use \n encoding

echo: specify -e to enable interpretation of the backslash escapes
@dhimmel
Copy link
Member Author

dhimmel commented Jul 2, 2019

This biggest barrier to this approach is going from the private key as text in deploy.key to the Travis settings environment variable. It is not entirely clear to me what characters are acceptable in the Travis value field and how escaping operates:

From Travis Docs:

These values are used directly in your build, so make sure to escape special characters (for bash) accordingly. In particular, if a value contains spaces, you should put quotes around that value. E.g. my secret passphrase should be written "my secret passphrase".

From the Travis CI settings page:

If your secret variable has special characters like &, escape them by adding \ in front of each special character. For example, ma&w!doc would be entered as ma\&w\!doc.

Here is what a private key looks like (1024 bits, ours are longer):

-----BEGIN RSA PRIVATE KEY-----
MIICXQIBAAKBgQDLgt1feiXdtoDC7SU02gMHdQTgaG3MhffGJLeM0HFH0xRMfiJb
IdE6H36d3cheId3qiFna3XmcZiIebT3gsIJxp3MsmInBm786UBjR+JKtlzoGJ16n
lc0+O+3mmrqwyjuAxWSACjL43cCRPo95pITuQdn3uMsIW1QSlHebTVrgywIDAQAB
AoGBAIJwBR1N9/ksIUlOn/tJBIoCCbcfl8hv0UhxfifF7eVgN/FzIugQO78qMohS
fzs+L7ND43uxWmHQ4GtqDy/1UhUKMaHQw/x09G9thl5lC37dQyP0xbgFrHY8hHAC
B5eo50+fiGrIR1PD7hlGviTfxmb+YZhrET9+0Ey7XRcM+6D5AkEA/m0Q7Vat80gP
VQmbZDX4xw4E9hp82oE7/RNyPplJBrCZMVkhipQYn9VOzyJM5QPmOpNVTEWpJA6Q
9tOVApY5vwJBAMzFKjbD207+HDnjlUR4R0cpP6S5Ax1qZYTn2Nl14h0/dqj9sn0l
RAm21t/J1o3KFnMJFlNHkGXzXHQYc51vo/UCQQCtCkbR9PsYFHGBF6idDmwmDe3n
5/n0rqK7LCeuVZiqOR/nxUQfuTvKMUyJaj28INvMCPqhhltUT6feh+a3vK6HAkAV
6kwYTGHeVGfk8ix5hX3raci78mkY7tgqnz2gGHO4uaATegNuTVy4xW69yLZDuoso
iKJxC0my/5a3fh5xjQ85AkA0GNz4+/T3M/PLgpQ0PJilMOevvQ+sMdvc4/64YPTF
/9dvNguk6zD7oY5xWpVt16Rju1fVq57x97IF2IUvphIn
-----END RSA PRIVATE KEY-----

So we have to massage this into something that works as a Travis settings environment variable. Two approaches were suggested in wp-cli/wp-cli#3798:

  1. replace newlines with \n and surround with double-quotes ". This is the approach I got to work in manubot/catalog. I did the conversion semi-manually, so we'll need to look into cross-platform automated approaches. The tr, sed, and paste commands may all be possibilities. Would be good to know which are available on Git for Windows.

  2. use a base64 encoding to convert the private key to a single alphanumeric string. However, the API for the mac and linux base64 command are different. In addition, I am not sure whether git for windows has base64. @agitter can you check? We could use a simple python script... but will every setup environment have Python? We could also use a separate command for windows users like certutil.

For SETUP.md, we are looking for a cross-platform and robust way to encode/escape the private key. deploy.sh has a much more controlled environment & system and is therefore less of an issue.

@dhimmel
Copy link
Member Author

dhimmel commented Jul 2, 2019

If we determine setup environments should have python, here's a cross-platform script (Python 2 & 3 compatible) to convert deploy.key to base64:

import base64
key = open('deploy.key', 'rb').read()
base64_key = base64.standard_b64encode(key)
print(base64_key.decode())

And here's a version that uses "universal newlines" to make sure all newlines are encoded as \n:

import base64
text_key = open('deploy.key', 'U').read()
base64_key = base64.standard_b64encode(text_key.encode())
print(base64_key.decode())

@agitter
Copy link
Member

agitter commented Jul 2, 2019

The current machine I'm on has a somewhat old version of Git Bash for Windows. It has tr, sed, paste, and base64 available. I did not test their behavior but could easily do so.

We could use a simple python script... but will every setup environment have Python?

It's hard to assess whether Git Bash or Python is more likely to be available on Windows machines of Manubot users. My uneducated guess is that Python is more common than Git Bash. If we (eventually) want to enable setup for non-technical users, they may not have either available.

Rootstock will no longer need these files once it switches to an
environment variable for the deploy key.

Hoping that this will not cause conflicts with legacy manuscripts.
@dhimmel
Copy link
Member Author

dhimmel commented Jul 2, 2019

@agitter can you paste the man page for base64 on Windows below? Try man base64 | cat.

@dhimmel
Copy link
Member Author

dhimmel commented Jul 2, 2019

base64 encoding commands

Linux and Windows with GNU coreutils version:

# encode
base64 --wrap=1000000 deploy.key
# decode
base64 --decode <<< $TESTKEY

macOS:

# encode
base64 --break=1000000 deploy.key

@agitter
Copy link
Member

agitter commented Jul 2, 2019

Oddly, Git Bash doesn't have the man command. Does this help?

$ base64 --help
Usage: base64 [OPTION]... [FILE]
Base64 encode or decode FILE, or standard input, to standard output.

With no FILE, or when FILE is -, read standard input.

Mandatory arguments to long options are mandatory for short options too.
  -d, --decode          decode data
  -i, --ignore-garbage  when decoding, ignore non-alphabet characters
  -w, --wrap=COLS       wrap encoded lines after COLS character (default 76).
                          Use 0 to disable line wrapping

      --help     display this help and exit
      --version  output version information and exit

The data are encoded as described for the base64 alphabet in RFC 4648.
When decoding, the input may contain newlines in addition to the bytes of
the formal base64 alphabet.  Use --ignore-garbage to attempt to recover
from any other non-alphabet bytes in the encoded stream.

GNU coreutils online help: <http://www.gnu.org/software/coreutils/>
Full documentation at: <http://www.gnu.org/software/coreutils/base64>
or available locally via: info '(coreutils) base64 invocation'

$ base64 --version
base64 (GNU coreutils) 8.25
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Simon Josefsson.

if [ -v MANUBOT_SSH_PRIVATE_KEY ]; then
set +o xtrace # TODO: better way of temporarily disabling xtrace
base64 --decode <<< "$MANUBOT_SSH_PRIVATE_KEY" | ssh-add -
else
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously we didn't disable xtrace, which means we probably were leaking $encrypted_9befd6eddffe_key to the build logs. However, if you check the build logs, I believe Travis CI was smart enough to remove that output.

Anyways we want a way to disable xtrace for specific commands. Ideally, after these commands finish, xtrace operates based on whether it was activate or not before the hidden commands.

@michaelmhoffman / @dongbohu any suggestions? Would also love any feedback on the changes to the bash command changes above since would be great to make this as robust as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented xtrace disabling/re-enabling in c1abee3.

Copy link
Contributor

Choose a reason for hiding this comment

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

Again I don't recommend using xtrace routinely like this. If you must, then you can disable it temporarily in a subshell like this:

(
    set +o xtrace
    base64 --decode <<< "$MANUBOT_SSH_PRIVATE_KEY" | ssh-add -
)

Remember that variables changed in the subshell don't affect the parent shell. That is not a problem now here.

SETUP.md Outdated Show resolved Hide resolved
SETUP.md Show resolved Hide resolved
@dhimmel dhimmel requested review from agitter and dongbohu July 2, 2019 23:34
SETUP.md Show resolved Hide resolved
@dhimmel
Copy link
Member Author

dhimmel commented Jul 3, 2019

Here's the base64 man page on a mac:

base64(1)                 BSD General Commands Manual                base64(1)

NAME
     base64 -- Encode and decode using Base64 representation

SYNOPSIS
     base64 [-h | -D] [-b count] [-i input_file] [-o output_file]

DESCRIPTION
     base64 encodes and decodes Base64 data, as specified in RFC 4648. With no
     options, base64 reads raw data from stdin and writes encoded data as a
     continuous block to stdout.

OPTIONS
     The following options are available:
     -b count
     --break=count        Insert line breaks every count characters. Default
                          is 0, which generates an unbroken stream.

     -D
     --decode             Decode incoming Base64 stream into binary data.

     -h
     --help               Print usage summary and exit.

     -i input_file
     --input=input_file   Read input from input_file.  Default is stdin; pass-
                          ing - also represents stdin.

     -o output_file
     --output=output_file
                          Write output to output_file.  Default is stdout;
                          passing - also represents stdout.

SEE ALSO
     openssl(1), wikipedia page <http://en.wikipedia.org/wiki/Base64>, RFC
     4648 <http://tools.ietf.org/html/rfc4648>

Mac OS X 10.7                  February 8, 2011                  Mac OS X 10.7

In my tests, base64 --break=1000000 does not actually work. It seems that there is a max line length of 1360 characters when break > 0. But this goes away when break=0.

The reason I didn't use wrap=0 with the GNU version was that this suppressed a closing newline. Without the newline at the end, it is easy to copy the following command prompt as part of the previous output. However, the macOS command seems to add the final newline.

@dhimmel
Copy link
Member Author

dhimmel commented Jul 3, 2019

There does seem to be one possible gotcha with base64. The character set for base64 is [A-Za-z0-9+/]. In addition, there is sometimes padding with = at the end. When I double click to select all for copying, the = padding is often omitted from selection.

We could use these terminal copy commands, but those add additional dependencies.

Copy link

@dongbohu dongbohu left a comment

Choose a reason for hiding this comment

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

I left a few comments.

SETUP.md Outdated Show resolved Hide resolved
SETUP.md Outdated Show resolved Hide resolved
ci/deploy.sh Show resolved Hide resolved
@slochower
Copy link
Collaborator

slochower commented Jul 3, 2019 via email

dhimmel added a commit to manubot/manubot.org that referenced this pull request Jul 3, 2019
dhimmel added a commit to manubot/manubot.org that referenced this pull request Jul 3, 2019
Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

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

I'm starting with general comments before doing a full review. The great news is that I was able to set up a new manuscript with Git Bash on Windows, so we can advertise that as an option now.

I want to think more about whether we can restructure any of the instructions to minimize switching between the browser and command line.

I doubt we'll be able to eliminate the browser-based steps entirely, but it may be possible to create the new repository from the command line:
https://stackoverflow.com/questions/2423777/is-it-possible-to-create-a-remote-repo-on-github-from-the-cli-without-opening-br/10325316#10325316

This command worked for me on Linux but hung on Git Bash:
curl -u 'agitter' https://api.github.com/user/repos -d '{"name":"manubot-test"}'

Based on the way I have Travis CI set up, the current instructions for enabling Travis CI for the new repository weren't directly applicable for me. I had to Manage repositories on GitHub
image
That button let me approve the new repository for the Travis CI GitHub App through GitHub. I expect this will be the trickiest step for new users who aren't familiar with continuous integration.

When adding the deploy key to GitHub, I didn't have to click "Add deploy key". The default view for the echoed URL was:
image

In the Finalize step, we now will have different unstaged changes.

$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)
        deleted:    content/02.delete-me.md
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)
        modified:   README.md
        modified:   ci/deploy.key.pub

The openssl syntax is different in Git Bash

$ openssl base64 -A -in=deploy.key
unknown option '-in=deploy.key'
options are
-in <file>     input file

The python and base64 versions worked.

ci/.gitignore Show resolved Hide resolved
SETUP.md Show resolved Hide resolved
SETUP.md Show resolved Hide resolved
@dhimmel
Copy link
Member Author

dhimmel commented Jul 4, 2019

it may be possible to create the new repository from the command line... This command worked for me on Linux but hung on Git Bash:

Let's save this for another pull request. How did authentication work with the curl-based-repo-creation command?

The openssl syntax is different in Git Bash

I'm hopeful aa17153 will fix that error.

The python and base64 versions worked.

I ended up removing those because I think any system with ssh-keygen will have openssl. Currently, instructions don't cat the base64 output to the terminal (thereby potentially having users open a file browser and text editor). I thought it would be least problematic for users to select all and copy the file contents. Copying from terminal could get confusing with trailing =s and no newline. Perhaps we should explore a cat command that adds a newline.

@agitter
Copy link
Member

agitter commented Jul 4, 2019

Let's save this for another pull request.

👍

How did authentication work with the curl-based-repo-creation command?

I was prompted for my GitHub password.

Enter host password for user 'agitter':

I'm hopeful aa17153 will fix that error.

Yes, I tested the new openssl call and it works on Git Bash.

@dhimmel
Copy link
Member Author

dhimmel commented Jul 8, 2019

PR is good to go on my end. Still haven't gotten feedback on the environment variable name (MANUBOT_SSH_PRIVATE_KEY, #243 (comment)). I think the new shell/bash is good (but my knowledge here is limited).

Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

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

I recommend changing line 6 of SETUP.md now that we've confirmed Windows setup works:

Setup is supported on Linux, macOS, and Windows through Git Bash.

or

Setup is supported on Linux, macOS, and Windows. Windows setup requires Git Bash or Windows Subsystem for Linux.

We haven't tested it with Windows Subsystem for Linux, but I expect that would work.

The suggestion #243 (comment) to disable xtrace using a subshell looks like it would simplify the syntax and improve readability.

Everything else looks good to me.

ci/README.md Outdated Show resolved Hide resolved
@dhimmel dhimmel merged commit 2a5e425 into manubot:master Jul 9, 2019
@dhimmel
Copy link
Member Author

dhimmel commented Jul 9, 2019

Merged commit 2a5e425 had a deploy failure:

++echo Agent pid 6798
Agent pid 6798
+set +o xtrace
Identity added: (stdin) ((stdin))
+git remote set-branches --add origin gh-pages output
+git fetch origin gh-pages:gh-pages output:output
Warning: Permanently added the RSA host key for IP address '140.82.113.3' to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Script failed with status 128

Will look into this. I did add MANUBOT_SSH_PRIVATE_KEY to the Travis CI settings, but let me double check the public key did not change.

Update: the fingerprints I'm getting locally match the public key on GitHub:

ssh-keygen -E md5 -lf deploy.key
4096 MD5:6f:3a:8a:79:2b:4a:eb:1d:5e:41:71:23:f2:75:d6:eb travis@travis-ci.com (RSA)

ssh-keygen -E md5 -lf deploy.key.pub
4096 MD5:6f:3a:8a:79:2b:4a:eb:1d:5e:41:71:23:f2:75:d6:eb travis@travis-ci.com (RSA)

Update: I generated a new SSH key and added the public/private keys to GitHub/Travis. Deployment succeeded. Not sure what was wrong with the old key-pair.

Perhaps we should add a command like the following to setup so users can test if things are configured correctly (to some degree):

>>> ssh -T -o "IdentitiesOnly=yes" -i deploy.key git@github.com
Hi manubot/rootstock! You've successfully authenticated, but GitHub does not provide shell access.

dhimmel added a commit that referenced this pull request Jul 9, 2019
This build is based on
2a5e425.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/manubot/rootstock/builds/118484775
https://travis-ci.com/manubot/rootstock/jobs/214507138

[ci skip]

The full commit message that triggered this build is copied below:

Travis CI: use settings env var to store SSH private key

Merges #243
Closes #91
Closes #246

Switch to a Travis CI settings environment variable to store the
SSH private key for GitHub deployment. Setup no longer requires
the travis ruby gem, whose `travis encrypt` subcommand was 
incompatible with Windows. Setup is now supported on Windows
systems with a Unix/Linux/POSIX-compliant shell with the proper
dependencies, such as ssh-keygen & openssl.

The SSH private key is stored in MANUBOT_SSH_PRIVATE_KEY,
after being base64 encoded to remove newline characters.

Remove files for the legacy encrypted file method. Deprecate
the encrypted file decryption in deploy.sh. In the future, we may
only support private key specification via MANUBOT_SSH_PRIVATE_KEY.
dhimmel added a commit that referenced this pull request Jul 9, 2019
This build is based on
2a5e425.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/manubot/rootstock/builds/118484775
https://travis-ci.com/manubot/rootstock/jobs/214507138

[ci skip]

The full commit message that triggered this build is copied below:

Travis CI: use settings env var to store SSH private key

Merges #243
Closes #91
Closes #246

Switch to a Travis CI settings environment variable to store the
SSH private key for GitHub deployment. Setup no longer requires
the travis ruby gem, whose `travis encrypt` subcommand was 
incompatible with Windows. Setup is now supported on Windows
systems with a Unix/Linux/POSIX-compliant shell with the proper
dependencies, such as ssh-keygen & openssl.

The SSH private key is stored in MANUBOT_SSH_PRIVATE_KEY,
after being base64 encoded to remove newline characters.

Remove files for the legacy encrypted file method. Deprecate
the encrypted file decryption in deploy.sh. In the future, we may
only support private key specification via MANUBOT_SSH_PRIVATE_KEY.
adebali pushed a commit to CompGenomeLab/lemur-manuscript-archive that referenced this pull request Mar 4, 2020
Merges manubot/rootstock#243
Closes manubot/rootstock#91
Closes manubot/rootstock#246

Switch to a Travis CI settings environment variable to store the
SSH private key for GitHub deployment. Setup no longer requires
the travis ruby gem, whose `travis encrypt` subcommand was 
incompatible with Windows. Setup is now supported on Windows
systems with a Unix/Linux/POSIX-compliant shell with the proper
dependencies, such as ssh-keygen & openssl.

The SSH private key is stored in MANUBOT_SSH_PRIVATE_KEY,
after being base64 encoded to remove newline characters.

Remove files for the legacy encrypted file method. Deprecate
the encrypted file decryption in deploy.sh. In the future, we may
only support private key specification via MANUBOT_SSH_PRIVATE_KEY.
ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this pull request Aug 6, 2024
Merges manubot/rootstock#243
Closes manubot/rootstock#91
Closes manubot/rootstock#246

Switch to a Travis CI settings environment variable to store the
SSH private key for GitHub deployment. Setup no longer requires
the travis ruby gem, whose `travis encrypt` subcommand was 
incompatible with Windows. Setup is now supported on Windows
systems with a Unix/Linux/POSIX-compliant shell with the proper
dependencies, such as ssh-keygen & openssl.

The SSH private key is stored in MANUBOT_SSH_PRIVATE_KEY,
after being base64 encoded to remove newline characters.

Remove files for the legacy encrypted file method. Deprecate
the encrypted file decryption in deploy.sh. In the future, we may
only support private key specification via MANUBOT_SSH_PRIVATE_KEY.
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.

travis.com authentification problem Deploy with windows generated keys fails
5 participants