Merge lp:~chipaca/snappy/auth into lp:~snappy-dev/snappy/snappy-moved-to-github

Proposed by John Lenton
Status: Rejected
Rejected by: John Lenton
Proposed branch: lp:~chipaca/snappy/auth
Merge into: lp:~snappy-dev/snappy/snappy-moved-to-github
Prerequisite: lp:~chipaca/snappy/merged-list
Diff against target: 962 lines (+784/-41)
11 files modified
daemon/api.go (+24/-17)
daemon/basicauth_1.3.go (+54/-0)
daemon/basicauth_1.4.go (+30/-0)
daemon/crypt/bcrypt.go (+38/-0)
daemon/crypt/crypt.go (+120/-0)
daemon/crypt/crypt_test.go (+248/-0)
daemon/crypt/scrypt.go (+78/-0)
daemon/crypt/sha512crypt.go (+120/-0)
daemon/daemon.go (+23/-1)
daemon/daemon_test.go (+48/-23)
daemon/response.go (+1/-0)
To merge this branch: bzr merge lp:~chipaca/snappy/auth
Reviewer Review Type Date Requested Status
Tyler Hicks (community) Needs Fixing
Sergio Schvezov Approve
Review via email: mp+273684@code.launchpad.net

This proposal supersedes a proposal from 2015-10-07.

Commit message

Require authentication for non-guest methods.

Description of the change

Only GET can be used by non-authenticated, and then only for commands flagged with GuestOK. This means that we need some way to bootsrap, which'll come in a followup branch. We also want to change the credentials via the same REST API, which is yet another followup.

I'll repeat it in bigger letters in case you missed it: THIS WILL LOCK YOU OUT OF THE REST API.

This'll probably be breaking integration tests; need to check with the guys.

To post a comment you must log in.
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

This code looks really neat and tidy, kudos!

I added a few irrelevant comments here and there (tl;rl; they are all irrelevant).

The only thing that I might consider trying is forcing garbage collection and getting rid of the 'password' from memory as soon as we have a hash, unless I missed that and is already taken care of.

Revision history for this message
John Lenton (chipaca) wrote :

re gc: probably not possible without major work, as the password will be clear(ish) in the http request headers.

If that's a concern, we should switch to digest auth.

Revision history for this message
Sergio Schvezov (sergiusens) :
review: Approve
lp:~chipaca/snappy/auth updated
752. By John Lenton

annotated 1.3's basicAuth with return variable names

Revision history for this message
Tyler Hicks (tyhicks) wrote :

I have a few inline comments and then some higher level questions below.

1) Why implement scrypt, bcrypt, and sha512crypt all right out of the gate? The design should definitely incorporate algorithm agility so that we can move from scrypt to something else down the road if scrypt is found to be flawed. However, I found it odd that we're bringing the complexity of scrypt plus two "lesser" algorithms in from the start.
 - I've already reviewed the bcrypt implementation and it looks good
 - I still need to review the scrypt implementation
 - I'm ignoring sha512crypt until we've determined that it is needed.

2) This change creates a single username/password combo, which is different than the username/password combo for PAM logins, to be used for all sensitive requests made to snapd. How will that work in the future in a multi-user scenario? All users of the system will need to share the same snapd login information? Would it make sense for snapd to talk to PAM and authenticate these requests in that way?

Note that I haven't yet had a chance to review the tests.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

The username handling doesn't look correct to me. AuthOK() calls crypt.Check([]byte(user + pass)) but crypt.Check() just treats the entire thing as the password.

review: Needs Fixing
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Have you benchmarked scrypt on the BBB or Raspberry Pi 2? I'm curious if there are significant delays when checking the password. I think scrypt is generally the best choice from a security standpoint but it may take considerable time to authenticate these REST API calls.

Revision history for this message
John Lenton (chipaca) wrote :

Wrt colons in username, “a user-id containing a colon character is invalid”, says the rfc about basic auth. Also I don't really care, because

Wrt password vs user+pass, i concatenate user+password from basic auth to create the "password" that crypt needs. As there aren't users, this seemed ok (the alternative being to ask for a username and discard it).

The multiple choice of algorithms are precisely because I benchmarked them on the bbb. sha512crypt is the only one that gives sub-second performance; scrypt takes about 30 seconds.

Revision history for this message
John Lenton (chipaca) wrote :

http://pastebin.ubuntu.com/12697607/
Note it says N but it's actually log2(N). That is, 14 means an N of 16k.

