Add asymmetric JWT signing #16010

lunny · 2021-05-28T20:12:26Z

KN4CK3R commented

2021-05-28 20:12:26 +00:00

(Migrated from github.com)

Added asymmetric JWT signing. Supports HS256 (already implemented), HS384, HS512, RS256, RS384, RS512, ES256, ES384 and ES512.

The default signing algorithm is changed from HS256 (symmetric encryption) to RS256 (asymmetric encryption).

⚠️ BREAKING ⚠️

Asymmetric algorithms require a secret asymmetric key pair to be in JWT_SIGNING_PRIVATE_KEY_FILE (by default APP_DATA_PATH/jwt), a pair will be generated if it is not present. (NB: this was originally in CUSTOM_PATH but was changed by #16227)
The original symmetric JWT_SECRET will only be used if JWT_SIGNING_ALGORITHM is set to HS256 (previous default), HS384 or HS512.
As a result of the change of algorithm gitea OAuth2 tokens (and potentially client secrets) will need to be regenerated unless you change your JWT_SIGNING_ALGORITHM back to HS256.
Legacy installations of Drone assume a short key length, however as no automated migrations are run by Drone you'll need to do them manually if the column type for user_oauth_token and user_oauth_refresh are limited to 500 characters:

-- an example manual migration for Drone database connected to postgresql
alter table users alter column user_oauth_token type bytea using convert_to(user_oauth_token, 'LATIN1');
alter table users alter column user_oauth_refresh type bytea using convert_to(user_oauth_refresh, 'LATIN1');

or see: https://github.com/go-gitea/gitea/pull/16010#issuecomment-879381265

Close #15912 Added asymmetric JWT signing. Supports `HS256` (already implemented), `HS384`, `HS512`, `RS256`, `RS384`, `RS512`, `ES256`, `ES384` and `ES512`. The default signing algorithm is changed from `HS256` (symmetric encryption) to `RS256` (asymmetric encryption). ## ⚠️ BREAKING ⚠️ * Asymmetric algorithms require a secret asymmetric key pair to be in `JWT_SIGNING_PRIVATE_KEY_FILE` (by default `APP_DATA_PATH/jwt`), a pair will be generated if it is not present. (NB: this was originally in `CUSTOM_PATH` but was changed by #16227) * The original symmetric `JWT_SECRET` will only be used if `JWT_SIGNING_ALGORITHM` is set to `HS256` (previous default), `HS384` or `HS512`. * As a result of the change of algorithm gitea OAuth2 tokens (and potentially client secrets) will need to be regenerated unless you change your `JWT_SIGNING_ALGORITHM` back to `HS256`. * Legacy installations of Drone assume a short key length, however as no automated migrations are run by Drone you'll need to do them manually if the column type for user_oauth_token and user_oauth_refresh are limited to 500 characters: ```sql -- an example manual migration for Drone database connected to postgresql alter table users alter column user_oauth_token type bytea using convert_to(user_oauth_token, 'LATIN1'); alter table users alter column user_oauth_refresh type bytea using convert_to(user_oauth_refresh, 'LATIN1'); ``` or see: https://github.com/go-gitea/gitea/pull/16010#issuecomment-879381265

lunny reviewed 2021-05-30 11:18:34 +00:00

models/oauth2.go

						
				@ -132,6 +132,9 @@ func GetActiveOAuth2Providers() ([]string, map[string]OAuth2Provider, error) {

				// InitOAuth2 initialize the OAuth2 lib and register all active OAuth2 providers in the library

				func InitOAuth2() error {

					if err := oauth2.InitSigningKey(); err != nil {

lunny commented

2021-05-30 11:18:34 +00:00

Merge it into oauth2.Init is better?

KN4CK3R (Migrated from github.com) reviewed 2021-05-30 18:12:25 +00:00

models/oauth2.go

						
				@ -132,6 +132,9 @@ func GetActiveOAuth2Providers() ([]string, map[string]OAuth2Provider, error) {

				// InitOAuth2 initialize the OAuth2 lib and register all active OAuth2 providers in the library

				func InitOAuth2() error {

					if err := oauth2.InitSigningKey(); err != nil {

KN4CK3R (Migrated from github.com) commented

2021-05-30 18:12:24 +00:00

I did not add it in the existing method because oauth2.Init is used for the other direction (Gitea -> other service). So I thought both methods should be logical separated.

I did not add it in the existing method because `oauth2.Init` is used for the other direction (Gitea -> other service). So I thought both methods should be logical separated.

techknowlogick reviewed 2021-05-31 06:19:41 +00:00

docs/content/doc/advanced/config-cheat-sheet.en-us.md

techknowlogick commented

2021-05-31 06:19:41 +00:00

per discussion in chat, these changed lines are fine due to the fact that it cleans up line endings (shakes fist at windows)

techknowlogick requested changes 2021-05-31 06:48:58 +00:00

techknowlogick left a comment

Thank you so much so far on your work on this PR!!

I was able to sign into Sourcegraph using this PR, however testing this PR more I found that 1. sourcegraph may not be using the cert to validate the signature on this token, 2. possibly the upstream jwt library may not be signing the token correctly.

Thank you so much so far on your work on this PR!! I was able to sign into Sourcegraph using this PR, however testing this PR more I found that 1. sourcegraph may not be using the cert to validate the signature on this token, 2. possibly the upstream jwt library may not be signing the token correctly.

models/oauth2_application.go

						
				@ -587,3 +588,2 @@

					token.IssuedAt = time.Now().Unix()

					jwtToken := jwt.NewWithClaims(jwt.SigningMethodHS256, token)

					return jwtToken.SignedString([]byte(clientSecret))

					jwtToken := jwt.NewWithClaims(signingKey.SigningMethod(), token)

techknowlogick commented

2021-05-31 06:45:06 +00:00

Attempting to verify the generated token using JWT.io, it is unable to parse the signature. It works w/ HS256, but not RS256. I suspect it is something upstream in github.com/dgrijalva/jwt-go that is signing it differently than other libraries expect. I've tried to dig down into the code changes from this PR, but it looks like everything you are setting is correct.

Please let me know if you require any additional information from me to replicate.

Appendix:
You can also see at https://replit.com/@techknowlogick/BruisedCompetitiveLightweightprocess attempting to parse the key, that in headers it is unable to return KeyID (using smallstep CLI to fetch example token from google step oauth --oidc --bare, you can see that KeyID is populated).
Sample RS256 token:

eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiI5M2NiNDM5OS04Mzc4LTQ5NGYtYTQwNy1mYzVjODIzYTJjOTIiLCJleHAiOjE2MjI0NDUxMjcsImlhdCI6MTYyMjQ0MTUyNywiaXNzIjoiaHR0cHM6Ly90a2xrLWlubGV0cy5oZXJva3VhcHAuY29tLyIsInN1YiI6IjEiLCJub25jZSI6IjhmZWFlYmYxMzg0MzIwZDNmOTU4YTMxODE0MTQ1NWI5NDk2ZTg4NDA1NTNlZjk1NmVjODcyNGJiYTMzYzQ5MmIifQ.HfksMJPGgjhEP9UJqPulwPvh02VPTgmNkgAxGUdIn8PKiWbPcCkfT2yedFAMccduCqg3FQxLT_LEhPB0T8zuVl4wXdPS-t_qfGem1KE3sZubVdzy2PAA3Iya76kokbk5ufcfnt1bYpGp0p8sCGDm7oOma3N2yvsrXIWIC7EIzPzVICdD7a89ZFMC1TVw2zK_MKU9pUFYcuTrSfrb_Sx5OzpT3DxSMFJ0qfrVIdyYGlwbsujdxaBN2d1BcHv94Ur68FBxxbXoBqggIL7_gOZHmR8ENbRD1D8yJTTqk0pCjR42CqxaosEIv-vqFpLt92J_ZJCr4ldvA0EuVO9HhdjcaDPHrVVW4eh__QOI2aLLTuOcnM165LN5uGt5QN13Y9Ws6rU6inmhycPEqvdK11-o25Y3KlqwgKxNhJLyX-Ctr1Va1y5VZNNK6Oa30bF-_Mb4T92qELMoXthHi5d-wpiiwgURO2xfltxoBrfop0cA7-eCbQ2PPgYZ8oxkXCVGLtlwbcTQXEtzhpu7pz_YBptVtZtnGpqtQqXLcA_afDWuK7KE8xI3vgflSErFwx_QTPF8J_CPnf21IxXp5AoFNUcfZ-k0g7YxnpbX1tNctYGisieTIbW2Jp5iGODdAEKTzsb638jzrz3sOsE1ty01_IrpMkyfHbcbJYeA5SWiL1AA9l8

Attempting to verify the generated token using JWT.io, it is unable to parse the signature. It works w/ HS256, but not RS256. I suspect it is something upstream in github.com/dgrijalva/jwt-go that is signing it differently than other libraries expect. I've tried to dig down into the code changes from this PR, but it looks like everything you are setting is correct. Please let me know if you require any additional information from me to replicate. Appendix: You can also see at https://replit.com/@techknowlogick/BruisedCompetitiveLightweightprocess attempting to parse the key, that in headers it is unable to return KeyID (using smallstep CLI to fetch example token from google `step oauth --oidc --bare`, you can see that KeyID is populated). Sample RS256 token: ``` eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiI5M2NiNDM5OS04Mzc4LTQ5NGYtYTQwNy1mYzVjODIzYTJjOTIiLCJleHAiOjE2MjI0NDUxMjcsImlhdCI6MTYyMjQ0MTUyNywiaXNzIjoiaHR0cHM6Ly90a2xrLWlubGV0cy5oZXJva3VhcHAuY29tLyIsInN1YiI6IjEiLCJub25jZSI6IjhmZWFlYmYxMzg0MzIwZDNmOTU4YTMxODE0MTQ1NWI5NDk2ZTg4NDA1NTNlZjk1NmVjODcyNGJiYTMzYzQ5MmIifQ.HfksMJPGgjhEP9UJqPulwPvh02VPTgmNkgAxGUdIn8PKiWbPcCkfT2yedFAMccduCqg3FQxLT_LEhPB0T8zuVl4wXdPS-t_qfGem1KE3sZubVdzy2PAA3Iya76kokbk5ufcfnt1bYpGp0p8sCGDm7oOma3N2yvsrXIWIC7EIzPzVICdD7a89ZFMC1TVw2zK_MKU9pUFYcuTrSfrb_Sx5OzpT3DxSMFJ0qfrVIdyYGlwbsujdxaBN2d1BcHv94Ur68FBxxbXoBqggIL7_gOZHmR8ENbRD1D8yJTTqk0pCjR42CqxaosEIv-vqFpLt92J_ZJCr4ldvA0EuVO9HhdjcaDPHrVVW4eh__QOI2aLLTuOcnM165LN5uGt5QN13Y9Ws6rU6inmhycPEqvdK11-o25Y3KlqwgKxNhJLyX-Ctr1Va1y5VZNNK6Oa30bF-_Mb4T92qELMoXthHi5d-wpiiwgURO2xfltxoBrfop0cA7-eCbQ2PPgYZ8oxkXCVGLtlwbcTQXEtzhpu7pz_YBptVtZtnGpqtQqXLcA_afDWuK7KE8xI3vgflSErFwx_QTPF8J_CPnf21IxXp5AoFNUcfZ-k0g7YxnpbX1tNctYGisieTIbW2Jp5iGODdAEKTzsb638jzrz3sOsE1ty01_IrpMkyfHbcbJYeA5SWiL1AA9l8 ```

KN4CK3R (Migrated from github.com) reviewed 2021-05-31 09:15:02 +00:00

models/oauth2_application.go

						
				@ -587,3 +588,2 @@

					token.IssuedAt = time.Now().Unix()

					jwtToken := jwt.NewWithClaims(jwt.SigningMethodHS256, token)

					return jwtToken.SignedString([]byte(clientSecret))

					jwtToken := jwt.NewWithClaims(signingKey.SigningMethod(), token)

KN4CK3R (Migrated from github.com) commented

2021-05-31 09:15:02 +00:00

JWT.io works for me without changes. Did you change the "Algorithm" on top?

I used this container to get the token:

version: '3.5'

services:
  oidc-test-client:
    image: beryju/oidc-test-client
    ports:
      - 9009:9009
    network_mode: bridge
    environment:
      OIDC_CLIENT_ID: id
      OIDC_CLIENT_SECRET: secret
      OIDC_PROVIDER: gitea-endpoint
      OIDC_DO_INTROSPECTION: "false"

I then used
openssl rsa -in private.pem -out rsa_priv.pem
and
openssl rsa -in rsa_priv.pem -pubout > rsa_pub.pem
to get the private and public key in the desired format.

JWT.io works for me without changes. Did you change the "Algorithm" on top? ![grafik](https://user-images.githubusercontent.com/1666336/120169415-37fc4400-c200-11eb-884d-4d8b6332a2f7.png) I used this container to get the token: ``` version: '3.5' services: oidc-test-client: image: beryju/oidc-test-client ports: - 9009:9009 network_mode: bridge environment: OIDC_CLIENT_ID: id OIDC_CLIENT_SECRET: secret OIDC_PROVIDER: gitea-endpoint OIDC_DO_INTROSPECTION: "false" ``` I then used `openssl rsa -in private.pem -out rsa_priv.pem` and `openssl rsa -in rsa_priv.pem -pubout > rsa_pub.pem` to get the private and public key in the desired format.

KN4CK3R (Migrated from github.com) reviewed 2021-05-31 14:21:12 +00:00

modules/auth/oauth2/jwtsigningkey.go

						
				@ -0,0 +296,4 @@

				func loadOrCreateSymmetricKey() (interface{}, error) {

					key := make([]byte, 32)

					n, err := base64.RawURLEncoding.Decode(key, []byte(setting.OAuth2.JWTSecretBase64))

					if err != nil || n != 32 {

KN4CK3R (Migrated from github.com) commented

2021-05-31 14:21:12 +00:00

Currently only 32 bytes are allowed. Should we really enforce this? Smaller keys are valid and larger keys are hashed by the internal hashing algorithm to create a secret with the correct length.
2ebe77a2fd/src/crypto/hmac/hmac.go (L148-L152)

Currently only 32 bytes are allowed. Should we really enforce this? Smaller keys are valid and larger keys are hashed by the internal hashing algorithm to create a secret with the correct length. https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/crypto/hmac/hmac.go#L148-L152

techknowlogick approved these changes 2021-06-13 19:01:49 +00:00

jtran (Migrated from github.com) reviewed 2021-06-16 16:38:40 +00:00

modules/auth/oauth2/jwtsigningkey.go

jtran (Migrated from github.com) commented

2021-06-16 16:38:39 +00:00

This is the first instance, but there are lots of misspellings of "signing".

type hmacSigningKey struct {

This is the first instance, but there are lots of misspellings of "signing". ```suggestion type hmacSigningKey struct { ```

6543 (Migrated from github.com) reviewed 2021-06-16 21:35:34 +00:00

modules/setting/setting.go

6543 (Migrated from github.com) commented

2021-06-16 21:35:33 +00:00

should we realy drop this?

KN4CK3R (Migrated from github.com) reviewed 2021-06-16 21:37:32 +00:00

modules/setting/setting.go

KN4CK3R (Migrated from github.com) commented

2021-06-16 21:37:32 +00:00

It's not dropped. You can find it in loadOrCreateSymmetricKey

It's not dropped. You can find it in `loadOrCreateSymmetricKey`

6543 (Migrated from github.com) reviewed 2021-06-16 21:47:34 +00:00

modules/setting/setting.go

6543 (Migrated from github.com) commented

2021-06-16 21:47:34 +00:00

so it's created on demand ... I would prefer if it get added on first gitea start ...

KN4CK3R (Migrated from github.com) reviewed 2021-06-16 21:51:53 +00:00

modules/setting/setting.go

KN4CK3R (Migrated from github.com) commented

2021-06-16 21:51:53 +00:00

It gets added on first gitea start if you use HSxxx as signing algorithm. If you use another algorithm it is not used at all.

It gets added on first gitea start if you use `HSxxx` as signing algorithm. If you use another algorithm it is not used at all.

6543 (Migrated from github.com) reviewed 2021-06-17 21:02:08 +00:00

modules/setting/setting.go

						
				@ -380,2 +381,4 @@

						RefreshTokenExpirationTime: 730,

						InvalidateRefreshTokens:    false,

						JWTSigningAlgorithm:        "RS256",

						JWTSigningPrivateKeyFile:   "jwt/private.pem",

6543 (Migrated from github.com) commented

2021-06-17 21:02:08 +00:00

this change default alg to async - not sure if it is breaking ...

6543 (Migrated from github.com) reviewed 2021-06-17 21:03:26 +00:00

modules/setting/setting.go

						
				@ -380,2 +381,4 @@

						RefreshTokenExpirationTime: 730,

						InvalidateRefreshTokens:    false,

						JWTSigningAlgorithm:        "RS256",

						JWTSigningPrivateKeyFile:   "jwt/private.pem",

6543 (Migrated from github.com) commented

2021-06-17 21:03:26 +00:00

		JWTSigningAlgorithm:        "HS256",

```suggestion JWTSigningAlgorithm: "HS256", ```

6543 (Migrated from github.com) reviewed 2021-06-17 21:20:13 +00:00

modules/setting/setting.go

						
				@ -380,2 +381,4 @@

						RefreshTokenExpirationTime: 730,

						InvalidateRefreshTokens:    false,

						JWTSigningAlgorithm:        "RS256",

						JWTSigningPrivateKeyFile:   "jwt/private.pem",

6543 (Migrated from github.com) commented

2021-06-17 21:20:13 +00:00

as per KN4CK3R ...
https://accounts.google.com/.well-known/openid-configuration and other big tech have RS256 - so go for it

as per KN4CK3R ... `https://accounts.google.com/.well-known/openid-configuration` and other big tech have **RS256** - so go for it

KN4CK3R (Migrated from github.com) reviewed 2021-06-17 21:25:55 +00:00

modules/setting/setting.go

						
				@ -380,2 +381,4 @@

						RefreshTokenExpirationTime: 730,

						InvalidateRefreshTokens:    false,

						JWTSigningAlgorithm:        "RS256",

						JWTSigningPrivateKeyFile:   "jwt/private.pem",

KN4CK3R (Migrated from github.com) commented

2021-06-17 21:25:55 +00:00

For clarification:
RS256 is defined as "create the signature with SHA256 and use the provided RSA key as secret". The RSA key is allowed to have any size and not just 256 bit. If Gitea generates a key the default size is 4096 bit.

For clarification: RS256 is defined as "create the signature with SHA**256** and use the provided RSA key as secret". The RSA key is allowed to have any size and **not** just 256 bit. If Gitea generates a key the default size is 4096 bit.