Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

SigningMethodRSAPSS Validation issue #285

Closed
ymizrachi opened this issue Aug 8, 2018 · 2 comments · Fixed by #305
Closed

SigningMethodRSAPSS Validation issue #285

ymizrachi opened this issue Aug 8, 2018 · 2 comments · Fixed by #305

Comments

@ymizrachi
Copy link

ymizrachi commented Aug 8, 2018

Hey,
I'm trying to create JWS with PS256 algorithm and the signature i create is Invalid.

here is the code i try:

func GenerateSignaturePS256() (string,error) {

//Read RSA private key File.
	l_privateKey, err := ioutil.ReadFile("/<path_to_cert>/rsa.key") 
	if err != nil {
		return "", err
	}

//Read RSA public key File.
	l_publicKey, err := ioutil.ReadFile("/<path_to_cert>/rsa.crt")
	if err != nil {
		return "", err
	}

//Remove First Line and Last Line From public Key RSA certificate.
	l_pubRmPad := strings.TrimPrefix(string(l_publicKey), "-----BEGIN CERTIFICATE-----\n")
	l_pubRmPad = strings.TrimRight(l_pubRmPad, "\n-----END CERTIFICATE-----")


	token := jws.NewWithClaims(jws.SigningMethodPS256, jws.MapClaims{
		"data":         "My Data",
	})

	token.Header["x5c"] = []string{l_pubRmPad}

	var rsaPSSKey *rsa.PrivateKey
	if rsaPSSKey, err = jws.ParseRSAPrivateKeyFromPEM(l_privateKey); err != nil {
		return "", err
	}
	// Sign and get the complete encoded token as a string using the secret
	tokenString, err := token.SignedString(rsaPSSKey)
	fmt.Println(token.Valid, tokenString)

return tokenString, err
}

The Print Result :

