Skip to content

Commit

Permalink
feat: stateless authorization code flow (#3515)
Browse files Browse the repository at this point in the history
This patch optimizes the performance of authorization code grant flows by minimizing the number of database queries. We acheive this by storing the flow in an AEAD-encoded cookie and AEAD-encoded request parameters for the authentication and consent screens. 

BREAKING CHANGE:

* The client that is used as part of the authorization grant flow is stored in the AEAD-encoding. Therefore, running flows will not observe updates to the client after they were started.
* Because the login and consent challenge values now include the AEAD-encoded flow, their size increased to around 1kB for a flow without any metadata (and increases linearly with the amount of metadata). Please adjust your ingress / gateway accordingly.
  • Loading branch information
hperl authored Jun 12, 2023
1 parent fa8c053 commit f29fe3a
Show file tree
Hide file tree
Showing 115 changed files with 3,697 additions and 1,546 deletions.
2 changes: 0 additions & 2 deletions .docker-home/.gitignore

This file was deleted.

2 changes: 1 addition & 1 deletion .docker/Dockerfile-build
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.19-alpine3.18 AS builder
FROM golang:1.20-alpine3.17 AS builder

RUN apk -U --no-cache --upgrade --latest add build-base git gcc bash

Expand Down
2 changes: 1 addition & 1 deletion .docker/Dockerfile-hsm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.19-alpine3.18 AS builder
FROM golang:1.20-alpine3.18 AS builder

RUN apk -U --no-cache --upgrade --latest add build-base git gcc bash

Expand Down
1 change: 0 additions & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
docs
node_modules
.circleci
.docker-home
.github
scripts
sdk/js
Expand Down
21 changes: 10 additions & 11 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ jobs:
# We must fetch at least the immediate parents so that if this is
# a pull request then we can checkout the head.
fetch-depth: 2
- uses: actions/setup-go@v2
- uses: actions/setup-go@v3
with:
go-version: "1.19"
go-version: "1.20"
- name: Start service
run: ./test/conformance/start.sh
- name: Run tests
Expand Down Expand Up @@ -80,22 +80,21 @@ jobs:
path: |
internal/httpclient
key: ${{ needs.sdk-generate.outputs.sdk-cache-key }}
- uses: actions/setup-go@v2
- uses: actions/setup-go@v4
with:
go-version: "1.19"
go-version: "1.20"
- run: go list -json > go.list
- name: Run nancy
uses: sonatype-nexus-community/nancy-github-action@v1.0.2
with:
nancyVersion: v1.0.42
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v2
uses: golangci/golangci-lint-action@v3
env:
GOGC: 100
with:
args: --timeout 10m0s
version: v1.47.3
skip-go-installation: true
version: v1.53.2
skip-pkg-cache: true
- name: Run go-acc (tests)
run: |
Expand Down Expand Up @@ -124,9 +123,9 @@ jobs:
path: |
internal/httpclient
key: ${{ needs.sdk-generate.outputs.sdk-cache-key }}
- uses: actions/setup-go@v2
- uses: actions/setup-go@v3
with:
go-version: "1.19"
go-version: "1.20"
- name: Setup HSM libs and packages
run: |
sudo apt install -y softhsm opensc
Expand Down Expand Up @@ -175,9 +174,9 @@ jobs:
docker start cockroach
name: Start CockroachDB
- uses: ory/ci/checkout@master
- uses: actions/setup-go@v2
- uses: actions/setup-go@v3
with:
go-version: "1.19"
go-version: "1.20"
- uses: actions/cache@v2
with:
path: ./test/e2e/hydra
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: 1.19
go-version: "1.20"
- run: make format
- name: Indicate formatting issues
run: git diff HEAD --exit-code --color
6 changes: 3 additions & 3 deletions .github/workflows/licenses.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ jobs:
check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: "1.18"
go-version: "1.20"
- uses: actions/setup-node@v2
with:
node-version: "18"
Expand Down
4 changes: 1 addition & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ linters:
- gosimple
- bodyclose
- staticcheck
# Disabled due to Go 1.19 changes and Go-Swagger incompatibility
# https://github.com/ory/hydra/issues/3227
# - goimports
- goimports
disable:
- ineffassign
- deadcode
Expand Down
2 changes: 2 additions & 0 deletions .grype.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ignore:
- vulnerability: CVE-2023-2650
1 change: 1 addition & 0 deletions .trivyignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CVE-2023-2650
16 changes: 6 additions & 10 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ export PATH := .bin:${PATH}
export PWD := $(shell pwd)
export IMAGE_TAG := $(if $(IMAGE_TAG),$(IMAGE_TAG),latest-sqlite)

GOLANGCI_LINT_VERSION = 1.46.2
GOLANGCI_LINT_VERSION = 1.53.2

GO_DEPENDENCIES = github.com/ory/go-acc \
github.com/golang/mock/mockgen \
golang.org/x/tools/cmd/goimports \
github.com/go-swagger/go-swagger/cmd/swagger

define make-go-dependency
Expand Down Expand Up @@ -37,9 +38,6 @@ node_modules: package-lock.json
docs/cli: .bin/clidoc
clidoc .

.bin/goimports: go.sum Makefile
GOBIN=$(shell pwd)/.bin go install golang.org/x/tools/cmd/goimports@latest

.bin/licenses: Makefile
curl https://raw.githubusercontent.com/ory/ci/master/licenses/install | sh

Expand All @@ -63,12 +61,9 @@ test: .bin/go-acc
# Resets the test databases
.PHONY: test-resetdb
test-resetdb: node_modules
docker kill hydra_test_database_mysql || true
docker kill hydra_test_database_postgres || true
docker kill hydra_test_database_cockroach || true
docker rm -f hydra_test_database_mysql || true
docker rm -f hydra_test_database_postgres || true
docker rm -f hydra_test_database_cockroach || true
docker rm --force --volumes hydra_test_database_mysql || true
docker rm --force --volumes hydra_test_database_postgres || true
docker rm --force --volumes hydra_test_database_cockroach || true
docker run --rm --name hydra_test_database_mysql --platform linux/amd64 -p 3444:3306 -e MYSQL_ROOT_PASSWORD=secret -d mysql:8.0.26
docker run --rm --name hydra_test_database_postgres --platform linux/amd64 -p 3445:5432 -e POSTGRES_PASSWORD=secret -e POSTGRES_DB=postgres -d postgres:11.8
docker run --rm --name hydra_test_database_cockroach --platform linux/amd64 -p 3446:26257 -d cockroachdb/cockroach:v22.1.10 start-single-node --insecure
Expand Down Expand Up @@ -122,6 +117,7 @@ sdk: .bin/swagger .bin/ory node_modules
swagger generate spec -m -o spec/swagger.json \
-c github.com/ory/hydra/v2/client \
-c github.com/ory/hydra/v2/consent \
-c github.com/ory/hydra/v2/flow \
-c github.com/ory/hydra/v2/health \
-c github.com/ory/hydra/v2/jwk \
-c github.com/ory/hydra/v2/oauth2 \
Expand Down
28 changes: 28 additions & 0 deletions aead/aead.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright © 2023 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package aead

import (
"context"

"github.com/ory/fosite"
)

// Cipher provides AEAD (authenticated encryption with associated data). The
// ciphertext is returned base64url-encoded.
type Cipher interface {
// Encrypt encrypts and encodes the given plaintext, optionally using
// additiona data.
Encrypt(ctx context.Context, plaintext, additionalData []byte) (ciphertext string, err error)

// Decrypt decodes, decrypts, and verifies the plaintext and additional data
// from the ciphertext. The ciphertext must be given in the form as returned
// by Encrypt.
Decrypt(ctx context.Context, ciphertext string, additionalData []byte) (plaintext []byte, err error)
}

type Dependencies interface {
fosite.GlobalSecretProvider
fosite.RotatedGlobalSecretsProvider
}
154 changes: 154 additions & 0 deletions aead/aead_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
// Copyright © 2022 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package aead_test

import (
"context"
"crypto/rand"
"fmt"
"io"
"testing"

"github.com/ory/hydra/v2/aead"
"github.com/ory/hydra/v2/driver/config"
"github.com/ory/hydra/v2/internal"

"github.com/pborman/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func secret(t *testing.T) string {
bytes := make([]byte, 32)
_, err := io.ReadFull(rand.Reader, bytes)
require.NoError(t, err)
return fmt.Sprintf("%X", bytes)
}

func TestAEAD(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
new func(aead.Dependencies) aead.Cipher
}{
{"AES-GCM", func(d aead.Dependencies) aead.Cipher { return aead.NewAESGCM(d) }},
{"XChaChaPoly", func(d aead.Dependencies) aead.Cipher { return aead.NewXChaCha20Poly1305(d) }},
} {
tc := tc

t.Run("cipher="+tc.name, func(t *testing.T) {
NewCipher := tc.new

t.Run("case=without-rotation", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
a := NewCipher(c)

plain := []byte(uuid.New())
ct, err := a.Encrypt(ctx, plain, nil)
assert.NoError(t, err)

ct2, err := a.Encrypt(ctx, plain, nil)
assert.NoError(t, err)
assert.NotEqual(t, ct, ct2, "ciphertexts for the same plaintext must be different each time")

res, err := a.Decrypt(ctx, ct, nil)
assert.NoError(t, err)
assert.Equal(t, plain, res)
})

t.Run("case=wrong-secret", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
a := NewCipher(c)

ct, err := a.Encrypt(ctx, []byte(uuid.New()), nil)
require.NoError(t, err)

c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
_, err = a.Decrypt(ctx, ct, nil)
require.Error(t, err)
})

t.Run("case=with-rotation", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
old := secret(t)
c.MustSet(ctx, config.KeyGetSystemSecret, []string{old})
a := NewCipher(c)

plain := []byte(uuid.New())
ct, err := a.Encrypt(ctx, plain, nil)
require.NoError(t, err)

// Sets the old secret as a rotated secret and creates a new one.
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t), old})
res, err := a.Decrypt(ctx, ct, nil)
require.NoError(t, err)
assert.Equal(t, plain, res)

