From b73fe8c157acc673199caa3c1bb84cd8a82ac493 Mon Sep 17 00:00:00 2001 From: max furman Date: Thu, 2 May 2019 15:26:18 -0700 Subject: [PATCH] Add used OTT to DB during authToken step --- Gopkg.lock | 4 +- authority/authorize.go | 23 ++++++++--- authority/authorize_test.go | 77 ++++++++++++++++++++++++++++++++++++- authority/db_test.go | 11 ++++++ authority/tls_test.go | 20 ++++++++-- db/db.go | 21 +++++++++- db/db_test.go | 74 +++++++++++++++++++++++++++++++++++ db/noop.go | 5 +++ 8 files changed, 222 insertions(+), 13 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index d4514ef2..8f6edc08 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -363,7 +363,7 @@ [[projects]] branch = "master" - digest = "1:f1f1df1e19d55a1ef1f0a63633191e7d2a99993c0a17f945c0b9ebd16b17871b" + digest = "1:5e778214d472b6d2ad4d544d293d1478d9b222db8ffc6079623fbe3e58e1841e" name = "github.com/smallstep/nosql" packages = [ ".", @@ -373,7 +373,7 @@ "mysql", ] pruneopts = "UT" - revision = "5a355c598075a346d9ca9b50ec10e3f86ac66148" + revision = "b66b34823456721912ba037126e92414690c07d6" [[projects]] branch = "master" diff --git a/authority/authorize.go b/authority/authorize.go index 62152d09..3f4d048a 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/db" "github.com/smallstep/cli/jose" ) @@ -72,11 +73,23 @@ func (a *Authority) authorizeToken(ott string) (provisioner.Interface, error) { reuseKey = claims.Nonce } if reuseKey != "" { - if _, ok := a.ottMap.LoadOrStore(reuseKey, &idUsed{ - UsedAt: time.Now().Unix(), - Subject: claims.Subject, - }); ok { - return nil, &apiError{errors.Errorf("authorizeToken: token already used"), http.StatusUnauthorized, errContext} + switch a.db.(type) { + case *db.NoopDB: + if _, ok := a.ottMap.LoadOrStore(reuseKey, &idUsed{ + UsedAt: time.Now().Unix(), + Subject: claims.Subject, + }); ok { + return nil, &apiError{errors.Errorf("authorizeToken: token already used"), http.StatusUnauthorized, errContext} + } + default: + ok, err := a.db.UseToken(reuseKey, ott) + if err != nil { + return nil, &apiError{errors.Wrap(err, "authorizeToken: failed when checking if token already used"), + http.StatusInternalServerError, errContext} + } + if !ok { + return nil, &apiError{errors.Errorf("authorizeToken: token already used"), http.StatusUnauthorized, errContext} + } } } diff --git a/authority/authorize_test.go b/authority/authorize_test.go index 90365655..43b51ff8 100644 --- a/authority/authorize_test.go +++ b/authority/authorize_test.go @@ -117,7 +117,7 @@ func TestAuthority_authorizeToken(t *testing.T) { http.StatusUnauthorized, context{"ott": raw}}, } }, - "ok": func(t *testing.T) *authorizeTest { + "ok/noopdb": func(t *testing.T) *authorizeTest { cl := jwt.Claims{ Subject: "test.smallstep.com", Issuer: validIssuer, @@ -133,7 +133,7 @@ func TestAuthority_authorizeToken(t *testing.T) { ott: raw, } }, - "fail/token-already-used": func(t *testing.T) *authorizeTest { + "fail/noopdb/token-already-used": func(t *testing.T) *authorizeTest { _a := testAuthority(t) cl := jwt.Claims{ @@ -155,6 +155,79 @@ func TestAuthority_authorizeToken(t *testing.T) { http.StatusUnauthorized, context{"ott": raw}}, } }, + "ok/mockNoSQLDB": func(t *testing.T) *authorizeTest { + _a := testAuthority(t) + _a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return true, nil + }, + } + + cl := jwt.Claims{ + Subject: "test.smallstep.com", + Issuer: validIssuer, + NotBefore: jwt.NewNumericDate(now), + Expiry: jwt.NewNumericDate(now.Add(time.Minute)), + Audience: validAudience, + ID: "43", + } + raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize() + assert.FatalError(t, err) + return &authorizeTest{ + auth: _a, + ott: raw, + } + }, + "fail/mockNoSQLDB/error": func(t *testing.T) *authorizeTest { + _a := testAuthority(t) + _a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return false, errors.New("force") + }, + } + + cl := jwt.Claims{ + Subject: "test.smallstep.com", + Issuer: validIssuer, + NotBefore: jwt.NewNumericDate(now), + Expiry: jwt.NewNumericDate(now.Add(time.Minute)), + Audience: validAudience, + ID: "43", + } + raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize() + assert.FatalError(t, err) + return &authorizeTest{ + auth: _a, + ott: raw, + err: &apiError{errors.New("authorizeToken: failed when checking if token already used: force"), + http.StatusInternalServerError, context{"ott": raw}}, + } + }, + "fail/mockNoSQLDB/token-already-used": func(t *testing.T) *authorizeTest { + _a := testAuthority(t) + _a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return false, nil + }, + } + + cl := jwt.Claims{ + Subject: "test.smallstep.com", + Issuer: validIssuer, + NotBefore: jwt.NewNumericDate(now), + Expiry: jwt.NewNumericDate(now.Add(time.Minute)), + Audience: validAudience, + ID: "43", + } + raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize() + assert.FatalError(t, err) + return &authorizeTest{ + auth: _a, + ott: raw, + err: &apiError{errors.New("authorizeToken: token already used"), + http.StatusUnauthorized, context{"ott": raw}}, + } + }, } for name, genTestCase := range tests { diff --git a/authority/db_test.go b/authority/db_test.go index b4e073f1..870a3984 100644 --- a/authority/db_test.go +++ b/authority/db_test.go @@ -13,6 +13,7 @@ type MockAuthDB struct { isRevoked func(string) (bool, error) revoke func(rci *db.RevokedCertificateInfo) error storeCertificate func(crt *x509.Certificate) error + useToken func(id, tok string) (bool, error) shutdown func() error } @@ -33,6 +34,16 @@ func (m *MockAuthDB) IsRevoked(sn string) (bool, error) { return m.ret1.(bool), m.err } +func (m *MockAuthDB) UseToken(id, tok string) (bool, error) { + if m.useToken != nil { + return m.useToken(id, tok) + } + if m.ret1 == nil { + return false, m.err + } + return m.ret1.(bool), m.err +} + func (m *MockAuthDB) Revoke(rci *db.RevokedCertificateInfo) error { if m.revoke != nil { return m.revoke(rci) diff --git a/authority/tls_test.go b/authority/tls_test.go index ea2c4063..7efe7ab9 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -640,7 +640,12 @@ func TestRevoke(t *testing.T) { }, "error/db-revoke": func() test { a := testAuthority(t) - a.db = &MockAuthDB{err: errors.New("force")} + a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return true, nil + }, + err: errors.New("force"), + } cl := jwt.Claims{ Subject: "sn", @@ -671,7 +676,12 @@ func TestRevoke(t *testing.T) { }, "error/already-revoked": func() test { a := testAuthority(t) - a.db = &MockAuthDB{err: db.ErrAlreadyExists} + a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return true, nil + }, + err: db.ErrAlreadyExists, + } cl := jwt.Claims{ Subject: "sn", @@ -702,7 +712,11 @@ func TestRevoke(t *testing.T) { }, "ok/token": func() test { a := testAuthority(t) - a.db = &MockAuthDB{} + a.db = &MockAuthDB{ + useToken: func(id, tok string) (bool, error) { + return true, nil + }, + } cl := jwt.Claims{ Subject: "sn", diff --git a/db/db.go b/db/db.go index 886c3ef7..3687b113 100644 --- a/db/db.go +++ b/db/db.go @@ -10,8 +10,9 @@ import ( ) var ( - revokedCertsTable = []byte("revoked_x509_certs") certsTable = []byte("x509_certs") + revokedCertsTable = []byte("revoked_x509_certs") + usedOTTTable = []byte("used_ott") ) // ErrAlreadyExists can be returned if the DB attempts to set a key that has @@ -31,6 +32,7 @@ type AuthDB interface { IsRevoked(sn string) (bool, error) Revoke(rci *RevokedCertificateInfo) error StoreCertificate(crt *x509.Certificate) error + UseToken(id, tok string) (bool, error) Shutdown() error } @@ -126,6 +128,23 @@ func (db *DB) StoreCertificate(crt *x509.Certificate) error { return nil } +// UseToken returns true if we were able to successfully store the token for +// for the first time, false otherwise. +func (db *DB) UseToken(id, tok string) (bool, error) { + // If the error is `Not Found` then the certificate has not been revoked. + // Any other error should be propagated to the caller. + _, found, err := db.LoadOrStore(usedOTTTable, []byte(id), []byte(tok)) + switch { + case err != nil: + return false, errors.Wrapf(err, "error LoadOrStore-ing token %s/%s", + string(usedOTTTable), id) + case found: + return false, nil + default: + return true, nil + } +} + // Shutdown sends a shutdown message to the database. func (db *DB) Shutdown() error { if db.isUp { diff --git a/db/db_test.go b/db/db_test.go index c2b16d57..c5dbe4fe 100644 --- a/db/db_test.go +++ b/db/db_test.go @@ -20,6 +20,17 @@ type MockNoSQLDB struct { del func(bucket, key []byte) error list func(bucket []byte) ([]*database.Entry, error) update func(tx *database.Tx) error + loadOrStore func(bucket, key, value []byte) ([]byte, bool, error) +} + +func (m *MockNoSQLDB) LoadOrStore(bucket, key, value []byte) ([]byte, bool, error) { + if m.get != nil { + return m.loadOrStore(bucket, key, value) + } + if m.ret1 == nil { + return nil, false, m.err + } + return m.ret1.([]byte), m.ret2.(bool), m.err } func (m *MockNoSQLDB) Get(bucket, key []byte) ([]byte, error) { @@ -188,3 +199,66 @@ func TestRevoke(t *testing.T) { }) } } + +func TestUseToken(t *testing.T) { + type result struct { + err error + ok bool + } + tests := map[string]struct { + id, tok string + db *DB + want result + }{ + "fail/force-LoadOrStore-error": { + id: "id", + tok: "token", + db: &DB{&MockNoSQLDB{ + loadOrStore: func(bucket, key, value []byte) ([]byte, bool, error) { + return nil, false, errors.New("force") + }, + }, true}, + want: result{ + ok: false, + err: errors.New("error LoadOrStore-ing token id/token"), + }, + }, + "fail/LoadOrStore-found": { + id: "id", + tok: "token", + db: &DB{&MockNoSQLDB{ + loadOrStore: func(bucket, key, value []byte) ([]byte, bool, error) { + return []byte("foo"), true, nil + }, + }, true}, + want: result{ + ok: false, + }, + }, + "ok/LoadOrStore-not-found": { + id: "id", + tok: "token", + db: &DB{&MockNoSQLDB{ + loadOrStore: func(bucket, key, value []byte) ([]byte, bool, error) { + return nil, false, nil + }, + }, true}, + want: result{ + ok: true, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ok, err := tc.db.UseToken(tc.id, tc.tok) + if err != nil { + if assert.NotNil(t, tc.want.err) { + assert.HasPrefix(t, tc.want.err.Error(), err.Error()) + } + assert.False(t, ok) + } else { + assert.True(t, ok) + } + }) + } +} diff --git a/db/noop.go b/db/noop.go index 356b7f42..27373fa1 100644 --- a/db/noop.go +++ b/db/noop.go @@ -32,6 +32,11 @@ func (n *NoopDB) StoreCertificate(crt *x509.Certificate) error { return ErrNotImplemented } +// UseToken returns a "NotImplemented" error. +func (n *NoopDB) UseToken(id, tok string) (bool, error) { + return false, ErrNotImplemented +} + // Shutdown returns nil func (n *NoopDB) Shutdown() error { return nil