false eyJhbGciOiJQUzI1NiIsInR5cCI6IkpXVCIsIng1YyI6WyJNSUlHZURDQ0JHQ2dBd0lCQWdJR0FXUkdUZ1QyTUEwR0NTcUdTSWIzRFFFQkN3VUFNSHd4Q3pBSkJnTlZCQVlUQWs1TU1Ta3dKd1lEVlFRS0RDQlZUQ0JVY21GdWMyRmpkR2x2YmlCVFpXTjFjbWwwZVNCa2FYWnBjMmx2YmpFZ01CNEdBMVVFQ3d3WFZVd2dWRk1nTTBRdFUyVmpkWEpsSUZKUFQxUWdRMEV4SURBZUJnTlZCQU1NRjFWTUlGUlRJRE5FTFZObFkzVnlaU0JTVDA5VUlFTkJNQjRYRFRFNE1EWXlPREExTVRFeE5Wb1hEVEU1TURZeU9EQTFNVEV4TlZvd2F6RUxNQWtHQTFVRUJoTUNTVXd4RVRBUEJnTlZCQWdNQ0VoaFRXVnlZMkY2TVJJd0VBWURWUVFIREFsU1lXMWhkQ0JIWVc0eERUQUxCZ05WQkFvTUJFZEJWRVV4RERBS0JnTlZCQXNNQTJSbGRqRVlNQllHQTFVRUF3d1BaR1YyTG5SdmEyVnVhV1F1WTI5dE1JSUNJakFOQmdrcWhraUc5dzBCQVFFRkFBT0NBZzhBTUlJQ0NnS0NBZ0VBcVVzbCtjU24rWCtzZTY4ei9MTVp5Q0hyNStxRzRMSlArb1FrdGduZmJ1WngzcGZ4QnVKcWwybkNOM2M3eERQOVhmc3E4WFlUTW9jWUQwM3ZtUGZ4cW1ETjRWRTFnK2dVQkMydm5wR1RlM3VvMDVjV1JFd2E2ZVRXNnBFQ0oxbjdCU1kwU0ZuN0VZZDQvZjZkVE1YSmpITFkrdnhsa09PNXZNM2dDSVdFbStMd0tNVGx5MGpIcnZ4R2VEYjlYbERTNFVWSHU1ZmxpNjF2Q1YwaDc4KzIwMklnTDFuK3BsSWR1dE11SGZIOTRoakRqbFlqcGh3YTlCUDRGVDNKR2NEL3hXbDdjR2MxQ1pOVVFZM09udVdnZ3BrVjJ1LzY4MThlY0xxQXZxT1hPREp1cVZuQlhFWEZodVZtaWNpMXNVdmh2emxUMjJTVWh6OEt2L3pEYTVPZ3VRbDRpd1BSK3JDL1FHZzF5N3Rta0ozZ2NaSU5vTHoyNWFQejJzYXJ4NExtZTh0WEpaV2tUeU9lLzhCN1VBaitKNHpZTUJkYkQrcHVXR2hRUWt1SC9sb2xWeHpKcHhVdFlWdHVxYjNxVFpWdU94WDdvbFhhMExVc0NEUUNMWGljd3F3YlViN0pBU0xrenR2SmU2ZS9qbVd2VG54NFd3Q1JFMjN2b3VKYjYxNGVFSkxSSTJHUUg4NFM5TnJ6NE1CSjRtbDZXN1NsZERvUUtOUXlzSjR5MytuNTBwSkpyT0V1TDcwREYzdHdIN3ZXVzlkbE9FeUJ4OWxTQ3dWNVZyem1TNFJLYlNjSmt1bWh0T1B0VnVrNlhuQ1lSVGVWS28xWmo0cU5sS0UreGE4dzZ4WUVYWG9qQWF2VTNIY25seHFMRXlrLzJkTG5yem0xRXhLNFBHYUM4c2NDQXdFQUFhT0NBUTh3Z2dFTE1JR3ZCZ05WSFNNRWdhY3dnYVNBRkpoMmE0YUxnbXpBWURPYkFJZ3g3OUx3RzN2eW9ZR0FwSDR3ZkRFTE1Ba0dBMVVFQmhNQ1Rrd3hLVEFuQmdOVkJBb01JRlZNSUZSeVlXNXpZV04wYVc5dUlGTmxZM1Z5YVhSNUlHUnBkbWx6YVc5dU1TQXdIZ1lEVlFRTERCZFZUQ0JVVXlBelJDMVRaV04xY21VZ1VrOVBWQ0JEUVRFZ01CNEdBMVVFQXd3WFZVd2dWRk1nTTBRdFUyVmpkWEpsSUZKUFQxUWdRMEdDQ1FDVEw3M3JJNWc4b1RBZEJnTlZIUTRFRmdRVW91UzZtYkRpVWE4aGxCZGgxKzJUclZBVTlTMHdDUVlEVlIwVEJBSXdBREFPQmdOVkhROEJBZjhFQkFNQ0JlQXdIUVlEVlIwbEJCWXdGQVlJS3dZQkJRVUhBd0VHQ0NzR0FRVUZCd01DTUEwR0NTcUdTSWIzRFFFQkN3VUFBNElDQVFCdTQrNElsNzNIQnV1LzdCcmNQVFRLcUR0YXdFRHB1Zm95QmV1akpSalk0SW4vYjlUd001M0EyTFpOYmhQQlF2SHVLajlsaDMxVTRnWEh3a0dhT1JhSkdtcmd4Nmk1UmxnUmVuQkZJc0dTN0pXS3lqSEF3a3ZRVGg0SHZIajY5bGxhdVRCbk14UkdyeUsrV0N3djh6Z1lPY28xRzg3WmtoYWdMMVZXZzFoRXJqWDkzTjFJblExdU1mcXVERTFJVEZyWGkwcmpBdEVpbjkvaGNsNjMzUjkwQUl1bTdSWXFUb0pwRlJRVTNCcVgvNkVTcFp4b295Q1RLNVNWUzJyRkVIU0tqQ3pya3kxZ2RDaEFEdVNSbXFhMWVjSml1ZGNrSllzeWtyeDVzbDhjSWlTakdMbWRNSisrSU9rWVFOTWtSazNmb0hIOHM1TDRpczBob0pDSG1zUFhEVkNpYVdVSUlYbUdvcUtLME5WKzNzK3g5c2FwbWFTMm9RN2RJYmk3M0NHNHg3RENONmRoeWlhK01NQi8wK0J4Zmh4R0JNcm5lL2xWNWJlMG1JUUZ5WHFWeW4yaW51em9tU1owd1RIVjMwVGRldm9sWFArYUF0T1ZtUWhqVVEvbHVrOWRoNzduSEpqSTl4SnA5cnBVR0hzZ3RSVG1lY0xEaTUwc0NpV3ppdm9uMy9sRjVxQUU1c0N5d0hFaUN6aHJIV2lnTklLWHpjVUxDSDRBWjU4L1lNOEZTZnRtc0VYcUlxQ0hSMU9KZ2dmUHNiM1QwSzdteDdWR0MxSDJpejZ3ZDhQMi91UER1NEcrSU1Nb1V0ZW8vV2VGeXNJVjc5SitxV0pKZ2lCaEZmemM4dUozLzN6SmxoWGM0Z24rMnBBT3dPUlo4bmVvU2twTkZLS1ozRjIyMGc9PSJdfQ.eyJkYXRhIjoiTXkgRGF0YSJ9.m_snagatlRJukNLTBlQeBZLELVs7ky-2FvcVzVzQcKPJCQGkh8WWoWP_2kdTizL3gY_oxp41-bj1v1V33BkZ18d6JsSmUpokGTiqjn6EVsBmptNVt3S-bGqgYzJ-gkV5H5AH98E0a7N5fhnxGE1y-uHwYiPbnxxCnS3iieqGKd_gpg-KxutGBycAufB-NqmTrb7X1LyaqUH2UR2m489v9QFQZ6FzM0gIGlOylijM4GGv8nt_4k4QnFsrBRsWXnfrGumdKR8w_oiN8wqA40txviaGWlSXZ2UpSqvQuIcsqaLwENDq4HTv0k9R4ju-A3p0MGyCM013IZ_2tg5Iavx4ebbZIdQaTv7USWSU1-QJdbK67pN9tYKQmYAsFOjkc5sR-5GXdHccL7ZzfP0_KYXDU_dHqHuELwraSySZyxaRv0Y9AisM0jEcbuK0fIab6OmtPWPuANzdHZ3rWJPs4eueWL_wmdG0JjmaY1gWnI6QIasi_UREHSaWQuLB3ZUV7AhWF-Vcx68PBmQ-76dAQCS2TCHx8Vzc6z6ae8tsh2CihtjgjXK-4FdoPKRAJqeTBywHK0O7hl3M1BHAaDsDrJvS9exEUc2gXO845q4Tg_KT68cxgN9qj6h2WpcKh5H-UPrb-wHrGJaYBM6qP1S8Fyb30D91FsLM7-ncVYo7QdzrY3o

