veyron/services/identity: Adding CSRF protection to revocation interface in
identity server.
* Tokens are embedded into JS on page load.
* Server validates that token against the request's cookie.
* Server also checks that token matches one of the caveatIDs that the user can
revoke based on a map that is stored in memory.
Future:
* Consider a timeout on entries in the DirectoryStore.
* Ensure that tokens are refreshed frequently enough to ensure that this works.
Change-Id: If7ca60ed9cd2695592cf575657fe406dc29bbf7c
diff --git a/services/identity/googleoauth/handler.go b/services/identity/googleoauth/handler.go
index 96b0709..2ac5641 100644
--- a/services/identity/googleoauth/handler.go
+++ b/services/identity/googleoauth/handler.go
@@ -14,6 +14,7 @@
"encoding/base64"
"encoding/json"
"fmt"
+ "io/ioutil"
"net/http"
"path"
"strings"
@@ -40,6 +41,8 @@
// Prefix for the audit log from which data will be sourced.
// (auditor.ReadAuditLog).
Auditor string
+ // The RevocationManager is used to revoke blessings granted with a revocation caveat.
+ RevocationManager *revocation.RevocationManager
}
func (a *HandlerArgs) redirectURL() string {
@@ -61,7 +64,8 @@
// identity and bless it with the Google email address.
func NewHandler(args HandlerArgs) http.Handler {
config := NewOAuthConfig(args.ClientID, args.ClientSecret, args.redirectURL())
- return &handler{config, util.NewCSRFCop(), args.Auditor}
+ tokenMap := make(map[tokenCaveatIDKey]bool)
+ return &handler{config, util.NewCSRFCop(), args.Auditor, args.RevocationManager, tokenMap}
}
// NewOAuthConfig returns the oauth.Config required for obtaining just the email address from Google using OAuth 2.0.
@@ -76,10 +80,16 @@
}
}
+type tokenCaveatIDKey struct {
+ token, caveatID string
+}
+
type handler struct {
- config *oauth.Config
- csrfCop *util.CSRFCop
- auditor string
+ config *oauth.Config
+ csrfCop *util.CSRFCop
+ auditor string
+ revocationManager *revocation.RevocationManager
+ tokenRevocationCaveatMap map[tokenCaveatIDKey]bool
}
func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -88,13 +98,15 @@
h.auth(w, r)
case "oauth2callback":
h.callback(w, r)
+ case "revoke":
+ h.revoke(w, r)
default:
util.HTTPBadRequest(w, r, nil)
}
}
func (h *handler) auth(w http.ResponseWriter, r *http.Request) {
- csrf, err := h.csrfCop.NewToken(w, r)
+ csrf, err := h.csrfCop.NewToken(w, r, "VeyronHTTPIdentityClientID")
if err != nil {
vlog.Infof("Failed to create CSRF token[%v] for request %#v", err, r)
util.HTTPBadRequest(w, r, fmt.Errorf("Suspected automated request: %v", err))
@@ -104,7 +116,7 @@
}
func (h *handler) callback(w http.ResponseWriter, r *http.Request) {
- if err := h.csrfCop.ValidateToken(r.FormValue("state"), r); err != nil {
+ if err := h.csrfCop.ValidateToken(r.FormValue("state"), r, "VeyronHTTPIdentityClientID"); err != nil {
vlog.Infof("Invalid CSRF token: %v in request: %#v", err, r)
util.HTTPBadRequest(w, r, fmt.Errorf("Suspected request forgery: %v", err))
return
@@ -114,6 +126,13 @@
util.HTTPBadRequest(w, r, err)
return
}
+ // Create a new token to protect ensure that the revocation calls are protected.
+ csrf, err := h.csrfCop.NewToken(w, r, "VeyronHTTPIdentityRevocationID")
+ if err != nil {
+ vlog.Infof("Failed to create CSRF token[%v] for request %#v", err, r)
+ util.HTTPBadRequest(w, r, fmt.Errorf("Suspected automated request: %v", err))
+ return
+ }
type tmplentry struct {
Blessee security.PublicID
Start, End time.Time
@@ -121,11 +140,12 @@
RevocationCaveatID string
}
tmplargs := struct {
- Log chan tmplentry
- Email string
+ Log chan tmplentry
+ Email, CSRFToken string
}{
- Log: make(chan tmplentry),
- Email: email,
+ Log: make(chan tmplentry),
+ Email: email,
+ CSRFToken: csrf,
}
if entrych, err := auditor.ReadAuditLog(h.auditor, email); err != nil {
vlog.Errorf("Unable to read audit log: %v", err)
@@ -151,6 +171,10 @@
}
if blessEntry.RevocationCaveat != nil {
tmplentry.RevocationCaveatID = base64.URLEncoding.EncodeToString([]byte(blessEntry.RevocationCaveat.ID()))
+ // TODO(suharshs): Add a timeout that removes old entries to reduce storage space.
+ // TODO(suharshs): Make this map from CSRFToken to Email address, have
+ // revocation manager have map from caveatID to Email address in DirectoryStore.
+ h.tokenRevocationCaveatMap[tokenCaveatIDKey{csrf, tmplentry.RevocationCaveatID}] = true
}
// TODO(suharshs): Make the UI depend on where the caveatID exists and if it hasn't been revoked.
// Use the revocation manager IsRevoked function.
@@ -165,6 +189,68 @@
}
}
+func (h *handler) revoke(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ const (
+ success = `{"success": "true"}`
+ failure = `{"success": "false"}`
+ )
+ if h.revocationManager == nil {
+ vlog.Infof("no provided revocation manager")
+ w.Write([]byte(failure))
+ return
+ }
+
+ content, err := ioutil.ReadAll(r.Body)
+ if err != nil {
+ vlog.Infof("Failed to parse request: %s", err)
+ w.Write([]byte(failure))
+ return
+ }
+ var requestParams struct {
+ CaveatID, CSRFToken string
+ }
+ if err := json.Unmarshal(content, &requestParams); err != nil {
+ vlog.Infof("json.Unmarshal failed : %s", err)
+ w.Write([]byte(failure))
+ return
+ }
+
+ if err := h.validateTokenCaveatID(requestParams.CSRFToken, requestParams.CaveatID, r); err != nil {
+ vlog.Infof("failed to validate token for caveat: %s", err)
+ w.Write([]byte(failure))
+ return
+ }
+
+ decodedCaveatID, err := base64.URLEncoding.DecodeString(requestParams.CaveatID)
+ if err != nil {
+ vlog.Infof("base64 decoding failed: %s", err)
+ w.Write([]byte(failure))
+ return
+ }
+
+ caveatID := security.ThirdPartyCaveatID(string(decodedCaveatID))
+
+ if err := h.revocationManager.Revoke(caveatID); err != nil {
+ vlog.Infof("Revocation failed: %s", err)
+ w.Write([]byte(failure))
+ return
+ }
+
+ w.Write([]byte(success))
+ return
+}
+
+func (h *handler) validateTokenCaveatID(CSRFToken, revocationCaveatID string, r *http.Request) error {
+ if err := h.csrfCop.ValidateToken(CSRFToken, r, "VeyronHTTPIdentityRevocationID"); err != nil {
+ return fmt.Errorf("invalid CSRF token: %v in request: %#v", err, r)
+ }
+ if _, exists := h.tokenRevocationCaveatMap[tokenCaveatIDKey{CSRFToken, revocationCaveatID}]; exists {
+ return nil
+ }
+ return fmt.Errorf("this token has no matching entry for the provided caveat ID")
+}
+
// ExchangeAuthCodeForEmail exchanges the authorization code (which must
// have been obtained with scope=email) for an OAuth token and then uses Google's
// tokeninfo API to extract the email address from that token.
diff --git a/services/identity/googleoauth/template.go b/services/identity/googleoauth/template.go
index 4c03534..be8b525 100644
--- a/services/identity/googleoauth/template.go
+++ b/services/identity/googleoauth/template.go
@@ -41,16 +41,21 @@
// Setup the revoke buttons click events.
$(".revoke").click(function() {
- // TODO(suharshs): Implement "authorization" so that just making this post request with the URL doesn't work.
var revokeButton = $(this);
$.ajax({
- url: "/revoke/",
+ url: "/google/revoke",
type: "POST",
- data: $(this).val()
+ data: JSON.stringify({
+ "CaveatID": revokeButton.val(),
+ "CSRFToken": "{{.CSRFToken}}"
+ })
}).done(function(data) {
// TODO(suharshs): Have a fail message, add a strikethrough on the revoked caveats.
console.log(data)
revokeButton.remove()
+ }).fail(function(jqXHR, textStatus){
+ console.log(jqXHR)
+ console.log("The request failed :( :", textStatus)
});
});
});
diff --git a/services/identity/handlers/revoke.go b/services/identity/handlers/revoke.go
deleted file mode 100644
index ff3d5a4..0000000
--- a/services/identity/handlers/revoke.go
+++ /dev/null
@@ -1,50 +0,0 @@
-package handlers
-
-import (
- "encoding/base64"
- "fmt"
- "io/ioutil"
- "net/http"
- "veyron/services/identity/revocation"
-
- "veyron2/security"
-)
-
-// Revoke is an http.Handler implementation that revokes a Veyron PrivateID.
-type Revoke struct {
- RevocationManager *revocation.RevocationManager
-}
-
-// TODO(suharshs): Move this to the googleoauth handler to enable authorization on this.
-func (h Revoke) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- w.Header().Set("Content-Type", "application/json")
- const (
- success = `{"success": "true"}`
- failure = `{"success": "false"}`
- )
- // Get the caveat string from the request.
- // TODO(suharshs): Send a multi part form value from the client to server and parse it here.
- content, err := ioutil.ReadAll(r.Body)
- if err != nil {
- fmt.Printf("Failed to parse request: %s", err)
- w.Write([]byte(failure))
- return
- }
- decoded_caveatID, err := base64.URLEncoding.DecodeString(string(content))
- if err != nil {
- fmt.Printf("base64 decoding failed: %s", err)
- w.Write([]byte(failure))
- return
- }
-
- caveatID := security.ThirdPartyCaveatID(string(decoded_caveatID))
-
- if err := h.RevocationManager.Revoke(caveatID); err != nil {
- fmt.Printf("Revocation failed: %s", err)
- w.Write([]byte(failure))
- return
- }
-
- w.Write([]byte(success))
- return
-}
diff --git a/services/identity/identityd/main.go b/services/identity/identityd/main.go
index 68cebb2..20c40af 100644
--- a/services/identity/identityd/main.go
+++ b/services/identity/identityd/main.go
@@ -75,9 +75,6 @@
http.Handle("/random/", handlers.Random{r}) // mint identities with a random name
}
http.HandleFunc("/bless/", handlers.Bless) // use a provided PrivateID to bless a provided PublicID
- if revocationManager != nil {
- http.Handle("/revoke/", handlers.Revoke{revocationManager}) // revoke an identity that was provided by the server.
- }
// Google OAuth
ipcServer, ipcServerEP, err := setupGoogleBlessingDischargingServer(r, revocationManager)
@@ -90,10 +87,11 @@
if enabled, clientID, clientSecret := enableGoogleOAuth(*googleConfigWeb); enabled && len(*auditprefix) > 0 {
n := "/google/"
http.Handle(n, googleoauth.NewHandler(googleoauth.HandlerArgs{
- Addr: fmt.Sprintf("%s%s", httpaddress(), n),
- ClientID: clientID,
- ClientSecret: clientSecret,
- Auditor: *auditprefix,
+ Addr: fmt.Sprintf("%s%s", httpaddress(), n),
+ ClientID: clientID,
+ ClientSecret: clientSecret,
+ Auditor: *auditprefix,
+ RevocationManager: revocationManager,
}))
}
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
diff --git a/services/identity/revocation/revocation_manager.go b/services/identity/revocation/revocation_manager.go
index e37dc90..b988696 100644
--- a/services/identity/revocation/revocation_manager.go
+++ b/services/identity/revocation/revocation_manager.go
@@ -54,8 +54,7 @@
func (r *RevocationManager) IsRevoked(caveatID security.ThirdPartyCaveatID) bool {
token, err := r.caveatMap.Get(hex.EncodeToString([]byte(caveatID)))
if err == nil {
- exists, _ := revocationMap.Exists(token)
- return exists
+ return revocationMap.Exists(token)
}
return false
}
@@ -69,11 +68,7 @@
return fmt.Errorf("missing call to NewRevocationManager")
}
revocationLock.RUnlock()
- exists, err := revocationMap.Exists(hex.EncodeToString(cav[:]))
- if err != nil {
- return err
- }
- if exists {
+ if revocationMap.Exists(hex.EncodeToString(cav[:])) {
return fmt.Errorf("revoked")
}
return nil
diff --git a/services/identity/util/csrf.go b/services/identity/util/csrf.go
index ee7666f..516cf84 100644
--- a/services/identity/util/csrf.go
+++ b/services/identity/util/csrf.go
@@ -13,11 +13,13 @@
"veyron2/vlog"
)
+const cookieLen = 16
+
type CSRFCop struct{}
func NewCSRFCop() *CSRFCop { return new(CSRFCop) }
-func (*CSRFCop) maybeSetCookie(w http.ResponseWriter, req *http.Request) ([]byte, error) {
+func (*CSRFCop) maybeSetCookie(w http.ResponseWriter, req *http.Request, cookieName string) ([]byte, error) {
cookie, err := req.Cookie(cookieName)
switch err {
case nil:
@@ -30,7 +32,7 @@
default:
vlog.Infof("Error decoding cookie %q in request: %v. Regenerating one.", cookieName, err)
}
- cookie, v := newCookie()
+ cookie, v := newCookie(cookieName)
if cookie == nil || v == nil {
return nil, fmt.Errorf("failed to create cookie")
}
@@ -40,8 +42,8 @@
// NewToken creates an anti-cross-site-request-forgery, aka CSRF aka XSRF token.
// It returns an error if the token could not be created.
-func (c *CSRFCop) NewToken(w http.ResponseWriter, r *http.Request) (string, error) {
- cookie, err := c.maybeSetCookie(w, r)
+func (c *CSRFCop) NewToken(w http.ResponseWriter, r *http.Request, cookieName string) (string, error) {
+ cookie, err := c.maybeSetCookie(w, r, cookieName)
if err != nil {
return "", fmt.Errorf("bad cookie: %v", err)
}
@@ -57,7 +59,7 @@
// otherwise.
// The returned error should not be shown to end-users, it is meant for
// consumption of the server process only.
-func (c *CSRFCop) ValidateToken(token string, req *http.Request) error {
+func (c *CSRFCop) ValidateToken(token string, req *http.Request, cookieName string) error {
cookie, err := req.Cookie(cookieName)
if err != nil {
return err
@@ -83,12 +85,7 @@
Error error
}
-const (
- cookieName = "VeyronHTTPIdentityClientID"
- cookieLen = 16
-)
-
-func newCookie() (*http.Cookie, []byte) {
+func newCookie(cookieName string) (*http.Cookie, []byte) {
b := make([]byte, cookieLen)
if n, err := rand.Read(b); n != cookieLen || err != nil {
vlog.Errorf("newCookie failed: Read %d random bytes, wanted %d. err: %v", n, cookieLen, err)
diff --git a/services/identity/util/directory_store.go b/services/identity/util/directory_store.go
index ef722d5..7c93638 100644
--- a/services/identity/util/directory_store.go
+++ b/services/identity/util/directory_store.go
@@ -13,9 +13,9 @@
dir string
}
-func (s DirectoryStore) Exists(key string) (bool, error) {
+func (s DirectoryStore) Exists(key string) bool {
_, err := os.Stat(s.pathName(key))
- return !os.IsNotExist(err), nil
+ return !os.IsNotExist(err)
}
func (s DirectoryStore) Put(key, value string) error {
