oauth2: Resolves client secrets from potentially leaking to the datab…

…ase in cleartext (ory#820)

This release resolves a security issue (reported by [platform.sh](https://www.platform.sh)) related to the fosite storage implementation in this project. Fosite used to pass all of the request body from both authorize and token endpoints to the storage adapters. As some of these values are needed in consecutive requests, the storage adapter of this project chose to drop all of the key/value pairs to the database in plaintext.

This implied that confidential parameters, such as the `client_secret` which can be passed in the request body since fosite version 0.15.0, were stored as key/value pairs in plaintext in the database. While most client secrets are generated programmatically (as opposed to set by the user) and most popular OAuth2 providers choose to store the secret in plaintext for later retrieval, we see it as a considerable security issue nonetheless.

The issue has been resolved by sanitizing the request body and only including those values truly required by their respective handlers. This also implies that typos (eg `client_secet`) won't "leak" to the database.

There are no special upgrade paths required for this version.

This issue does not apply to you if you do not use an SQL backend. If you do upgrade to this version, you need to run `hydra migrate sql path://to.your/database`.

If your users use POST body client authentication, it might
be a good move to remove old data. There are multiple ways of doing that. **Back up your data before you do this**:

1. **Radical solution:** Drop all rows from tables `hydra_oauth2_refresh`, `hydra_oauth2_access`, `hydra_oauth2_oidc`,
`hydra_oauth2_code`. This implies that all your users have to re-authorize.
2. **Sensitive solution:** Replace all values in column `form_data` in tables `hydra_oauth2_refresh`, `hydra_oauth2_access` with
an empty string. This will keep all authorization sessions alive. Tables `hydra_oauth2_oidc` and `hydra_oauth2_code`
do not contain sensitive information, unless your users accidentally sent the client_secret to the `/oauth2/auth` endpoint.

We would like to thank [platform.sh](https://www.platform.sh) for sponsoring the development of a patch that resolves this issue.

Gopkg.toml

@@ -75,7 +75,7 @@
 
 [[constraint]]
   name = "github.com/ory/fosite"
-  version = "=0.16.2"
+  version = "0.17.0"
 
 [[constraint]]
   name = "github.com/ory/graceful"

README.md

@@ -60,7 +60,7 @@ able to securely manage JSON Web Keys, and has a sophisticated policy-based acce
   - [Develop](#develop)
 - [Reception](#reception)
 - [Libraries and third-party projects](#libraries-and-third-party-projects)
-- [Blog posts & articles](#blog-posts--articles)
+- [Blog posts & articles](#blog-posts-&-articles)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 

UPGRADE.md

@@ -10,6 +10,7 @@ before finalizing the upgrade process.
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 
 
+- [0.11.11](#01111)
 - [0.11.3](#0113)
 - [0.11.0](#0110)
 - [0.10.0](#0100)
@@ -54,6 +55,38 @@ before finalizing the upgrade process.
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
+## 0.11.11
+
+This release resolves a security issue (reported by [platform.sh](https://www.platform.sh)) related to the fosite
+storage implementation in this project. Fosite used to pass all of the request body from both authorize and token
+endpoints to the storage adapters. As some of these values are needed in consecutive requests, the storage adapter
+of this project chose to drop all of the key/value pairs to the database in plaintext.
+
+This implied that confidential parameters, such as the `client_secret` which can be passed in the request body since
+fosite version 0.15.0, were stored as key/value pairs in plaintext in the database. While most client secrets are generated
+programmatically (as opposed to set by the user) and most popular OAuth2 providers choose to store the secret in plaintext
+for later retrieval, we see it as a considerable security issue nonetheless.
+
+The issue has been resolved by sanitizing the request body and only including those values truly required by their
+respective handlers. This also implies that typos (eg `client_secet`) won't "leak" to the database.
+
+There are no special upgrade paths required for this version.
+
+This issue does not apply to you if you do not use an SQL backend. If you do upgrade to this version, you need to run
+`hydra migrate sql path://to.your/database`.
+
+If your users use POST body client authentication, it might
+be a good move to remove old data. There are multiple ways of doing that. **Back up your data before you do this**:
+
+1. **Radical solution:** Drop all rows from tables `hydra_oauth2_refresh`, `hydra_oauth2_access`, `hydra_oauth2_oidc`,
+`hydra_oauth2_code`. This implies that all your users have to re-authorize.
+2. **Sensitive solution:** Replace all values in column `form_data` in tables `hydra_oauth2_refresh`, `hydra_oauth2_access` with
+an empty string. This will keep all authorization sessions alive. Tables `hydra_oauth2_oidc` and `hydra_oauth2_code`
+do not contain sensitive information, unless your users accidentally sent the client_secret to the `/oauth2/auth` endpoint.
+
+We would like to thank [platform.sh](https://www.platform.sh) for sponsoring the development of a patch that resolves this
+issue.
+
 ## 0.11.3
 
 The experimental endpoint `/health/metrics` has been removed as it caused various issues such as increased memory usage,

docker-compose.yml

@@ -58,7 +58,7 @@ services:
       - HYDRA_CLIENT_ID=admin
       - HYDRA_CLIENT_SECRET=demo-password
       - NODE_TLS_REJECT_UNAUTHORIZED=0
-    image: oryd/hydra-consent-app-express:v0.10.0-alpha.7
+    image: oryd/hydra-consent-app-express:v0.11.0
     links:
       - hydra
     ports:

oauth2/fosite_store_sql.go

@@ -50,6 +50,16 @@ func sqlSchemaUp(table string, id string) string {
 	session_data  	text NOT NULL
 )`, table),
 		"2": fmt.Sprintf("ALTER TABLE hydra_oauth2_%s ADD subject varchar(255) NOT NULL DEFAULT ''", table),
+		"3": `CREATE TABLE IF NOT EXISTS hydra_oauth2_pkce (
+	signature      	varchar(255) NOT NULL PRIMARY KEY,
+	request_id  	varchar(255) NOT NULL,
+	requested_at  	timestamp NOT NULL DEFAULT now(),
+	client_id  		text NOT NULL,
+	scope  			text NOT NULL,
+	granted_scope 	text NOT NULL,
+	form_data  		text NOT NULL,
+	session_data  	text NOT NULL
+)`,
 	}
 
 	return schemas[id]
@@ -59,6 +69,7 @@ func sqlSchemaDown(table string, id string) string {
 	schemas := map[string]string{
 		"1": fmt.Sprintf(`DROP TABLE %s)`, table),
 		"2": fmt.Sprintf("ALTER TABLE hydra_oauth2_%s DROP COLUMN subject", table),
+		"3": "DROP TABLE hydra_oauth2_pkce",
 	}
 
 	return schemas[id]
@@ -69,6 +80,7 @@ const (
 	sqlTableAccess  = "access"
 	sqlTableRefresh = "refresh"
 	sqlTableCode    = "code"
+	sqlTablePKCE    = "pkce"
 )
 
 var migrations = &migrate.MemoryMigrationSource{
@@ -103,6 +115,15 @@ var migrations = &migrate.MemoryMigrationSource{
 				sqlSchemaDown(sqlTableOpenID, "2"),
 			},
 		},
+		{
+			Id: "3",
+			Up: []string{
+				sqlSchemaUp(sqlTablePKCE, "3"),
+			},
+			Down: []string{
+				sqlSchemaDown(sqlTablePKCE, "3"),
+			},
+		},
 	},
 }
 
@@ -281,6 +302,18 @@ func (s *FositeSQLStore) DeleteRefreshTokenSession(_ context.Context, signature
 	return s.deleteSession(signature, sqlTableRefresh)
 }
 
+func (s *FositeSQLStore) CreatePKCERequestSession(_ context.Context, signature string, requester fosite.Requester) error {
+	return s.createSession(signature, requester, sqlTablePKCE)
+}
+
+func (s *FositeSQLStore) GetPKCERequestSession(_ context.Context, signature string, session fosite.Session) (fosite.Requester, error) {
+	return s.findSessionBySignature(signature, session, sqlTablePKCE)
+}
+
+func (s *FositeSQLStore) DeletePKCERequestSession(_ context.Context, signature string) error {
+	return s.deleteSession(signature, sqlTablePKCE)
+}
+
 func (s *FositeSQLStore) CreateImplicitAccessTokenSession(ctx context.Context, signature string, requester fosite.Requester) error {
 	return s.CreateAccessTokenSession(ctx, signature, requester)
 }

oauth2/fosite_store_test.go

@@ -111,3 +111,10 @@ func TestRevokeRefreshToken(t *testing.T) {
 		t.Run(fmt.Sprintf("case=%s", k), TestHelperRevokeRefreshToken(m))
 	}
 }
+
+func TestPKCEReuqest(t *testing.T) {
+	t.Parallel()
+	for k, m := range clientManagers {
+		t.Run(fmt.Sprintf("case=%s", k), TestHelperCreateGetDeletePKCERequestSession(m))
+	}
+}

oauth2/fosite_store_test_helpers.go

@@ -149,3 +149,24 @@ func TestHelperCreateGetDeleteAccessTokenSession(m pkg.FositeStorer) func(t *tes
 		assert.NotNil(t, err)
 	}
 }
+
+func TestHelperCreateGetDeletePKCERequestSession(m pkg.FositeStorer) func(t *testing.T) {
+	return func(t *testing.T) {
+		ctx := context.Background()
+		_, err := m.GetAccessTokenSession(ctx, "4321", &fosite.DefaultSession{})
+		assert.NotNil(t, err)
+
+		err = m.CreateAccessTokenSession(ctx, "4321", &defaultRequest)
+		require.NoError(t, err)
+
+		res, err := m.GetAccessTokenSession(ctx, "4321", &fosite.DefaultSession{})
+		require.NoError(t, err)
+		AssertObjectKeysEqual(t, &defaultRequest, res, "Scopes", "GrantedScopes", "Form", "Session")
+
+		err = m.DeleteAccessTokenSession(ctx, "4321")
+		require.NoError(t, err)
+
+		_, err = m.GetAccessTokenSession(ctx, "4321", &fosite.DefaultSession{})
+		assert.NotNil(t, err)
+	}
+}