From ab17165352a9b04ac69c158efe7fdac20197cdc0 Mon Sep 17 00:00:00 2001 From: rob boll Date: Mon, 23 Nov 2015 16:10:08 -0500 Subject: [PATCH 1/9] v2http: refactor http basic auth refactor http basic auth code to combine basic auth extraction and validation --- etcdserver/api/v2http/client_auth.go | 54 ++++++++--------- etcdserver/api/v2http/client_auth_test.go | 71 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/etcdserver/api/v2http/client_auth.go b/etcdserver/api/v2http/client_auth.go index cf1585bed..0907c438a 100644 --- a/etcdserver/api/v2http/client_auth.go +++ b/etcdserver/api/v2http/client_auth.go @@ -37,6 +37,25 @@ func hasWriteRootAccess(sec auth.Store, r *http.Request) bool { return hasRootAccess(sec, r) } +func userFromBasicAuth(sec auth.Store, r *http.Request) *auth.User { + username, password, ok := r.BasicAuth() + if !ok { + plog.Warningf("auth: malformed basic auth encoding") + return nil + } + user, err := sec.GetUser(username) + if err != nil { + return nil + } + + ok = sec.CheckPassword(user, password) + if !ok { + plog.Warningf("auth: incorrect password for user: %s", username) + return nil + } + return &user +} + func hasRootAccess(sec auth.Store, r *http.Request) bool { if sec == nil { // No store means no auth available, eg, tests. @@ -45,26 +64,18 @@ func hasRootAccess(sec auth.Store, r *http.Request) bool { if !sec.AuthEnabled() { return true } - username, password, ok := r.BasicAuth() - if !ok { - return false - } - rootUser, err := sec.GetUser(username) - if err != nil { + + rootUser := userFromBasicAuth(sec, r) + if rootUser == nil { return false } - ok = sec.CheckPassword(rootUser, password) - if !ok { - plog.Warningf("auth: wrong password for user %s", username) - return false - } for _, role := range rootUser.Roles { if role == auth.RootRoleName { return true } } - plog.Warningf("auth: user %s does not have the %s role for resource %s.", username, auth.RootRoleName, r.URL.Path) + plog.Warningf("auth: user %s does not have the %s role for resource %s.", rootUser.User, auth.RootRoleName, r.URL.Path) return false } @@ -80,21 +91,12 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b plog.Warningf("auth: no authorization provided, checking guest access") return hasGuestAccess(sec, r, key) } - username, password, ok := r.BasicAuth() - if !ok { - plog.Warningf("auth: malformed basic auth encoding") - return false - } - user, err := sec.GetUser(username) - if err != nil { - plog.Warningf("auth: no such user: %s.", username) - return false - } - authAsUser := sec.CheckPassword(user, password) - if !authAsUser { - plog.Warningf("auth: incorrect password for user: %s.", username) + + user := userFromBasicAuth(sec, r) + if user == nil { return false } + writeAccess := r.Method != "GET" && r.Method != "HEAD" for _, roleName := range user.Roles { role, err := sec.GetRole(roleName) @@ -109,7 +111,7 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b return true } } - plog.Warningf("auth: invalid access for user %s on key %s.", username, key) + plog.Warningf("auth: invalid access for user %s on key %s.", user.User, key) return false } diff --git a/etcdserver/api/v2http/client_auth_test.go b/etcdserver/api/v2http/client_auth_test.go index 734e19ac0..5afbd750b 100644 --- a/etcdserver/api/v2http/client_auth_test.go +++ b/etcdserver/api/v2http/client_auth_test.go @@ -440,6 +440,14 @@ func mustAuthRequest(method, username, password string) *http.Request { return req } +func unauthedRequest(method string) *http.Request { + req, err := http.NewRequest(method, "path", strings.NewReader("")) + if err != nil { + panic("Cannot make request: " + err.Error()) + } + return req +} + func TestPrefixAccess(t *testing.T) { var table = []struct { key string @@ -701,3 +709,66 @@ func TestPrefixAccess(t *testing.T) { } } } + +func TestUserFromBasicAuth(t *testing.T) { + sec := &mockAuthStore{ + users: map[string]*auth.User{ + "user": { + User: "user", + Roles: []string{"root"}, + Password: "password", + }, + }, + roles: map[string]*auth.Role{ + "root": { + Role: "root", + }, + }, + } + + var table = []struct { + username string + req *http.Request + userExists bool + }{ + { + // valid user, valid pass + username: "user", + req: mustAuthRequest("GET", "user", "password"), + userExists: true, + }, + { + // valid user, bad pass + username: "user", + req: mustAuthRequest("GET", "user", "badpass"), + userExists: false, + }, + { + // valid user, no pass + username: "user", + req: mustAuthRequest("GET", "user", ""), + userExists: false, + }, + { + // missing user + username: "missing", + req: mustAuthRequest("GET", "missing", "badpass"), + userExists: false, + }, + { + // no basic auth + req: unauthedRequest("GET"), + userExists: false, + }, + } + + for i, tt := range table { + user := userFromBasicAuth(sec, tt.req) + if tt.userExists == (user == nil) { + t.Errorf("#%d: userFromBasicAuth doesn't match (expected %v)", i, tt.userExists) + } + if user != nil && (tt.username != user.User) { + t.Errorf("#%d: userFromBasicAuth username doesn't match (expected %s, got %s)", i, tt.username, user.User) + } + } +} From ff5709bb4178c49f48be36edda66600864141837 Mon Sep 17 00:00:00 2001 From: rob boll Date: Mon, 23 Nov 2015 23:11:56 -0500 Subject: [PATCH 2/9] v2http: client cert cn authentication introduce client certificate authentication using certificate cn. --- etcdserver/api/v2http/client_auth.go | 51 ++++++++-- etcdserver/api/v2http/client_auth_test.go | 108 ++++++++++++++++++++- etcdserver/etcdhttp/testdata/ca.pem | 19 ++++ etcdserver/etcdhttp/testdata/otheruser.pem | 20 ++++ etcdserver/etcdhttp/testdata/user.pem | 20 ++++ 5 files changed, 207 insertions(+), 11 deletions(-) create mode 100644 etcdserver/etcdhttp/testdata/ca.pem create mode 100644 etcdserver/etcdhttp/testdata/otheruser.pem create mode 100644 etcdserver/etcdhttp/testdata/user.pem diff --git a/etcdserver/api/v2http/client_auth.go b/etcdserver/api/v2http/client_auth.go index 0907c438a..286dd3209 100644 --- a/etcdserver/api/v2http/client_auth.go +++ b/etcdserver/api/v2http/client_auth.go @@ -56,6 +56,24 @@ func userFromBasicAuth(sec auth.Store, r *http.Request) *auth.User { return &user } +func userFromClientCertificate(sec auth.Store, r *http.Request) *auth.User { + if r.TLS == nil { + return nil + } + + for _, chains := range r.TLS.VerifiedChains { + for _, chain := range chains { + plog.Debugf("auth: found common name %s.\n", chain.Subject.CommonName) + user, err := sec.GetUser(chain.Subject.CommonName) + if err == nil { + plog.Debugf("auth: authenticated user %s by cert common name.", user.User) + return &user + } + } + } + return nil +} + func hasRootAccess(sec auth.Store, r *http.Request) bool { if sec == nil { // No store means no auth available, eg, tests. @@ -65,9 +83,17 @@ func hasRootAccess(sec auth.Store, r *http.Request) bool { return true } - rootUser := userFromBasicAuth(sec, r) - if rootUser == nil { - return false + var rootUser *auth.User + if r.Header.Get("Authorization") == "" { + rootUser = userFromClientCertificate(sec, r) + if rootUser == nil { + return false + } + } else { + rootUser = userFromBasicAuth(sec, r) + if rootUser == nil { + return false + } } for _, role := range rootUser.Roles { @@ -87,14 +113,19 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b if !sec.AuthEnabled() { return true } - if r.Header.Get("Authorization") == "" { - plog.Warningf("auth: no authorization provided, checking guest access") - return hasGuestAccess(sec, r, key) - } - user := userFromBasicAuth(sec, r) - if user == nil { - return false + var user *auth.User + if r.Header.Get("Authorization") == "" { + user = userFromClientCertificate(sec, r) + if user == nil { + plog.Warningf("auth: no authorization provided, checking guest access") + return hasGuestAccess(sec, r, key) + } + } else { + user = userFromBasicAuth(sec, r) + if user == nil { + return false + } } writeAccess := r.Method != "GET" && r.Method != "HEAD" diff --git a/etcdserver/api/v2http/client_auth_test.go b/etcdserver/api/v2http/client_auth_test.go index 5afbd750b..8976148a9 100644 --- a/etcdserver/api/v2http/client_auth_test.go +++ b/etcdserver/api/v2http/client_auth_test.go @@ -15,9 +15,13 @@ package v2http import ( + "crypto/tls" + "crypto/x509" "encoding/json" + "encoding/pem" "errors" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -448,6 +452,24 @@ func unauthedRequest(method string) *http.Request { return req } +func tlsAuthedRequest(req *http.Request, certname string) *http.Request { + bytes, err := ioutil.ReadFile(fmt.Sprintf("testdata/%s.pem", certname)) + if err != nil { + panic(err) + } + + block, _ := pem.Decode(bytes) + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + panic(err) + } + + req.TLS = &tls.ConnectionState{ + VerifiedChains: [][]*x509.Certificate{{cert}}, + } + return req +} + func TestPrefixAccess(t *testing.T) { var table = []struct { key string @@ -710,6 +732,88 @@ func TestPrefixAccess(t *testing.T) { } } +func TestUserFromClientCertificate(t *testing.T) { + witherror := &mockAuthStore{ + users: map[string]*auth.User{ + "user": { + User: "user", + Roles: []string{"root"}, + Password: "password", + }, + "basicauth": { + User: "basicauth", + Roles: []string{"root"}, + Password: "password", + }, + }, + roles: map[string]*auth.Role{ + "root": { + Role: "root", + }, + }, + err: errors.New(""), + } + + noerror := &mockAuthStore{ + users: map[string]*auth.User{ + "user": { + User: "user", + Roles: []string{"root"}, + Password: "password", + }, + "basicauth": { + User: "basicauth", + Roles: []string{"root"}, + Password: "password", + }, + }, + roles: map[string]*auth.Role{ + "root": { + Role: "root", + }, + }, + } + + var table = []struct { + req *http.Request + userExists bool + store auth.Store + username string + }{ + { + // non tls request + req: unauthedRequest("GET"), + userExists: false, + store: witherror, + }, + { + // cert with cn of existing user + req: tlsAuthedRequest(unauthedRequest("GET"), "user"), + userExists: true, + username: "user", + store: noerror, + }, + { + // cert with cn of non-existing user + req: tlsAuthedRequest(unauthedRequest("GET"), "otheruser"), + userExists: false, + store: witherror, + }, + } + + for i, tt := range table { + user := userFromClientCertificate(tt.store, tt.req) + userExists := user != nil + + if tt.userExists != userExists { + t.Errorf("#%d: userFromClientCertificate doesn't match (expected %v)", i, tt.userExists) + } + if user != nil && (tt.username != user.User) { + t.Errorf("#%d: userFromClientCertificate username doesn't match (expected %s, got %s)", i, tt.username, user.User) + } + } +} + func TestUserFromBasicAuth(t *testing.T) { sec := &mockAuthStore{ users: map[string]*auth.User{ @@ -764,7 +868,9 @@ func TestUserFromBasicAuth(t *testing.T) { for i, tt := range table { user := userFromBasicAuth(sec, tt.req) - if tt.userExists == (user == nil) { + userExists := user != nil + + if tt.userExists != userExists { t.Errorf("#%d: userFromBasicAuth doesn't match (expected %v)", i, tt.userExists) } if user != nil && (tt.username != user.User) { diff --git a/etcdserver/etcdhttp/testdata/ca.pem b/etcdserver/etcdhttp/testdata/ca.pem new file mode 100644 index 000000000..60cbee3bb --- /dev/null +++ b/etcdserver/etcdhttp/testdata/ca.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDEjCCAfqgAwIBAgIIYpX+8HgWGfkwDQYJKoZIhvcNAQELBQAwFTETMBEGA1UE +AxMKZXRjZCB0ZXN0czAeFw0xNTExMjQwMzA1MDBaFw0yMDExMjIwMzA1MDBaMBUx +EzARBgNVBAMTCmV0Y2QgdGVzdHMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQDa9PkwEwiBD8mB+VIKz5r5gRHnNF4Icj6T6R/RsdatecQe6vU0EU4FXtKZ +drWnCGlATyrQooqHpb+rDc7CUt3mXrIxrNkcGTMaesF7P0GWxVkyOGSjJMxGBv3e +bAZknBe4eLMi68L1aT/uYmxcp/B3L2mfdFtc1Gd6mYJpNm1PgilRyIrO0mY5ysIX +4WHCa3yudAv8HrFbQcw7l7OyKA6uSWg6h07lE3d5jw5YOly+hz0iaRtzhb4tJrYD +Lm1tehb0nnoLuW6yYblRSoyBVDT50MFVlyvW40Po5WkOXw/wnsnyxWRR4yqU23wq +quQU0HXJEBLFnT+KbLOQ0EAE35vXAgMBAAGjZjBkMA4GA1UdDwEB/wQEAwIBBjAS +BgNVHRMBAf8ECDAGAQH/AgECMB0GA1UdDgQWBBSbUCGB95ochDrbEZlzGGYuA7xu +xjAfBgNVHSMEGDAWgBSbUCGB95ochDrbEZlzGGYuA7xuxjANBgkqhkiG9w0BAQsF +AAOCAQEAardO/SGCu7Snz3YRBUinzpZEUFTFend+FJtBkxBXCao1RvTXg8PBMkza +LUsaR4mLsGoXLIbNCoIinvVG0QULYCZe11N3l1L0G2g5uhEM4MfJ2rwrMD0o17i+ +nwNRRE3tfKAlWhYQg+4ye36kQVxASPniHjdQgjKYUFTNXdyG6DzuAclaVte9iVw6 +cWl61fB2CZya3+uMtih8t/Kgl2KbMO2PvNByfnDjKmW+v58qHbXyoJZqnpvDn14+ +p2Ox+AvvxYiEiUIvFdWy101QB7NJMCtdwq6oG6OvIOgXzLgitTFSq4kfWDfupQjW +iFoQ+vWmYhK5ld0nBaiz+JmHuemK7A== +-----END CERTIFICATE----- diff --git a/etcdserver/etcdhttp/testdata/otheruser.pem b/etcdserver/etcdhttp/testdata/otheruser.pem new file mode 100644 index 000000000..d0c74eb9f --- /dev/null +++ b/etcdserver/etcdhttp/testdata/otheruser.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDOTCCAiGgAwIBAgIINYpsso1f3SswDQYJKoZIhvcNAQELBQAwFTETMBEGA1UE +AxMKZXRjZCB0ZXN0czAeFw0xNTExMjQwMzA4MDBaFw0xNjExMjMwMzA4MDBaMBQx +EjAQBgNVBAMTCW90aGVydXNlcjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC +ggEBAPOAUa5GblwIjHTEnox2c/Am9jV1TMvzBuVXxnp2UnNHMNwstAooFrEs/Z+d +ft5AOsooP6zVuM3eBQa4i9huJbVNDfPU2H94yA89jYfJYUgo7C838V6NjGsCCptQ +WzkKPNlDbT9xA/7XpIUJ2WltuYDRrjWq8pXQONqTjcg5n4l0JO8xdHJHRUkFQ76F +1npXeLndgGaP11lqzpYlglEGi5URhzAT1xxQ0hLSe8WNmiCxxkq++C8Gx4sPg9mX +M94aoJDzZSnoaqDxckbP/7Q0ZKe/fVdCFkd5+jqT4Mt7hwmz9jTCHcVnAz4EKI+t +rbWgbCfMK6013GotXz7InStVe+MCAwEAAaOBjTCBijAOBgNVHQ8BAf8EBAMCBaAw +HQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYD +VR0OBBYEFFwMmf+pnaejmri6y1T+lfU+MBq/MB8GA1UdIwQYMBaAFJtQIYH3mhyE +OtsRmXMYZi4DvG7GMAsGA1UdEQQEMAKCADANBgkqhkiG9w0BAQsFAAOCAQEACOn6 +mec29MTMGPt/EPOmSyhvTKSwH+5YWjCbyUFeoB8puxrJlIphK4mvT+sXp2wzno89 +FVCliO/rJurdErKvyOjlK1QrVGPYIt7Wz9ssAfvlwCyBM8PqgEG8dJN9aAkf2h4r +Ye+hBh1y6Nnataf7lxe9mqAOvD/7wVIgzjCnMD1q5QSY2Mln3HwVQXtbZFbY363Z +X9Fk3PUpjJSX9jbEz9kIlT8AJAdxl6GB8Z9B8PrA8qf4Bhk15ICRHxb67EhDrGWV +8q7ArU2XBqs/+GWpUIMoGKNZv+K+/SksZK1KnzaUvApUCJzt+ac+p8HOgMdvDRgr +GfVVJqcZgyEmeczy0A== +-----END CERTIFICATE----- diff --git a/etcdserver/etcdhttp/testdata/user.pem b/etcdserver/etcdhttp/testdata/user.pem new file mode 100644 index 000000000..0fc210865 --- /dev/null +++ b/etcdserver/etcdhttp/testdata/user.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDNDCCAhygAwIBAgIIcQ0DAfgevocwDQYJKoZIhvcNAQELBQAwFTETMBEGA1UE +AxMKZXRjZCB0ZXN0czAeFw0xNTExMjQwMzA4MDBaFw0xNjExMjMwMzA4MDBaMA8x +DTALBgNVBAMTBHVzZXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQD0 ++3Lm1SmUJJLufaFTYz+e5qyQEshNRyeAhXIeZ1aw+yBjslXGZQ3/uGOwnOnGqUeA +Nidc9ty4NsK6RVppHlezUrBnpl4hws8vHWFKZpU2R6kKL8EYLmg+iVqEBj7XqfAp +8bJqqZI3KOqLXpRH55mA69KP7VEK9ngTVR/tERSrUPT8jcjwbvhSOqD8Qk07BUDR +6RpDr94Mnaf+fMGG36Sh7iUl+i4Oh6FFar+7+b0+5Bhs2/6uVsK4A1Z3jqqfSQH8 +q8Wf5h9Ka4aqGSw4ia5G3Uw7Jsl2aDgpJ7uwJo1k8SclbMYnYdhZuo+U+esY/Fai +YdbjG+AroZ+y9TB8bMlHAgMBAAGjgY0wgYowDgYDVR0PAQH/BAQDAgWgMB0GA1Ud +JQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQW +BBRuTt0lJIVKYaz76aSxl/MQOLRwfDAfBgNVHSMEGDAWgBSbUCGB95ochDrbEZlz +GGYuA7xuxjALBgNVHREEBDACggAwDQYJKoZIhvcNAQELBQADggEBABLRWZm+Lgjs +c5qDXbgOJW2pR630syY8ixR9c6HvzPVJim8mFioMX+xrlbOC6BmOUlOb9j83bTKn +aOg/0xlpxNbd8QYzgRxZmHZLULPdiNeeRvIzsrzrH88+inrmZhRXRVcHjdO6CG6t +hCdDdRiNU6GkF7dPna0xNcEOKe2wUfzd1ZtKOqzi1w+fKjSeMplZomeWgP4WRvkh +JJ/0ujlMMckgyTxRh8EEaJ35OnpXX7EdipoWhOMmiUnlPqye2icC8Y+CMdZsrod6 +nkoEQnXDCLf/Iv0qj7B9iKbxn7t3QDVxY4UILUReDuD8yrGULlGOl//aY/T3pkZ6 +R5trduZhI3o= +-----END CERTIFICATE----- From 0f0d32b073a998f8c7a8fdaf5202616068662b7a Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:20:42 -0700 Subject: [PATCH 3/9] v2http: move 'testdata' from 'etcdhttp' --- etcdserver/{etcdhttp => api/v2http}/testdata/ca.pem | 0 etcdserver/{etcdhttp => api/v2http}/testdata/otheruser.pem | 0 etcdserver/{etcdhttp => api/v2http}/testdata/user.pem | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename etcdserver/{etcdhttp => api/v2http}/testdata/ca.pem (100%) rename etcdserver/{etcdhttp => api/v2http}/testdata/otheruser.pem (100%) rename etcdserver/{etcdhttp => api/v2http}/testdata/user.pem (100%) diff --git a/etcdserver/etcdhttp/testdata/ca.pem b/etcdserver/api/v2http/testdata/ca.pem similarity index 100% rename from etcdserver/etcdhttp/testdata/ca.pem rename to etcdserver/api/v2http/testdata/ca.pem diff --git a/etcdserver/etcdhttp/testdata/otheruser.pem b/etcdserver/api/v2http/testdata/otheruser.pem similarity index 100% rename from etcdserver/etcdhttp/testdata/otheruser.pem rename to etcdserver/api/v2http/testdata/otheruser.pem diff --git a/etcdserver/etcdhttp/testdata/user.pem b/etcdserver/api/v2http/testdata/user.pem similarity index 100% rename from etcdserver/etcdhttp/testdata/user.pem rename to etcdserver/api/v2http/testdata/user.pem From 9510bd6036336f2e22222fd5b1aaaa6df79620aa Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:22:59 -0700 Subject: [PATCH 4/9] etcdserver: add 'ClientCertAuthEnabled' option --- etcdserver/config.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/etcdserver/config.go b/etcdserver/config.go index f7cfe8634..b66a63323 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -56,6 +56,9 @@ type ServerConfig struct { StrictReconfigCheck bool EnablePprof bool + + // ClientCertAuthEnabled is true when cert has been signed by the client CA. + ClientCertAuthEnabled bool } // VerifyBootstrap sanity-checks the initial config for bootstrap case From be001c44e8e550d8020b54c6ed85f64e5c36eaec Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:23:24 -0700 Subject: [PATCH 5/9] embed: set 'ClientCertAuthEnabled' --- embed/etcd.go | 1 + 1 file changed, 1 insertion(+) diff --git a/embed/etcd.go b/embed/etcd.go index 6f730aa06..b6f3cc19e 100644 --- a/embed/etcd.go +++ b/embed/etcd.go @@ -116,6 +116,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { QuotaBackendBytes: cfg.QuotaBackendBytes, StrictReconfigCheck: cfg.StrictReconfigCheck, EnablePprof: cfg.EnablePprof, + ClientCertAuthEnabled: cfg.ClientTLSInfo.ClientCertAuth, } if e.Server, err = etcdserver.NewServer(srvcfg); err != nil { From 68ece954fb975e8b0f4368739fe709ede849143d Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:23:41 -0700 Subject: [PATCH 6/9] v2http: add 'ClientCertAuthEnabled' in handlers --- etcdserver/api/v2http/client_auth.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/etcdserver/api/v2http/client_auth.go b/etcdserver/api/v2http/client_auth.go index 286dd3209..2b3278528 100644 --- a/etcdserver/api/v2http/client_auth.go +++ b/etcdserver/api/v2http/client_auth.go @@ -26,15 +26,16 @@ import ( ) type authHandler struct { - sec auth.Store - cluster api.Cluster + sec auth.Store + cluster api.Cluster + clientCertAuthEnabled bool } -func hasWriteRootAccess(sec auth.Store, r *http.Request) bool { +func hasWriteRootAccess(sec auth.Store, r *http.Request, clientCertAuthEnabled bool) bool { if r.Method == "GET" || r.Method == "HEAD" { return true } - return hasRootAccess(sec, r) + return hasRootAccess(sec, r, clientCertAuthEnabled) } func userFromBasicAuth(sec auth.Store, r *http.Request) *auth.User { @@ -74,7 +75,7 @@ func userFromClientCertificate(sec auth.Store, r *http.Request) *auth.User { return nil } -func hasRootAccess(sec auth.Store, r *http.Request) bool { +func hasRootAccess(sec auth.Store, r *http.Request, clientCertAuthEnabled bool) bool { if sec == nil { // No store means no auth available, eg, tests. return true @@ -84,7 +85,7 @@ func hasRootAccess(sec auth.Store, r *http.Request) bool { } var rootUser *auth.User - if r.Header.Get("Authorization") == "" { + if r.Header.Get("Authorization") == "" && clientCertAuthEnabled { rootUser = userFromClientCertificate(sec, r) if rootUser == nil { return false @@ -105,7 +106,7 @@ func hasRootAccess(sec auth.Store, r *http.Request) bool { return false } -func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive bool) bool { +func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive, clientCertAuthEnabled bool) bool { if sec == nil { // No store means no auth available, eg, tests. return true @@ -115,7 +116,7 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b } var user *auth.User - if r.Header.Get("Authorization") == "" { + if r.Header.Get("Authorization") == "" && clientCertAuthEnabled { user = userFromClientCertificate(sec, r) if user == nil { plog.Warningf("auth: no authorization provided, checking guest access") @@ -178,7 +179,7 @@ func (sh *authHandler) baseRoles(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } - if !hasRootAccess(sh.sec, r) { + if !hasRootAccess(sh.sec, r, sh.clientCertAuthEnabled) { writeNoAuth(w, r) return } @@ -242,7 +243,7 @@ func (sh *authHandler) forRole(w http.ResponseWriter, r *http.Request, role stri if !allowMethod(w, r.Method, "GET", "PUT", "DELETE") { return } - if !hasRootAccess(sh.sec, r) { + if !hasRootAccess(sh.sec, r, sh.clientCertAuthEnabled) { writeNoAuth(w, r) return } @@ -326,7 +327,7 @@ func (sh *authHandler) baseUsers(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } - if !hasRootAccess(sh.sec, r) { + if !hasRootAccess(sh.sec, r, sh.clientCertAuthEnabled) { writeNoAuth(w, r) return } @@ -398,7 +399,7 @@ func (sh *authHandler) forUser(w http.ResponseWriter, r *http.Request, user stri if !allowMethod(w, r.Method, "GET", "PUT", "DELETE") { return } - if !hasRootAccess(sh.sec, r) { + if !hasRootAccess(sh.sec, r, sh.clientCertAuthEnabled) { writeNoAuth(w, r) return } @@ -511,7 +512,7 @@ func (sh *authHandler) enableDisable(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "PUT", "DELETE") { return } - if !hasWriteRootAccess(sh.sec, r) { + if !hasWriteRootAccess(sh.sec, r, sh.clientCertAuthEnabled) { writeNoAuth(w, r) return } From 25aeeb35c304b24c7f033300689d70bf501ee9c5 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:24:15 -0700 Subject: [PATCH 7/9] v2http: set 'ClientCertAuthEnabled' in client.go --- etcdserver/api/v2http/client.go | 43 ++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/etcdserver/api/v2http/client.go b/etcdserver/api/v2http/client.go index c399e50af..af69b4809 100644 --- a/etcdserver/api/v2http/client.go +++ b/etcdserver/api/v2http/client.go @@ -65,11 +65,12 @@ func NewClientHandler(server *etcdserver.EtcdServer, timeout time.Duration) http sec := auth.NewStore(server, timeout) kh := &keysHandler{ - sec: sec, - server: server, - cluster: server.Cluster(), - timer: server, - timeout: timeout, + sec: sec, + server: server, + cluster: server.Cluster(), + timer: server, + timeout: timeout, + clientCertAuthEnabled: server.Cfg.ClientCertAuthEnabled, } sh := &statsHandler{ @@ -82,6 +83,7 @@ func NewClientHandler(server *etcdserver.EtcdServer, timeout time.Duration) http cluster: server.Cluster(), timeout: timeout, clock: clockwork.NewRealClock(), + clientCertAuthEnabled: server.Cfg.ClientCertAuthEnabled, } dmh := &deprecatedMachinesHandler{ @@ -89,8 +91,9 @@ func NewClientHandler(server *etcdserver.EtcdServer, timeout time.Duration) http } sech := &authHandler{ - sec: sec, - cluster: server.Cluster(), + sec: sec, + cluster: server.Cluster(), + clientCertAuthEnabled: server.Cfg.ClientCertAuthEnabled, } mux := http.NewServeMux() @@ -132,11 +135,12 @@ func NewClientHandler(server *etcdserver.EtcdServer, timeout time.Duration) http } type keysHandler struct { - sec auth.Store - server etcdserver.Server - cluster api.Cluster - timer etcdserver.RaftTimer - timeout time.Duration + sec auth.Store + server etcdserver.Server + cluster api.Cluster + timer etcdserver.RaftTimer + timeout time.Duration + clientCertAuthEnabled bool } func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -156,7 +160,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } // The path must be valid at this point (we've parsed the request successfully). - if !hasKeyPrefixAccess(h.sec, r, r.URL.Path[len(keysPrefix):], rr.Recursive) { + if !hasKeyPrefixAccess(h.sec, r, r.URL.Path[len(keysPrefix):], rr.Recursive, h.clientCertAuthEnabled) { writeKeyNoAuth(w) return } @@ -199,18 +203,19 @@ func (h *deprecatedMachinesHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } type membersHandler struct { - sec auth.Store - server etcdserver.Server - cluster api.Cluster - timeout time.Duration - clock clockwork.Clock + sec auth.Store + server etcdserver.Server + cluster api.Cluster + timeout time.Duration + clock clockwork.Clock + clientCertAuthEnabled bool } func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "POST", "DELETE", "PUT") { return } - if !hasWriteRootAccess(h.sec, r) { + if !hasWriteRootAccess(h.sec, r, h.clientCertAuthEnabled) { writeNoAuth(w, r) return } From 5066981cc75b654eb0a1d1fcf0c10ed3d2a1b8a2 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:24:33 -0700 Subject: [PATCH 8/9] v2http: test with 'ClientCertAuthEnabled' --- etcdserver/api/v2http/client_auth_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/etcdserver/api/v2http/client_auth_test.go b/etcdserver/api/v2http/client_auth_test.go index 8976148a9..b5e32c487 100644 --- a/etcdserver/api/v2http/client_auth_test.go +++ b/etcdserver/api/v2http/client_auth_test.go @@ -720,13 +720,13 @@ func TestPrefixAccess(t *testing.T) { } for i, tt := range table { - if tt.hasRoot != hasRootAccess(tt.store, tt.req) { + if tt.hasRoot != hasRootAccess(tt.store, tt.req, true) { t.Errorf("#%d: hasRoot doesn't match (expected %v)", i, tt.hasRoot) } - if tt.hasKeyPrefixAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, false) { + if tt.hasKeyPrefixAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, false, true) { t.Errorf("#%d: hasKeyPrefixAccess doesn't match (expected %v)", i, tt.hasRoot) } - if tt.hasRecursiveAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, true) { + if tt.hasRecursiveAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, true, true) { t.Errorf("#%d: hasRecursiveAccess doesn't match (expected %v)", i, tt.hasRoot) } } From 42db8f55b25d49c3e7b7050fd724e22a61cc7bb4 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 20 Jul 2016 16:55:45 -0700 Subject: [PATCH 9/9] e2e: test auth enabled with CN name cert --- e2e/ctl_v2_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ e2e/etcd_test.go | 19 ++++++++++++------- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/e2e/ctl_v2_test.go b/e2e/ctl_v2_test.go index 382a48013..5e7b8f790 100644 --- a/e2e/ctl_v2_test.go +++ b/e2e/ctl_v2_test.go @@ -276,6 +276,42 @@ func TestCtlV2Backup(t *testing.T) { // For https://github.com/coreos/etcd/issue } } +func TestCtlV2AuthWithCommonName(t *testing.T) { + defer testutil.AfterTest(t) + + copiedCfg := configClientTLS + copiedCfg.clientCertAuthEnabled = true + + epc := setupEtcdctlTest(t, &copiedCfg, false) + defer func() { + if err := epc.Close(); err != nil { + t.Fatalf("error closing etcd processes (%v)", err) + } + }() + + if err := etcdctlRoleAdd(epc, "testrole"); err != nil { + t.Fatalf("failed to add role (%v)", err) + } + if err := etcdctlRoleGrant(epc, "testrole", "--rw", "--path=/foo"); err != nil { + t.Fatalf("failed to grant role (%v)", err) + } + if err := etcdctlUserAdd(epc, "root", "123"); err != nil { + t.Fatalf("failed to add user (%v)", err) + } + if err := etcdctlUserAdd(epc, "Autogenerated CA", "123"); err != nil { + t.Fatalf("failed to add user (%v)", err) + } + if err := etcdctlUserGrant(epc, "Autogenerated CA", "testrole"); err != nil { + t.Fatalf("failed to grant role (%v)", err) + } + if err := etcdctlAuthEnable(epc); err != nil { + t.Fatalf("failed to enable auth (%v)", err) + } + if err := etcdctlSet(epc, "foo", "bar"); err != nil { + t.Fatalf("failed to write (%v)", err) + } +} + func etcdctlPrefixArgs(clus *etcdProcessCluster) []string { endpoints := "" if proxies := clus.proxies(); len(proxies) != 0 { @@ -348,6 +384,13 @@ func etcdctlRoleAdd(clus *etcdProcessCluster, role string) error { return spawnWithExpect(cmdArgs, role) } +func etcdctlRoleGrant(clus *etcdProcessCluster, role string, perms ...string) error { + cmdArgs := append(etcdctlPrefixArgs(clus), "role", "grant") + cmdArgs = append(cmdArgs, perms...) + cmdArgs = append(cmdArgs, role) + return spawnWithExpect(cmdArgs, role) +} + func etcdctlRoleList(clus *etcdProcessCluster, expectedRole string) error { cmdArgs := append(etcdctlPrefixArgs(clus), "role", "list") return spawnWithExpect(cmdArgs, expectedRole) diff --git a/e2e/etcd_test.go b/e2e/etcd_test.go index c7afd3aa7..d96e3e45f 100644 --- a/e2e/etcd_test.go +++ b/e2e/etcd_test.go @@ -149,13 +149,14 @@ type etcdProcessClusterConfig struct { snapCount int // default is 10000 - clientTLS clientConnType - isPeerTLS bool - isPeerAutoTLS bool - isClientAutoTLS bool - forceNewCluster bool - initialToken string - quotaBackendBytes int64 + clientTLS clientConnType + clientCertAuthEnabled bool + isPeerTLS bool + isPeerAutoTLS bool + isClientAutoTLS bool + forceNewCluster bool + initialToken string + quotaBackendBytes int64 } // newEtcdProcessCluster launches a new cluster from etcd processes, returning @@ -325,6 +326,10 @@ func (cfg *etcdProcessClusterConfig) tlsArgs() (args []string) { "--ca-file", caPath, } args = append(args, tlsClientArgs...) + + if cfg.clientCertAuthEnabled { + args = append(args, "--client-cert-auth") + } } }