// THis should also work when we re-encrypt the same plain text.
ct2, err := a.Encrypt(ctx, plain, nil)
require.NoError(t, err)
assert.NotEqual(t, ct2, ct)

res, err = a.Decrypt(ctx, ct, nil)
require.NoError(t, err)
assert.Equal(t, plain, res)
})

t.Run("case=with-rotation-wrong-secret", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
a := NewCipher(c)

plain := []byte(uuid.New())
ct, err := a.Encrypt(ctx, plain, nil)
require.NoError(t, err)

// When the secrets do not match, an error should be thrown during decryption.
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t), secret(t)})
_, err = a.Decrypt(ctx, ct, nil)
require.Error(t, err)
})

t.Run("suite=with additional data", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
c.MustSet(ctx, config.KeyGetSystemSecret, []string{secret(t)})
a := NewCipher(c)

plain := []byte(uuid.New())
ct, err := a.Encrypt(ctx, plain, []byte("additional data"))
assert.NoError(t, err)

t.Run("case=additional data matches", func(t *testing.T) {
res, err := a.Decrypt(ctx, ct, []byte("additional data"))
assert.NoError(t, err)
assert.Equal(t, plain, res)
})

t.Run("case=additional data does not match", func(t *testing.T) {
res, err := a.Decrypt(ctx, ct, []byte("wrong data"))
assert.Error(t, err)
assert.Nil(t, res)
})

t.Run("case=missing additional data", func(t *testing.T) {
res, err := a.Decrypt(ctx, ct, nil)
assert.Error(t, err)
assert.Nil(t, res)
})
})
})
}
}
Loading

0 comments on commit f29fe3a

Please sign in to comment.