I try to use PyJWT package to create JWS with the "PS256" alg and the same arguments in the header and the signature is Valid.

if my code in go is incorrect please help me understand the wrong part.
can it be a bug in the SigningMethodRSAPSS implementation?
i tried to validate the JWS given in "rsa_pss_test.go" too and their signature Invalid either

Thank you,
Yaniv

@longsleep
Copy link

The problem is that jwt-go uses the wrong parameters to create the RSA-PSS signature (wrong salt size).

See https://www.ietf.org/mail-archive/web/jose/current/msg02905.html which says that the salt size shoult be the same as the hash size. Unfortunately this is not really anywhere specified.

The jwt-go uses auto (like for example in https://github.com/dgrijalva/jwt-go/blob/master/rsa_pss.go#L32) by default.

Luckily Options is exposed and the value can be set to rsa.PSSSaltLengthEqualsHash which then makes the Go created RSA-PSS signatures valid with other libraries (like PyJWT and oidc-client-js).

@skipor
Copy link
Contributor

skipor commented Nov 9, 2018

RFC-7518 says:

The size of the salt value is the same size as the hash function output.

But now, rsa.PSSSaltLengthAuto used.
Now all tokens signed by PS256, PS384, PS512, have invalid signature due to https://jwt.io, pyjwtPython implementation, and, seems, many other.

package jwt
var (
	SigningMethodPS256 = &SigningMethodRSAPSS{
		&SigningMethodRSA{
			Name: "PS256",
			Hash: crypto.SHA256,
		},
		&rsa.PSSOptions{
			SaltLength: rsa.PSSSaltLengthAuto,
			Hash:       crypto.SHA256,
		},
	}
)
....
package rsa
const (
	// PSSSaltLengthAuto causes the salt in a PSS signature to be as large
	// as possible when signing, and to be auto-detected when verifying.
	PSSSaltLengthAuto = 0
        // PSSSaltLengthEqualsHash causes the salt length to equal the length
	// of the hash used in the signature.
	PSSSaltLengthEqualsHash = -1
)

There is an example how to make fixed SigningMethod and how it behaves compared to not fixed:

package main

import (
	"crypto/rsa"
	"fmt"
	"strings"
	"time"

	"github.com/dgrijalva/jwt-go"
	"github.com/dgrijalva/jwt-go/test"
)

func main() {
	// Invalid signature on jwt.io: https://bit.ly/2FfYHLr
	before := makeToken(jwt.SigningMethodPS256)
	fmt.Printf("Before: %s\nAccepted before: %v\nAccepted after fix: %v\n",
		before, verify(jwt.SigningMethodPS256, before), verify(fixedSigningMethodPS256, before),
	)
	fmt.Println()
	// Valid signature on jwt.io: https://bit.ly/2FfYHLr (print some spaces to Encoded field to refresh signature status)
	after := makeToken(fixedSigningMethodPS256)
	fmt.Printf("After: %s\nAccepted before: %v\nAccepted after fix: %v\n",
		after, verify(jwt.SigningMethodPS256, after), verify(fixedSigningMethodPS256, after),
	)
}

func makeToken(method jwt.SigningMethod) string {
	token := jwt.NewWithClaims(method, jwt.StandardClaims{
		Issuer:   "example",
		IssuedAt: time.Now().Unix(),
	})
	privateKey := test.LoadRSAPrivateKeyFromDisk("test/sample_key")
	signed, err := token.SignedString(privateKey)
	if err != nil {
		panic(err)
	}
	return signed
}

var fixedSigningMethodPS256 = &jwt.SigningMethodRSAPSS{
	SigningMethodRSA: jwt.SigningMethodPS256.SigningMethodRSA,
	Options: &rsa.PSSOptions{
		SaltLength: rsa.PSSSaltLengthEqualsHash,
	},
}

func verify(signingMethod jwt.SigningMethod, token string) bool {
	segments := strings.Split(token, ".")
	err := signingMethod.Verify(strings.Join(segments[:2], "."), segments[2], test.LoadRSAPublicKeyFromDisk("test/sample_key.pub"))
	return err == nil
}

Outputs:

Before: eyJhbGciOiJQUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE1NDE3ODcxNDMsImlzcyI6ImV4YW1wbGUifQ.WFiwtxwV6s0JwP2s-E669JMpQ5t9J1WbAfRZiYk-zIZNlnr4SvSWWqtSAfH1Q991m27hF6NbedmTb2zwV3zoTiJTMQl0rLP2Gc_y77J8rEEElcvgRIxX8SqE9lyzOmHu5wWqw4WKXDbQXu6FATMeFqH6ZY55Dxwg3gfNOveZ2jG5NyEyhmvN5_mDSNWaJJWGHcbw2MA56Tr3uLbCPcAdzocuD8cnpj5WCSULKv_kyoS4czi4p2Fai9KsXKm9dLaimt0XySTJZ2pqyU8XyQGZiPD3W5zyXMhTxQyiObHWUO92hJ5danAFWQFAynydgs0AE4-mH4J43SAqopMdYUl5Gw
Accepted before: true
Accepted after fix: false

After: eyJhbGciOiJQUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE1NDE3ODcxNDMsImlzcyI6ImV4YW1wbGUifQ.WdQoZRSm4RPHyE9u4BPrSCZnX6VOMcJ8S7UOKesfuWVbK-g7izPwU8PwvWK2C3T0lqMREMmwQdj-kF1KzBXuthF5PGzLDqSkyyD21ARbZCs-Sp7TzTs0cm2WIKSQS9CejCaw8TkOzdziVhflJL6rwkRdtuNuUW_Xvl9mkZlVxIL267UMsvBYtx33JwzpCg3KgH3HiH3rkY2XFSUS4RBQ1ZL2ijIAhBEWV-x2DTTG2XR1hwPenTmkrbbkcHI9yvt70DLbObejrPgbn0x0JK1vWj8MZyBChaiupVagFTSorGHaP_H-ZVCrKUYRvtAWjMhWif97hJmgs5wfwETk5r-h2g
Accepted before: true
Accepted after fix: true

Seems that just changing salt length will cause validation errors from non-patched versions.
Fixed version should generate tokens with correct length, but accept previous salt length too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants