From 9ae97c341cd4dee04ade0a7dec6b893602116ad6 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Tue, 25 Oct 2016 16:57:40 -0600 Subject: [PATCH] fix(helm): read passphrase from prompt This prompts the user to enter a passphrase if the given PGP key is encrypted. Closes #1447 --- cmd/helm/package.go | 14 ++++++ docs/provenance.md | 5 +- pkg/provenance/sign.go | 45 ++++++++++++++++++ pkg/provenance/sign_test.go | 35 ++++++++++++++ .../testdata/helm-password-key.secret | Bin 0 -> 2562 bytes 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 pkg/provenance/testdata/helm-password-key.secret diff --git a/cmd/helm/package.go b/cmd/helm/package.go index d99309c4a..f9cf4cdbc 100644 --- a/cmd/helm/package.go +++ b/cmd/helm/package.go @@ -23,8 +23,10 @@ import ( "io/ioutil" "os" "path/filepath" + "syscall" "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" "k8s.io/helm/cmd/helm/helmpath" "k8s.io/helm/pkg/chartutil" @@ -145,6 +147,10 @@ func (p *packageCmd) clearsign(filename string) error { return err } + if err := signer.DecryptKey(promptUser); err != nil { + return err + } + sig, err := signer.ClearSign(filename) if err != nil { return err @@ -156,3 +162,11 @@ func (p *packageCmd) clearsign(filename string) error { return ioutil.WriteFile(filename+".prov", []byte(sig), 0755) } + +// promptUser implements provenance.PassphraseFetcher +func promptUser(name string) ([]byte, error) { + fmt.Printf("Password for key %q > ", name) + pw, err := terminal.ReadPassword(int(syscall.Stdin)) + fmt.Println() + return []byte(pw), err +} diff --git a/docs/provenance.md b/docs/provenance.md index e427c533a..6a6faac36 100644 --- a/docs/provenance.md +++ b/docs/provenance.md @@ -27,11 +27,14 @@ This section describes a potential workflow for using provenance data effectivel Prerequisites: -- A valid, passphrase-less PGP keypair in a binary (not ASCII-armored) format +- A valid PGP keypair in a binary (not ASCII-armored) format - The `helm` command line tool - GnuPG command line tools (optional) - Keybase command line tools (optional) +**NOTE:** If your PGP private key has a passphrase, you will be prompted to enter +that passphrase for any commands that support the `--sign` option. + Creating a new chart is the same as before: ``` diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index 90b14cdc5..480932648 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -144,10 +144,55 @@ func NewFromKeyring(keyringfile, id string) (*Signatory, error) { if vague { return s, fmt.Errorf("more than one key contain the id %q", id) } + s.Entity = candidate return s, nil } +// PassphraseFetcher returns a passphrase for decrypting keys. +// +// This is used as a callback to read a passphrase from some other location. The +// given name is the Name field on the key, typically of the form: +// +// USER_NAME (COMMENT) +type PassphraseFetcher func(name string) ([]byte, error) + +// DecryptKey decrypts a private key in the Signatory. +// +// If the key is not encrypted, this will return without error. +// +// If the key does not exist, this will return an error. +// +// If the key exists, but cannot be unlocked with the passphrase returned by +// the PassphraseFetcher, this will return an error. +// +// If the key is successfully unlocked, it will return nil. +func (s *Signatory) DecryptKey(fn PassphraseFetcher) error { + if s.Entity == nil || s.Entity.PrivateKey == nil { + return errors.New("private key not found") + } + + // Nothing else to do if key is not encrypted. + if !s.Entity.PrivateKey.Encrypted { + return nil + } + + fname := "Unknown" + for i := range s.Entity.Identities { + if i != "" { + fname = i + break + } + } + + p, err := fn(fname) + if err != nil { + return err + } + + return s.Entity.PrivateKey.Decrypt(p) +} + // ClearSign signs a chart with the given key. // // This takes the path to a chart archive file and a key, and it returns a clear signature. diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 747a9376a..388941deb 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -32,6 +32,9 @@ const ( // phrase. Use `gpg --export-secret-keys helm-test` to export the secret. testKeyfile = "testdata/helm-test-key.secret" + // testPasswordKeyFile is a keyfile with a password. + testPasswordKeyfile = "testdata/helm-password-key.secret" + // testPubfile is the public key file. // Use `gpg --export helm-test` to export the public key. testPubfile = "testdata/helm-test-key.pub" @@ -39,6 +42,8 @@ const ( // Generated name for the PGP key in testKeyFile. testKeyName = `Helm Testing (This key should only be used for testing. DO NOT TRUST.) ` + testPasswordKeyName = `password key (fake) ` + testChartfile = "testdata/hashtest-1.2.3.tgz" // testSigBlock points to a signature generated by an external tool. @@ -177,6 +182,36 @@ func TestDigestFile(t *testing.T) { } } +func TestDecryptKey(t *testing.T) { + k, err := NewFromKeyring(testPasswordKeyfile, testPasswordKeyName) + if err != nil { + t.Fatal(err) + } + + if !k.Entity.PrivateKey.Encrypted { + t.Fatal("Key is not encrypted") + } + + // We give this a simple callback that returns the password. + if err := k.DecryptKey(func(s string) ([]byte, error) { + return []byte("secret"), nil + }); err != nil { + t.Fatal(err) + } + + // Re-read the key (since we already unlocked it) + k, err = NewFromKeyring(testPasswordKeyfile, testPasswordKeyName) + if err != nil { + t.Fatal(err) + } + // Now we give it a bogus password. + if err := k.DecryptKey(func(s string) ([]byte, error) { + return []byte("secrets_and_lies"), nil + }); err == nil { + t.Fatal("Expected an error when giving a bogus passphrase") + } +} + func TestClearSign(t *testing.T) { signer, err := NewFromFiles(testKeyfile, testPubfile) if err != nil { diff --git a/pkg/provenance/testdata/helm-password-key.secret b/pkg/provenance/testdata/helm-password-key.secret new file mode 100644 index 0000000000000000000000000000000000000000..03c8aa5833f0f2a5627a05c406ea7a2ff7aa4899 GIT binary patch literal 2562 zcmV+d3jOt!1HJ@U58Flo2ms3+hB6`sr-UH+X*^v%vLpo1Tzw7Tro;0b9MG+uG15G_ ziPcQ9>=H3V8=T1;oo-SPr+@$c$lCNz)uj=h_{vmYR}R79|B^49bnLhz<~;GETy54> zJ{Un-NN{0L)0xeL>XtE8lE)K1V*j#;5_-R{8()WB@?m@{1uqs{rEWsC&J zRmZ@RDQS&Ghr<{!euy9->k0{4u9e#Z*mqVT)qv`$>XLtkE*e&$*p|+O-~&yXl=?-` z=&fw1Zbf?ypPu20Z01o%laoFJ z|4X8nKT%m6;wfosI&+%}1#*mKd@ol!tS{p)EswcxGU z4FklxTTba+1dg@RKBy3=To~w|6WkMtOLZ$Km^_zTW~kgVINa=Lkb=>L;l-xN#2x!F z0SImLEhGjgcoB8Mw&hCh>!+Dm*){uH4;+*RAZxR_lbT8MY|u@nM4zjOgyvtp|9>pL zC$XT#l0!dRic@j(Lz!94Z3F41nd?hpBn=c_0%I^JR?K@Vd0%sadsKYnXh^=e-&c5F z@*>Gx?1e7*X!sNtUsBY(Zxr>QT`VI{h0zKn07QgU!PDpvh8j#o15LD`!)Q083{cju zT$_X4Red8llclG(;8L0hbuA&V$|WzX`V%!4?8$pcN|K}qh*0#QkyTP=Ni!_nBgX}n zWj8VR7CE0)4G%nrOh^r=RX}7z3@U%F*?M%Pa$LqPu$469l3prHFyIA+xIP%-({hGw z>a^H^#xTz(W}D6#T2x!_p$U%{c?9=R9{FK@-f;T{04VbCA4-AtjX#hCY-#t^xoW+9 zEG?-^a<8^X;RWr$X=YTe#u<@c!F-{3T@cR(fdnQV7ISMq_hlpyLfoe@DKN!!5zf`0 zc8`Edp`Rhdr0!Ck6^mB>G9QX`(Jq%rQikyfBG=|QjbnJmL1SJ`p?P$XN5{od+MWO0 zgAVcrhHbdO(v4Utuobm-qo_E|4te9{^(V^T%Oy;Q@QI!6uoF1cIiLaE_Xhal)62Du z_OWO($sROUaT7Hmo)6^Aavj`H9n#A?w8W1@~F@iWVlLV5~Vv?6d}b8~lZa%3QDWqBYdW?^e( zDIh#%VQXbTXk~0|E^}x;i2*kR69EbUAq4_h58Flp8v_Lk2?z%R1r-Vj2nz)k0s{d6 z0v-VZ7k~f?2@raT7BR66qOsaZ2mUs<*aiSz2B-<))eW~`+bQkAx&5TEVV`A8`m`{Y zSmSfX*KF6eml(Vp_=4AJ7mraMJnJsafoZ0D6zOu-HyZPmM!P&SNfg#jtW4kf$xE>6 zqT-b8-_~w%jH&Qww@M=rdeW|uE<wy39o%k%a(^us7T@st#`i(Ui z*xr4R90Psj`>>-uZL>1A714Akg!NeNbZ%lEa;p6kukRzIrPiGogERBnmxSS7wP_#$ z#C@BIr*xQB4QYJUg#qY4D}$rVaR&9|Kcz64kg+eQHh0KBRgK#xZ0PuECJBt1~f7q^(%qm)_k zxuW~o^7>9uDuSKZA;I=0#N?Xvkt3JK+wF49bIP6ObEo*P!b)?9e_hX%Tn?df-p)&< zqT<7SGb=Z?w5s^#CDnV@H07Vty`YfU(I7aG-K)3I zKxt)1TxdS$uN+_NjQkqHY3>hiKqE7&pkZ8d z3Y|qYX72}77>9M2ald@7Mo9(+G{X>dA~;-5roNw+lhtTi0shrsNw{QRNOH@%0x~#E zvZt$Z;Eq}3GX$6+`5(Dhr{f8dW6E|T;}o7J_0i$vDQ2<@8c-lv>wx-Y59`VrJ#Dz^ zl5#Wqj)1qwzDxnOL!;ELc_0ZQC)a*Xx>#W?9Sf`Q9TAJpDGS~u*rF{CEzAtw@0GQ< zk7f4hM^y@22|b3JKJZMz&x}nlSm+j|A6#V_#dip;@C#3GXP`n?+5%U#=N~%Bv`KfU zHU^eE{q&<*x)Brj^O#ULldk_LSbpv%L_Q0DwLSDc^qzxGP-DVc>weL@slnHJ0jW^l z17vXJjW_n-YovP(B_N$cwMaNx7*)$Jgsz??{*qg8?p-5uL`Yii@&msQh=3MsE$=hc zna`a~Me6aLP3~8@&FRYovboD4qQ))eSIh8WkJzUL#aG2h*=8y82UetPl@W#MjB2a3 zm)$tRSTuWb5i%+PA*N1%x{@PTRoF*KLZz>D6zBWtCEzq+jI=m%kslwY@0v2d;`M{5 z?7Db6I+k({A92bs<23ojz&nCJ1aUv6N0%vFOBZ{l4DOSOLCbp@O-2>n5i?Wtz;&e+ za+Ua)Ec}0ba}P>@`33`wx_#qp^LIrbgGr{zD{)U7V4?m+`9@n0?|!x|CUXHh9yzf& zA6B2uSC$CcJj6ch_qJyttDQ3~@TYOCd!{c24TnHG8-)Bxi&nROOl`D@0Urby0SW*K z1p-(P+eQK#3;+rV5PFFgF|iGzv0o$y{xFcdf`-DHXxm;#2MuT(f>rhV#>PO-Z5>|W zG)iO zHQIznJ)0H>Pa%p>s9Xm{Q;3{J-$KTX+UI;!&+3wPwy|!!=Nf3GR(3E6CVJ)}N-It> zR8W`wSeH{d12&%hE367sJQ7P@m8a+mNK zWH;!lmfFOUqlP7ucV#NaNvQz;9#qeU7V>yl3mQtp%j+q0C%{=t(~sL()Xqu};Spe} YR26Mae070tz>^S098h1bS^5