That run was stapping at over 5s; I did a run with N of 1<<14 to check, and it was 23 seconds. Not 30, sorry -- even on that i7 14 is rather slow for a REST API (and on my slightly older laptop i7 it's about 2× that).

Revision history for this message
Tyler Hicks (tyhicks) wrote :

> The multiple choice of algorithms are precisely because I benchmarked them on
> the bbb. sha512crypt is the only one that gives sub-second performance; scrypt
> takes about 30 seconds.

Then is scrypt even an option? This MP has 'var DefaultAlg = SCRYPT' and crypt.Write() unconditionally uses SCRYPT.

Revision history for this message
John Lenton (chipaca) wrote :

>
> Then is scrypt even an option? This MP has 'var DefaultAlg = SCRYPT' and
> crypt.Write() unconditionally uses SCRYPT.

in a later branch I intend to let an oem snap override the default (so on arm you'd tweak it), and it's an scrypt with N=1<<10 so it's ok on even not-so-zippy intels.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

> Wrt colons in username, “a user-id containing a colon character is invalid”,
> says the rfc about basic auth. Also I don't really care, because
>
> Wrt password vs user+pass, i concatenate user+password from basic auth to
> create the "password" that crypt needs. As there aren't users, this seemed ok
> (the alternative being to ask for a username and discard it).

Ok, so that again locks us into a single username and password combo for authenticating to snapd. Maybe that's fine but it would be good for one of the Snappy architects to comment here regarding whether or not multi-user support will be important in the near to medium term for snapd and Snappy, in general.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Can you add a comment to the code where the socket is created to make it clear that if this API is ever exposed over the network, HTTPS *must* used?

review: Needs Fixing
Revision history for this message
Tyler Hicks (tyhicks) wrote :

I need to start my weekend but I still need to finish looking at the scrypt implementation and research the N, R, and P values chosen. In the meantime, can you reach out to the architect team and get them to comment on how important multi-user support is?

Revision history for this message
Sergio Schvezov (sergiusens) wrote :
Revision history for this message
John Lenton (chipaca) wrote :

We're doing the other thing, switching to named unix sockets. Thank you and sorry to waste your time, Tyler.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

It wasn't a waste of time, at all! I'm glad we got the right design in place for local auth and this will allow me to start thinking about what will be needed for remote auth down the line. Thanks again, John!

Unmerged revisions

752. By John Lenton

annotated 1.3's basicAuth with return variable names

751. By John Lenton

fix for 1.3 not having basicAuth methods on http.Request. Also caught a nasty bug wrt null-termination

750. By John Lenton

restrict REST API calls w/auth

749. By John Lenton

Merged merged-list into auth.

748. By John Lenton

Merged merged-list into auth.

747. By John Lenton

Merged merged-list into auth.

746. By John Lenton

Merged merged-list into auth.

745. By John Lenton

first pass at auth

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'daemon/api.go'
2--- daemon/api.go 2015-10-08 12:50:11 +0000
3+++ daemon/api.go 2015-10-09 13:33:12 +0000
4@@ -57,35 +57,41 @@
5
6 var (
7 rootCmd = &Command{
8- Path: "/",
9- GET: SyncResponse([]string{"/1.0"}).Self,
10+ GuestOK: true,
11+ Path: "/",
12+ GET: SyncResponse([]string{"/1.0"}).Self,
13 }
14
15 v1Cmd = &Command{
16- Path: "/1.0",
17- GET: v1Get,
18+ GuestOK: true,
19+ Path: "/1.0",
20+ GET: v1Get,
21 }
22
23 metaIconCmd = &Command{
24- Path: "/1.0/icons/{icon}",
25- GET: metaIconGet,
26+ GuestOK: true,
27+ Path: "/1.0/icons/{icon}",
28+ GET: metaIconGet,
29 }
30
31 appIconCmd = &Command{
32- Path: "/1.0/icons/{name}.{origin}/icon",
33- GET: appIconGet,
34+ GuestOK: true,
35+ Path: "/1.0/icons/{name}.{origin}/icon",
36+ GET: appIconGet,
37 }
38
39 packagesCmd = &Command{
40- Path: "/1.0/packages",
41- GET: getPackagesInfo,
42- POST: sideloadPackage,
43+ GuestOK: true,
44+ Path: "/1.0/packages",
45+ GET: getPackagesInfo,
46+ POST: sideloadPackage,
47 }
48
49 packageCmd = &Command{
50- Path: "/1.0/packages/{name}.{origin}",
51- GET: getPackageInfo,
52- POST: postPackage,
53+ GuestOK: true,
54+ Path: "/1.0/packages/{name}.{origin}",
55+ GET: getPackageInfo,
56+ POST: postPackage,
57 }
58
59 packageConfigCmd = &Command{
60@@ -95,9 +101,10 @@
61 }
62
63 packageSvcsCmd = &Command{
64- Path: "/1.0/packages/{name}.{origin}/services",
65- GET: packageService,
66- PUT: packageService,
67+ GuestOK: true,
68+ Path: "/1.0/packages/{name}.{origin}/services",
69+ GET: packageService,
70+ PUT: packageService,
71 }
72
73 packageSvcCmd = &Command{
74
75=== added file 'daemon/basicauth_1.3.go'
76--- daemon/basicauth_1.3.go 1970-01-01 00:00:00 +0000
77+++ daemon/basicauth_1.3.go 2015-10-09 13:33:12 +0000
78@@ -0,0 +1,54 @@
79+// -*- Mode: Go; indent-tabs-mode: t -*-
80+// +build !go1.4
81+
82+/*
83+ * Copyright (C) 2014-2015 Canonical Ltd
84+ *
85+ * This program is free software: you can redistribute it and/or modify
86+ * it under the terms of the GNU General Public License version 3 as
87+ * published by the Free Software Foundation.
88+ *
89+ * This program is distributed in the hope that it will be useful,
90+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
91+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
92+ * GNU General Public License for more details.
93+ *
94+ * You should have received a copy of the GNU General Public License
95+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
96+ *
97+ */
98+
99+package daemon
100+
101+import (
102+ "bytes"
103+ "encoding/base64"
104+ "net/http"
105+ "strings"
106+)
107+
108+const authScheme = "Basic " // extra space because cheating
109+
110+func basicAuth(req *http.Request) (username string, password string, ok bool) {
111+ auth := req.Header.Get("Authorization")
112+ if !strings.HasPrefix(auth, authScheme) {
113+ return "", "", false
114+ }
115+
116+ up, err := base64.StdEncoding.DecodeString(auth[len(authScheme):])
117+ if err != nil {
118+ return "", "", false
119+ }
120+
121+ idx := bytes.IndexByte(up, ':')
122+ if idx < 0 {
123+ return "", "", false
124+ }
125+
126+ return string(up[:idx]), string(up[idx+1:]), true
127+}
128+
129+func setBasicAuth(req *http.Request, username, password string) {
130+ up := base64.StdEncoding.EncodeToString([]byte(username + ":" + password))
131+ req.Header.Set("Authorization", authScheme+up)
132+}
133
134=== added file 'daemon/basicauth_1.4.go'
135--- daemon/basicauth_1.4.go 1970-01-01 00:00:00 +0000
136+++ daemon/basicauth_1.4.go 2015-10-09 13:33:12 +0000
137@@ -0,0 +1,30 @@
138+// -*- Mode: Go; indent-tabs-mode: t -*-
139+// +build go1.4
140+
141+/*
142+ * Copyright (C) 2014-2015 Canonical Ltd
143+ *
144+ * This program is free software: you can redistribute it and/or modify
145+ * it under the terms of the GNU General Public License version 3 as
146+ * published by the Free Software Foundation.
147+ *
148+ * This program is distributed in the hope that it will be useful,
149+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
150+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
151+ * GNU General Public License for more details.
152+ *
153+ * You should have received a copy of the GNU General Public License
154+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
155+ *
156+ */
157+
158+package daemon
159+
160+import (
161+ "net/http"
162+)
163+
164+var (
165+ basicAuth = (*http.Request).BasicAuth
166+ setBasicAuth = (*http.Request).SetBasicAuth
167+)
168
169=== added directory 'daemon/crypt'
170=== added file 'daemon/crypt/bcrypt.go'
171--- daemon/crypt/bcrypt.go 1970-01-01 00:00:00 +0000
172+++ daemon/crypt/bcrypt.go 2015-10-09 13:33:12 +0000
173@@ -0,0 +1,38 @@
174+// -*- Mode: Go; indent-tabs-mode: t -*-
175+
176+/*
177+ * Copyright (C) 2014-2015 Canonical Ltd
178+ *
179+ * This program is free software: you can redistribute it and/or modify
180+ * it under the terms of the GNU General Public License version 3 as
181+ * published by the Free Software Foundation.
182+ *
183+ * This program is distributed in the hope that it will be useful,
184+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
185+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
186+ * GNU General Public License for more details.
187+ *
188+ * You should have received a copy of the GNU General Public License
189+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
190+ *
191+ */
192+
193+package crypt
194+
195+import (
196+ "golang.org/x/crypto/bcrypt"
197+)
198+
199+var bcryptAlg = cryptAlg{
200+ gen: func(password []byte) ([]byte, error) {
201+ hash, err := bcrypt.GenerateFromPassword(password, -1)
202+ if err != nil {
203+ return nil, err
204+ }
205+
206+ return hash, nil
207+ },
208+ cmp: func(hashedPassword, password []byte) error {
209+ return bcrypt.CompareHashAndPassword(hashedPassword, password)
210+ },
211+}
212
213=== added file 'daemon/crypt/crypt.go'
214--- daemon/crypt/crypt.go 1970-01-01 00:00:00 +0000
215+++ daemon/crypt/crypt.go 2015-10-09 13:33:12 +0000
216@@ -0,0 +1,120 @@
217+// -*- Mode: Go; indent-tabs-mode: t -*-
218+
219+/*
220+ * Copyright (C) 2014-2015 Canonical Ltd
221+ *
222+ * This program is free software: you can redistribute it and/or modify
223+ * it under the terms of the GNU General Public License version 3 as
224+ * published by the Free Software Foundation.
225+ *
226+ * This program is distributed in the hope that it will be useful,
227+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
228+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
229+ * GNU General Public License for more details.
230+ *
231+ * You should have received a copy of the GNU General Public License
232+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
233+ *
234+ */
235+
236+package crypt
237+
238+import (
239+ "bytes"
240+ "errors"
241+ "io"
242+ "os"
243+ "path/filepath"
244+
245+ "launchpad.net/snappy/dirs"
246+ "launchpad.net/snappy/helpers"
247+)
248+
249+type cryptAlg struct {
250+ gen func(password []byte) ([]byte, error)
251+ cmp func(hashed, password []byte) error
252+}
253+
254+// the known crypt algorithms
255+//
256+// xxx is algorithm the right name even
257+const (
258+ SHA512 = "sha512"
259+ SCRYPT = "scrypt"
260+ BCRYPT = "bcrypt"
261+)
262+
263+// the default algorithm
264+var DefaultAlg = SCRYPT
265+
266+var cryptAlgs = map[string]cryptAlg{
267+ SCRYPT: scryptAlg,
268+ BCRYPT: bcryptAlg,
269+ SHA512: sha512cryptAlg,
270+}
271+
272+var (
273+ // ErrBadCreds is a generic "bad password" error
274+ ErrBadCreds = errors.New("bad credentials")
275+)
276+
277+const (
278+ maxSecretLen = 1024 // upper bound of the length of a secret
279+ maxPrefixLen = 8 // upper bound of the prefix length (e.g. "scrypt:")
280+
281+ minpwlen = 8
282+)
283+
284+func pwFile() string {
285+ return filepath.Join(dirs.GlobalRootDir, dirs.SnappyDir, ".snapd_creds")
286+}
287+
288+// Write the given password out to the file, crypting it first
289+func Write(password []byte) error {
290+ // TODO: cracklib
291+ if len(password) < minpwlen {
292+ return ErrBadCreds
293+ }
294+
295+ hash, err := cryptAlgs[DefaultAlg].gen(password)
296+ if err != nil {
297+ return err
298+ }
299+
300+ buf := append([]byte(DefaultAlg+":"), hash...)
301+ if err := helpers.AtomicWriteFile(pwFile(), buf, 0600); err != nil {
302+ return err
303+ }
304+
305+ return nil
306+}
307+
308+// Check whether this password and the one on file match.
309+func Check(password []byte) bool {
310+ f, err := os.Open(pwFile())
311+ if err != nil {
312+ return false
313+ }
314+
315+ buf := make([]byte, maxPrefixLen+maxSecretLen)
316+ n, err := f.ReadAt(buf, 0)
317+ if err != io.EOF {
318+ return false
319+ }
320+
321+ if n < maxPrefixLen {
322+ return false
323+ }
324+
325+ idx := bytes.IndexByte(buf[:maxPrefixLen], ':')
326+ if idx < 1 {
327+ return false
328+ }
329+
330+ alg, ok := cryptAlgs[string(buf[:idx])]
331+ if !ok {
332+ return false
333+ }
334+
335+ return alg.cmp(buf[idx+1:n], password) == nil
336+}
337
338=== added file 'daemon/crypt/crypt_test.go'
339--- daemon/crypt/crypt_test.go 1970-01-01 00:00:00 +0000
340+++ daemon/crypt/crypt_test.go 2015-10-09 13:33:12 +0000
341@@ -0,0 +1,248 @@
342+// -*- Mode: Go; indent-tabs-mode: t -*-
343+
344+/*
345+ * Copyright (C) 2014-2015 Canonical Ltd
346+ *
347+ * This program is free software: you can redistribute it and/or modify
348+ * it under the terms of the GNU General Public License version 3 as
349+ * published by the Free Software Foundation.
350+ *
351+ * This program is distributed in the hope that it will be useful,
352+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
353+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
354+ * GNU General Public License for more details.
355+ *
356+ * You should have received a copy of the GNU General Public License
357+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
358+ *
359+ */
360+
361+package crypt
362+
363+import (
364+ "bytes"
365+ "crypto/rand"
366+ "errors"
367+ "io"
368+ "os"
369+ "path/filepath"
370+ "testing"
371+
372+ "gopkg.in/check.v1"
373+
374+ "launchpad.net/snappy/dirs"
375+)
376+
377+// Hook up check.v1 into the "go test" runner
378+func Test(t *testing.T) { check.TestingT(t) }
379+
380+type cryptSuite struct {
381+ d string
382+ oldReader io.Reader
383+ oldN int
384+ oldR int
385+ oldP int
386+}
387+
388+var _ = check.Suite(&cryptSuite{})
389+
390+var errBadStuff = errors.New("fail")
391+
392+type totallyRandom struct {
393+ err error
394+}
395+
396+func (t *totallyRandom) Read(p []byte) (int, error) {
397+ if t.err != nil {
398+ return 0, t.err
399+ }
400+ if len(p) < 1 {
401+ return 0, nil
402+ }
403+ p[0] = 9 // http://dilbert.com/strip/2001-10-25 -- wait, 2001? Ow.
404+ return 1, nil
405+}
406+
407+func (s *cryptSuite) SetUpTest(c *check.C) {
408+ s.d = c.MkDir()
409+ dirs.SetRootDir(s.d)
410+ c.Check(os.MkdirAll(filepath.Join(dirs.GlobalRootDir, dirs.SnappyDir), 0755), check.IsNil)
411+
412+ s.oldN = scryptN
413+ s.oldR = scryptR
414+ s.oldP = scryptP
415+ s.oldReader = rand.Reader
416+ rand.Reader = &totallyRandom{}
417+ scryptN = 2
418+ scryptR = 1
419+ scryptP = 1
420+}
421+
422+func (s *cryptSuite) TearDownTest(c *check.C) {
423+ rand.Reader = s.oldReader
424+ scryptN = s.oldN
425+ scryptR = s.oldR
426+ scryptP = s.oldP
427+}
428+
429+func (s *cryptSuite) TestCryptAlgs(c *check.C) {
430+
431+ password := []byte("password")
432+ // know any interesting password to try? call 0800-CHIPACA
433+ for k, v := range cryptAlgs {
434+ hash, err := v.gen(password)
435+ c.Assert(err, check.IsNil, check.Commentf(k))
436+ c.Assert(hash, check.NotNil, check.Commentf(k))
437+
438+ c.Check(v.cmp(hash, password), check.IsNil, check.Commentf(k))
439+ c.Check(v.cmp(hash, []byte("what")), check.NotNil, check.Commentf(k))
440+ }
441+}
442+
443+func (s *cryptSuite) TestCryptAlgsFailure(c *check.C) {
444+ defer func() {
445+ rand.Reader.(*totallyRandom).err = nil
446+ }()
447+
448+ password := []byte("password")
449+ for k, v := range cryptAlgs {
450+ rand.Reader.(*totallyRandom).err = nil
451+
452+ goodHash, err := v.gen(password)
453+ c.Check(err, check.IsNil, check.Commentf(k))
454+ c.Check(goodHash, check.NotNil, check.Commentf(k))
455+ c.Check(v.cmp(goodHash, password), check.IsNil, check.Commentf(k))
456+
457+ rand.Reader.(*totallyRandom).err = errBadStuff
458+
459+ hash, err := v.gen(password)
460+ c.Check(err, check.NotNil, check.Commentf(k))
461+ c.Check(hash, check.IsNil, check.Commentf(k))
462+ }
463+}
464+
465+func (s *cryptSuite) TestScryptFailures(c *check.C) {
466+ pass := []byte("password")
467+ c.Check(scryptAlg.cmp([]byte("w"), pass), check.ErrorMatches, "illegal ascii85 data .*")
468+
469+ hash, err := scryptAlg.gen(pass)
470+ c.Assert(err, check.IsNil)
471+ c.Check(scryptAlg.cmp(hash, pass), check.IsNil)
472+
473+ scryptN = 3
474+ defer func() {
475+ scryptN = 2
476+ }()
477+
478+ c.Check(scryptAlg.cmp(hash, pass), check.ErrorMatches, "scrypt: N .*")
479+ _, err = scryptAlg.gen(pass)
480+ c.Check(err, check.ErrorMatches, "scrypt: N .*")
481+}
482+
483+func (s *cryptSuite) TestSHA512CryptFailrues(c *check.C) {
484+ c.Check(sha512cryptAlg.cmp([]byte("$$$$$$$$$$"), []byte("x")), check.ErrorMatches, "invalid argument")
485+}
486+
487+func (s *cryptSuite) TestRoundtrip(c *check.C) {
488+ c.Check(Write([]byte("password")), check.IsNil)
489+ c.Check(Check([]byte("password")), check.Equals, true)
490+ c.Check(Check([]byte("fishfish")), check.Equals, false)
491+}
492+
493+func (s *cryptSuite) TestRoundtripChangedDefaults(c *check.C) {
494+ DefaultAlg = "bcrypt"
495+ c.Check(Write([]byte("password")), check.IsNil)
496+ DefaultAlg = "scrypt"
497+ c.Check(Check([]byte("password")), check.Equals, true)
498+ c.Check(Check([]byte("fishfish")), check.Equals, false)
499+}
500+
501+var failAlg = cryptAlg{
502+ gen: func(password []byte) ([]byte, error) {
503+ return nil, errBadStuff
504+ },
505+ cmp: func([]byte, []byte) error {
506+ return errBadStuff
507+ },
508+}
509+
510+func (s *cryptSuite) TestCryptFailures(c *check.C) {
511+ alg := cryptAlgs[DefaultAlg]
512+ defer func() { cryptAlgs[DefaultAlg] = alg }()
513+ pass := []byte("password")
514+ dirs.SetRootDir("/does/not/exist")
515+
516+ // first, try to read it without having written it first:
517+ c.Check(Check(pass), check.Equals, false)
518+
519+ // next, write it
520+ c.Check(Write(pass), check.ErrorMatches, ".*no such file or directory.*")
521+ // oops, my bad
522+ dirs.SetRootDir(s.d)
523+ // *now* write it
524+ c.Check(Write(pass[:3]), check.ErrorMatches, ".*bad credentials.*")
525+ // jk, jk
526+ c.Check(Write(pass), check.IsNil)
527+ // pass, oh joy, etc
528+ c.Check(Check(pass), check.Equals, true)
529+ // write it again
530+ c.Check(Write(pass), check.IsNil)
531+ // yep, still passes
532+ c.Check(Check(pass), check.Equals, true)
533+
534+ // now the algorithm starts to fail. darned algorithms, they
535+ // don't make 'em like they used.
536+ cryptAlgs[DefaultAlg] = failAlg
537+
538+ c.Check(Write(pass), check.NotNil)
539+ c.Check(Check(pass), check.Equals, false)
540+
541+ // back to normal
542+ cryptAlgs[DefaultAlg] = alg
543+ c.Check(Check(pass), check.Equals, true)
544+
545+ f, err := os.OpenFile(pwFile(), os.O_WRONLY, 0)
546+ c.Check(err, check.IsNil)
547+
548+ // but what's this? filesystem corruption!
549+ _, err = f.WriteAt([]byte{0}, 0)
550+ c.Check(err, check.IsNil)
551+ c.Check(Check(pass), check.Equals, false)
552+
553+ // mental
554+ _, err = f.WriteAt(bytes.Repeat([]byte{'x'}, maxPrefixLen*2), 0)
555+ c.Check(err, check.IsNil)
556+ c.Check(Check(pass), check.Equals, false)
557+
558+ // all better
559+ c.Check(Write(pass), check.IsNil)
560+ c.Check(Check(pass), check.Equals, true)
561+
562+ // stahp!
563+ os.Truncate(pwFile(), maxPrefixLen-1)
564+ c.Check(Check(pass), check.Equals, false)
565+
566+ // no, really!
567+ os.Truncate(pwFile(), 0)
568+ c.Check(Check(pass), check.Equals, false)
569+
570+ // ok, enough
571+ c.Check(os.Remove(pwFile()), check.IsNil)
572+ c.Check(os.Mkdir(pwFile(), 0755), check.IsNil)
573+ c.Check(Check(pass), check.Equals, false)
574+ c.Check(os.Remove(pwFile()), check.IsNil)
575+ c.Check(os.Symlink("does-not-exist", pwFile()), check.IsNil)
576+ c.Check(Check(pass), check.Equals, false)
577+}
578+
579+func (s *cryptSuite) TestCryptNil(c *check.C) {
580+ alg := DefaultAlg
581+ defer func() { DefaultAlg = alg }()
582+
583+ for k := range cryptAlgs {
584+ DefaultAlg = k
585+
586+ c.Check(Write(nil), check.NotNil, check.Commentf(k))
587+ c.Check(Check(nil), check.NotNil, check.Commentf(k))
588+ }
589+}
590
591=== added file 'daemon/crypt/scrypt.go'
592--- daemon/crypt/scrypt.go 1970-01-01 00:00:00 +0000
593+++ daemon/crypt/scrypt.go 2015-10-09 13:33:12 +0000
594@@ -0,0 +1,78 @@
595+// -*- Mode: Go; indent-tabs-mode: t -*-
596+
597+/*
598+ * Copyright (C) 2014-2015 Canonical Ltd
599+ *
600+ * This program is free software: you can redistribute it and/or modify
601+ * it under the terms of the GNU General Public License version 3 as
602+ * published by the Free Software Foundation.
603+ *
604+ * This program is distributed in the hope that it will be useful,
605+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
606+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
607+ * GNU General Public License for more details.
608+ *
609+ * You should have received a copy of the GNU General Public License
610+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
611+ *
612+ */
613+
614+package crypt
615+
616+import (
617+ "bytes"
618+ "crypto/rand"
619+ "encoding/ascii85"
620+ "io"
621+
622+ "golang.org/x/crypto/scrypt"
623+)
624+
625+const (
626+ scryptSaltLen = 32
627+ scryptHashLen = 64
628+)
629+
630+var (
631+ scryptN = 1 << 10
632+ scryptR = 8
633+ scryptP = 16
634+)
635+
636+var scryptAlg = cryptAlg{
637+ cmp: func(hashedPassword, password []byte) error {
638+ saltedHash := make([]byte, scryptSaltLen+scryptHashLen)
639+ _, _, err := ascii85.Decode(saltedHash, hashedPassword, true)
640+ if err != nil {
641+ return err
642+ }
643+
644+ salt := saltedHash[:scryptSaltLen]
645+ hash, err := scrypt.Key([]byte(password), salt, scryptN, scryptR, scryptP, scryptHashLen)
646+ if err != nil {
647+ return err
648+ }
649+
650+ if !bytes.Equal(hash, saltedHash[scryptSaltLen:]) {
651+ return ErrBadCreds
652+ }
653+
654+ return nil
655+ },
656+ gen: func(password []byte) ([]byte, error) {
657+ salt := make([]byte, scryptSaltLen)
658+ if _, err := io.ReadFull(rand.Reader, salt); err != nil {
659+ return nil, err
660+ }
661+
662+ hash, err := scrypt.Key([]byte(password), salt, scryptN, scryptR, scryptP, scryptHashLen)
663+ if err != nil {
664+ return nil, err
665+ }
666+
667+ coded := make([]byte, ascii85.MaxEncodedLen(scryptSaltLen+scryptHashLen))
668+ n := ascii85.Encode(coded, append(salt, hash...))
669+
670+ return coded[:n], nil
671+ },
672+}
673
674=== added file 'daemon/crypt/sha512crypt.go'
675--- daemon/crypt/sha512crypt.go 1970-01-01 00:00:00 +0000
676+++ daemon/crypt/sha512crypt.go 2015-10-09 13:33:12 +0000
677@@ -0,0 +1,120 @@
678+// -*- Mode: Go; indent-tabs-mode: t -*-
679+
680+/*
681+ * Copyright (C) 2014-2015 Canonical Ltd
682+ *
683+ * This program is free software: you can redistribute it and/or modify
684+ * it under the terms of the GNU General Public License version 3 as
685+ * published by the Free Software Foundation.
686+ *
687+ * This program is distributed in the hope that it will be useful,
688+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
689+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
690+ * GNU General Public License for more details.
691+ *
692+ * You should have received a copy of the GNU General Public License
693+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
694+ *
695+ */
696+
697+package crypt
698+
699+/*
700+#cgo CFLAGS: -D_GNU_SOURCE -O3 -Wextra -Werror -pedantic
701+#cgo LDFLAGS: -lcrypt
702+#include <crypt.h>
703+#include <string.h>
704+*/
705+import "C"
706+
707+import (
708+ "bytes"
709+ "crypto/rand"
710+ "encoding/binary"
711+ "unsafe"
712+)
713+
714+const (
715+ sha512saltLetters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789./"
716+ sha512saltNChars = 16
717+)
718+
719+func mksalt() ([]byte, error) {
720+ raw := make([]uint32, 3)
721+ if err := binary.Read(rand.Reader, binary.LittleEndian, &raw); err != nil {
722+ return nil, err
723+ }
724+
725+ full := make([]byte, sha512saltNChars+4)
726+ full[0] = byte('$')
727+ full[1] = byte('6')
728+ full[2] = byte('$')
729+ full[3+sha512saltNChars] = byte('$')
730+ salt := full[3 : 3+sha512saltNChars]
731+
732+ m := uint32(0)
733+ for i, n := range raw {
734+ for j := 1; j < 6; j++ {
735+ salt[i*5+j] = sha512saltLetters[n&63]
736+ n >>= 6
737+ }
738+ m += n << (2 * uint(i))
739+ }
740+ salt[0] = sha512saltLetters[m]
741+
742+ return full, nil
743+}
744+
745+func crypt(password, saltOrHashedPassword []byte, checkOnly bool) ([]byte, error) {
746+ s := C.struct_crypt_data{}
747+ s.initialized = 0
748+
749+ if password[len(password)-1] != 0 {
750+ password = append(password, 0)
751+ }
752+ if saltOrHashedPassword[len(saltOrHashedPassword)-1] != 0 {
753+ saltOrHashedPassword = append(saltOrHashedPassword, 0)
754+ }
755+
756+ p, err := C.crypt_r(
757+ (*C.char)(unsafe.Pointer(&password[0])),
758+ (*C.char)(unsafe.Pointer(&saltOrHashedPassword[0])),
759+ &s,
760+ )
761+ if err != nil {
762+ return nil, err
763+ }
764+
765+ n := C.strnlen(p, maxSecretLen)
766+ if n == maxSecretLen {
767+ // just in case
768+ return nil, ErrBadCreds
769+ }
770+
771+ buf := (*[maxSecretLen]byte)(unsafe.Pointer(p))[:n+1] // null-terminated
772+
773+ if checkOnly {
774+ if bytes.Equal(buf, saltOrHashedPassword) {
775+ return nil, nil
776+ }
777+ return nil, ErrBadCreds
778+ }
779+
780+ return append([]byte(nil), buf...), nil
781+}
782+
783+var sha512cryptAlg = cryptAlg{
784+ gen: func(password []byte) ([]byte, error) {
785+ salt, err := mksalt()
786+ if err != nil {
787+ return nil, err
788+ }
789+
790+ return crypt(password, salt, false)
791+ },
792+ cmp: func(hashedPassword, password []byte) error {
793+ _, err := crypt(password, hashedPassword, true)
794+
795+ return err
796+ },
797+}
798
799=== modified file 'daemon/daemon.go'
800--- daemon/daemon.go 2015-10-05 23:10:59 +0000
801+++ daemon/daemon.go 2015-10-09 13:33:12 +0000
802@@ -30,6 +30,7 @@
803 "github.com/gorilla/mux"
804 "gopkg.in/tomb.v2"
805
806+ "launchpad.net/snappy/daemon/crypt"
807 "launchpad.net/snappy/logger"
808 )
809
810@@ -47,7 +48,9 @@
811
812 // A Command routes a request to an individual per-verb ResponseFUnc
813 type Command struct {
814- Path string
815+ // GuestOK: whether GET requests can proceed without auth
816+ GuestOK bool
817+ Path string
818 //
819 GET ResponseFunc
820 PUT ResponseFunc
821@@ -57,7 +60,26 @@
822 d *Daemon
823 }
824
825+// AuthOK checks whether the request has enough authorization for the command
826+func (c *Command) AuthOK(r *http.Request) bool {
827+ if c.GuestOK && r.Method == "GET" {
828+ return true
829+ }
830+
831+ user, pass, ok := basicAuth(r)
832+ if !ok {
833+ return false
834+ }
835+
836+ return crypt.Check([]byte(user + pass))
837+}
838+
839 func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
840+ if !c.AuthOK(r) {
841+ Unauthorized.ServeHTTP(w, r)
842+ return
843+ }
844+
845 var rspf ResponseFunc
846 var rsp Response = BadMethod
847
848
849=== modified file 'daemon/daemon_test.go'
850--- daemon/daemon_test.go 2015-10-05 23:10:59 +0000
851+++ daemon/daemon_test.go 2015-10-09 13:33:12 +0000
852@@ -23,52 +23,77 @@
853 "fmt"
854 "net/http"
855 "net/http/httptest"
856+ "os"
857+ "path/filepath"
858
859 "github.com/gorilla/mux"
860 "gopkg.in/check.v1"
861+
862+ "launchpad.net/snappy/daemon/crypt"
863+ "launchpad.net/snappy/dirs"
864 )
865
866 type daemonSuite struct{}
867
868 var _ = check.Suite(&daemonSuite{})
869
870-type mockHandler struct {
871- cmd *Command
872- lastMethod string
873-}
874-
875-func (mck *mockHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
876- mck.lastMethod = r.Method
877-}
878-func (mck *mockHandler) Self(*Command, *http.Request) Response {
879- return mck
880-}
881-
882-func mkRF(c *check.C, cmd *Command, mck *mockHandler) ResponseFunc {
883+type nullHandler struct{}
884+
885+func (nu *nullHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
886+}
887+func (nu *nullHandler) Self(*Command, *http.Request) Response {
888+ return nu
889+}
890+
891+func mkRF(c *check.C, cmd *Command, method string) ResponseFunc {
892 return func(innerCmd *Command, req *http.Request) Response {
893 c.Assert(cmd, check.Equals, innerCmd)
894- return mck
895+ c.Assert(method, check.Equals, req.Method)
896+ return &nullHandler{}
897 }
898 }
899
900 func (s *daemonSuite) TestCommandMethodDispatch(c *check.C) {
901+ dirs.SetRootDir(c.MkDir())
902+ c.Check(os.MkdirAll(filepath.Join(dirs.GlobalRootDir, dirs.SnappyDir), 0755), check.IsNil)
903+
904 cmd := &Command{}
905- mck := &mockHandler{cmd: cmd}
906- rf := mkRF(c, cmd, mck)
907- cmd.GET = rf
908- cmd.PUT = rf
909- cmd.POST = rf
910- cmd.DELETE = rf
911+ cmd.GET = mkRF(c, cmd, "GET")
912+ cmd.PUT = mkRF(c, cmd, "PUT")
913+ cmd.POST = mkRF(c, cmd, "POST")
914+ cmd.DELETE = mkRF(c, cmd, "DELETE")
915+
916+ crypt.DefaultAlg = crypt.SHA512
917+ c.Check(crypt.Write([]byte("password")), check.IsNil)
918
919 for _, method := range []string{"GET", "POST", "PUT", "DELETE"} {
920 req, err := http.NewRequest(method, "", nil)
921- c.Assert(err, check.IsNil)
922- cmd.ServeHTTP(nil, req)
923- c.Check(mck.lastMethod, check.Equals, method)
924+ c.Assert(err, check.IsNil, check.Commentf(method))
925+
926+ cmd.GuestOK = false
927+ rec := httptest.NewRecorder()
928+ cmd.ServeHTTP(rec, req)
929+ c.Check(rec.Code, check.Equals, http.StatusUnauthorized, check.Commentf(method))
930+
931+ cmd.GuestOK = true
932+ rec = httptest.NewRecorder()
933+ cmd.ServeHTTP(rec, req)
934+ if method == "GET" {
935+ c.Check(rec.Code, check.Equals, http.StatusOK, check.Commentf(method))
936+ } else {
937+ c.Check(rec.Code, check.Equals, http.StatusUnauthorized, check.Commentf(method))
938+ }
939+ cmd.GuestOK = false
940+
941+ rec = httptest.NewRecorder()
942+ setBasicAuth(req, "pass", "word")
943+ cmd.ServeHTTP(rec, req)
944+ c.Check(rec.Code, check.Equals, http.StatusOK, check.Commentf(method))
945 }
946
947 req, err := http.NewRequest("POTATO", "", nil)
948 c.Assert(err, check.IsNil)
949+ setBasicAuth(req, "pass", "word")
950 rec := httptest.NewRecorder()
951 cmd.ServeHTTP(rec, req)
952 c.Check(rec.Code, check.Equals, http.StatusMethodNotAllowed)
953
954=== modified file 'daemon/response.go'
955--- daemon/response.go 2015-10-08 12:52:40 +0000
956+++ daemon/response.go 2015-10-09 13:33:12 +0000
957@@ -177,4 +177,5 @@
958 BadMethod = ErrorResponse(http.StatusMethodNotAllowed)
959 InternalError = ErrorResponse(http.StatusInternalServerError)
960 NotImplemented = ErrorResponse(http.StatusNotImplemented)
961+ Unauthorized = ErrorResponse(http.StatusUnauthorized)
962 )

Subscribers

People subscribed via source and target branches