Skip to content

Commit

Permalink
Actually verify signature on client assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
dehall committed Mar 20, 2024
1 parent 5db4e46 commit efcb5b1
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 2 deletions.
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@
<artifactId>java-jwt</artifactId>
<version>3.8.3</version>
</dependency>
<dependency>
<groupId>com.auth0</groupId>
<artifactId>jwks-rsa</artifactId>
<version>0.22.1</version>
</dependency>

<!-- https://mvnrepository.com/artifact/org.json/json -->
<dependency>
Expand Down
22 changes: 22 additions & 0 deletions src/main/java/org/mitre/fhir/HapiReferenceServerProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public class HapiReferenceServerProperties {
private static final String CONFIDENTIAL_CLIENT_ID_KEY = "inferno.confidential_client_id";
private static final String CONFIDENTIAL_CLIENT_SECRET_KEY = "inferno.confidential_client_secret";
private static final String BULK_CLIENT_ID = "inferno.bulk_client_id";
private static final String ASYMMETRIC_CLIENT_ID = "inferno.asymmetric_client_id";
private static final String ASYMMETRIC_CLIENT_JWKS = "inferno.asymmetric_client_jwks";
private static final String GROUP_ID = "inferno.group_id";
private static final String RESOURCES_FOLDER = "inferno.resources_folder";

Expand Down Expand Up @@ -296,6 +298,26 @@ public String getBulkClientId() {
return bulkClientId;
}

/**
* Returns the Asymmetric Client ID Property.
*
* @return the property
*/
public String getAsymmetricClientId() {
String asymmetricClientId = properties.getProperty(ASYMMETRIC_CLIENT_ID);
return asymmetricClientId;
}

/**
* Returns the Asymmetric Client JWKS Property.
*
* @return the property
*/
public String getAsymmetricClientJwks() {
String asymmetricClientJwks = properties.getProperty(ASYMMETRIC_CLIENT_JWKS);
return asymmetricClientJwks;
}

/**
* Returns the Group Id Property.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import com.auth0.jwk.InvalidPublicKeyException;
import com.auth0.jwk.Jwk;
import com.auth0.jwk.JwkException;
import com.auth0.jwk.JwkProvider;
import com.auth0.jwk.SigningKeyNotFoundException;
import com.auth0.jwk.UrlJwkProvider;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.SignatureVerificationException;
import com.auth0.jwt.interfaces.DecodedJWT;
import com.github.dnault.xmlpatch.internal.Log;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPrivateKey;
import java.security.interfaces.RSAPublicKey;
import java.util.ArrayList;
Expand Down Expand Up @@ -274,9 +283,50 @@ private static String validateClient(HttpServletRequest request, String clientId
clientId = clientIdRequestParam;
}
} else if (JWT_BEARER_CLIENT_ASSERTION_TYPE.equals(clientAssertionType)) {
// confindential asymmetric
// confidential asymmetric
DecodedJWT decodedJwt = JWT.decode(clientAssertion);
clientId = decodedJwt.getIssuer();

try {
// In this case we cache the JWKS file locally, but
// this verification is normally done against the registered JWKS for the given client, eg:
// JwkProvider provider = new UrlJwkProvider("https://inferno.healthit.gov/suites/custom/smart_stu2/");
HapiReferenceServerProperties properties = new HapiReferenceServerProperties();
URL jwks = AuthorizationController.class.getResource(properties.getAsymmetricClientJwks());
JwkProvider provider = new UrlJwkProvider(jwks);
Jwk jwk = provider.get(decodedJwt.getKeyId());
Algorithm algorithm;
if (decodedJwt.getAlgorithm().equals("RS384")) {
algorithm = Algorithm.RSA384((RSAPublicKey) jwk.getPublicKey(), null);
} else if (decodedJwt.getAlgorithm().equals("ES384")) {
algorithm = Algorithm.ECDSA384((ECPublicKey) jwk.getPublicKey(), null);
} else {
// the above are the only 2 options supported in the SMART app launch test kit.
// if more are added, report support for them in the WellKnownAuthorizationEndpointController
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
"Unsupported encryption method " + decodedJwt.getAlgorithm());
}

algorithm.verify(decodedJwt);
} catch (SignatureVerificationException e) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
"Client Assertion JWT failed signature verification", e);
} catch (SigningKeyNotFoundException e) {
// thrown by provider.get(jwt.kid) above
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
"No key found with kid " + decodedJwt.getKeyId(), e);
} catch (InvalidPublicKeyException e) {
// thrown by jwk.getPublicKey above, should never happen
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR,
"Failed to parse public key", e);
} catch (JwkException e) {
// thrown by provider.get(jwt.kid) above,
// shouldn't be possible in practice as the method only throws
// the more specific SigningKeyNotFound
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR,
"Unknown error occurred", e);
}

} else {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
"Unexpected Client Assertion Type: " + clientAssertionType);
Expand Down Expand Up @@ -554,7 +604,8 @@ private static String getDecodedBasicAuthorizationString(String basicHeader) {
private static void authorizeClientId(String clientId) {
HapiReferenceServerProperties properties = new HapiReferenceServerProperties();
if (!properties.getPublicClientId().equals(clientId)
&& !properties.getConfidentialClientId().equals(clientId)) {
&& !properties.getConfidentialClientId().equals(clientId)
&& !properties.getAsymmetricClientId().equals(clientId)) {
throw new InvalidClientIdException(clientId);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.IOException;
import java.security.interfaces.RSAPublicKey;
import java.util.Base64;
import java.util.List;
import javax.annotation.PostConstruct;
import javax.servlet.http.HttpServletRequest;
import org.json.JSONArray;
Expand Down Expand Up @@ -95,6 +96,9 @@ public String getWellKnownJson(HttpServletRequest theRequest) {
FhirReferenceServerUtils.getFhirServerBaseUrl(theRequest) + "/.well-known/jwk");
wellKnownJson.put(WELL_KNOWN_INTROSPECTION_ENDPOINT_KEY,
ServerConformanceWithAuthorizationProvider.getIntrospectExtensionUri(theRequest));
wellKnownJson.put("token_endpoint_auth_methods_supported", List.of("private_key_jwt"));
wellKnownJson.put("token_endpoint_auth_signing_alg_values_supported",
List.of("RS384", "ES384"));

return wellKnownJson.toString();
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/hapi.properties
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ inferno.public_client_id=SAMPLE_PUBLIC_CLIENT_ID
inferno.confidential_client_id=SAMPLE_CONFIDENTIAL_CLIENT_ID
inferno.confidential_client_secret=SAMPLE_CONFIDENTIAL_CLIENT_SECRET
inferno.bulk_client_id=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6InJlZ2lzdHJhdGlvbi10b2tlbiJ9.eyJqd2tzX3VybCI6Imh0dHA6Ly8xMC4xNS4yNTIuNzMvaW5mZXJuby8ud2VsbC1rbm93bi9qd2tzLmpzb24iLCJhY2Nlc3NUb2tlbnNFeHBpcmVJbiI6MTUsImlhdCI6MTU5NzQxMzE5NX0.q4v4Msc74kN506KTZ0q_minyapJw0gwlT6M_uiL73S4
inferno.asymmetric_client_id=SAMPLE_ASYMMETRIC_CLIENT_ID
inferno.asymmetric_client_jwks=/inferno_client_jwks.json
inferno.group_id=64fdf2a5-ebad-4ed0-a512-567970843d49
inferno.resources_folder=./resources
29 changes: 29 additions & 0 deletions src/main/resources/inferno_client_jwks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"keys": [
{
"kty": "EC",
"crv": "P-384",
"x": "JQKTsV6PT5Szf4QtDA1qrs0EJ1pbimQmM2SKvzOlIAqlph3h1OHmZ2i7MXahIF2C",
"y": "bRWWQRJBgDa6CTgwofYrHjVGcO-A7WNEnu4oJA5OUJPPPpczgx1g2NsfinK-D2Rw",
"use": "sig",
"key_ops": [
"verify"
],
"ext": true,
"kid": "4b49a739d1eb115b3225f4cf9beb6d1b",
"alg": "ES384"
},
{
"kty": "RSA",
"alg": "RS384",
"n": "vjbIzTqiY8K8zApeNng5ekNNIxJfXAue9BjoMrZ9Qy9m7yIA-tf6muEupEXWhq70tC7vIGLqJJ4O8m7yiH8H2qklX2mCAMg3xG3nbykY2X7JXtW9P8VIdG0sAMt5aZQnUGCgSS3n0qaooGn2LUlTGIR88Qi-4Nrao9_3Ki3UCiICeCiAE224jGCg0OlQU6qj2gEB3o-DWJFlG_dz1y-Mxo5ivaeM0vWuodjDrp-aiabJcSF_dx26sdC9dZdBKXFDq0t19I9S9AyGpGDJwzGRtWHY6LsskNHLvo8Zb5AsJ9eRZKpnh30SYBZI9WHtzU85M9WQqdScR69Vyp-6Uhfbvw",
"e": "AQAB",
"use": "sig",
"key_ops": [
"verify"
],
"ext": true,
"kid": "b41528b6f37a9500edb8a905a595bdd7"
}
]
}
66 changes: 66 additions & 0 deletions src/test/java/org/mitre/fhir/authorization/TestAuthorization.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.SignatureVerificationException;
import com.auth0.jwt.interfaces.DecodedJWT;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonMappingException;
Expand All @@ -16,6 +17,7 @@
import java.nio.file.Paths;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.interfaces.RSAPrivateKey;
import java.util.Base64;
import java.util.Calendar;
import java.util.Date;
Expand Down Expand Up @@ -624,6 +626,70 @@ public void testGetTokenWithBasicAuthWithInvalidClientSecret()
AuthorizationController authorizationController = new AuthorizationController();
authorizationController.getToken(FhirReferenceServerUtils.SAMPLE_CODE, null, null, null, "authorization_code", null, null, null, request);
}

@Test
public void testGetTokenWithhWithConfidentialAsymmetricClient()
throws Exception {
String serverBaseUrl = "";
MockHttpServletRequest request = new MockHttpServletRequest();
request.setLocalAddr("localhost");
request.setRequestURI(serverBaseUrl);
request.setServerPort(TestUtils.TEST_PORT);

String scopes = "";

String asymmetricClientID = "SAMPLE_ASYMMETRIC_CLIENT_ID";
// the key at src/test/resources/client_signing_key.key was generated via a JWK-to-PEM with the RSA private key from
// https://github.com/inferno-framework/smart-app-launch-test-kit/blob/main/lib/smart_app_launch/smart_jwks.json
RSAPrivateKey privateKey = RsaUtils.getRsaPrivateKey("/client_signing_key.key");
Algorithm algorithm = Algorithm.RSA384(null, privateKey);

String clientAssertionType = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer";
String clientAssertion = JWT.create()
.withIssuer(asymmetricClientID)
.withSubject(asymmetricClientID)
.withKeyId("b41528b6f37a9500edb8a905a595bdd7")
.sign(algorithm);

AuthorizationController authorizationController = new AuthorizationController();
// no error should be thrown
authorizationController.getToken(
FhirReferenceServerUtils.createCode(SAMPLE_CODE, scopes, testPatientId.getIdPart()), null,
null, null, "authorization_code", null, clientAssertionType, clientAssertion, request);
}

@Test
public void testGetTokenWithBasicAuthWithInvalidSignature()
throws JSONException, BearerTokenException, TokenNotFoundException, RsaKeyException {
String serverBaseUrl = "";
MockHttpServletRequest request = new MockHttpServletRequest();
request.setLocalAddr("localhost");
request.setRequestURI(serverBaseUrl);
request.setServerPort(TestUtils.TEST_PORT);

String asymmetricClientID = "SAMPLE_ASYMMETRIC_CLIENT_ID";
// the private key of this server doesn't match the private key of the client
// so signing with it should produce an error
RSAPrivateKey privateKey = RsaUtils.getRsaPrivateKey();
Algorithm algorithm = Algorithm.RSA384(null, privateKey);

String clientAssertionType = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer";
String clientAssertion = JWT.create()
.withIssuer(asymmetricClientID)
.withSubject(asymmetricClientID)
.withKeyId("b41528b6f37a9500edb8a905a595bdd7")
.sign(algorithm);

AuthorizationController authorizationController = new AuthorizationController();

try {
authorizationController.getToken(FhirReferenceServerUtils.SAMPLE_CODE, null, null, null, "authorization_code", null,
clientAssertionType, clientAssertion, request);
Assert.fail("Token request should have thrown an exception for invalid signature");
} catch (ResponseStatusException e) {
Assert.assertTrue(e.getCause() instanceof SignatureVerificationException);
}
}

@Test
public void testGetTokenGivesValidOpenId() throws IllegalArgumentException, RsaKeyException,
Expand Down
28 changes: 28 additions & 0 deletions src/test/resources/client_signing_key.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN RSA PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC+NsjNOqJjwrzM
Cl42eDl6Q00jEl9cC570GOgytn1DL2bvIgD61/qa4S6kRdaGrvS0Lu8gYuokng7y
bvKIfwfaqSVfaYIAyDfEbedvKRjZfsle1b0/xUh0bSwAy3lplCdQYKBJLefSpqig
afYtSVMYhHzxCL7g2tqj3/cqLdQKIgJ4KIATbbiMYKDQ6VBTqqPaAQHej4NYkWUb
93PXL4zGjmK9p4zS9a6h2MOun5qJpslxIX93Hbqx0L11l0EpcUOrS3X0j1L0DIak
YMnDMZG1YdjouyyQ0cu+jxlvkCwn15FkqmeHfRJgFkj1Ye3NTzkz1ZCp1JxHr1XK
n7pSF9u/AgMBAAECggEBAK64lfRmIpouW8u0zluMTYev4GAR1UQwbH7djhRfKmqX
VR7dhjbkQw8XPweoGuk2NhJ4djCyi069XQ91uBSHUwiYjHq66K6dOxSUu5yRDIFk
A8a34JF+PxKq4VuNi+XeL8qWJ0VxBFuruIM5MebhTpHbyQSCuwrCHUmgRWIaHIZ8
pzYL2ylmrIuK5doWshB/LXX9we2ejiB/WpbKQU2qt4/8ybfypK6WBMyFG+FIACwe
TU1p+WCq9AL5P2rDoPGH41YkMDvQEhmlo0HidkcfDhEMATEB0gzAkljYlpmDrvT3
MEmRXNby8as8iDPEK0EiRwgjKP8BmIJHv69Ai3jjRxECgYEA5hH/QApWGeobRi1n
7XbMfJYohB8K3JDPa0MspfplHpJ+17JiGG2sNoBdBcpaPRf9OX48P8VqO0qrSSRA
k+I+uO6OO9BHbIukXJILqnY2JmurYzbcYbt5FVbknlHRJojkF6+7sFBazpueUlOn
XCw7X7Z/SkfNE4QX5Ejm2Zm5mekCgYEA06bZz7c7K9s1+aEZsxYnLJ9eTpKlt1tI
BDA/LwIh5W3w259pes2kUtimbnkyOf+V2ZIERsFCh5s+S9IOEMvAIa6M5j9GW1IL
NT7AcHIUfcyFcH+FF8BU/KJdRP5PXnIXFdYcylvsdoIdchy1AaUIzyiKRCU3HBYI
75hez0l/F2cCgYEAh/sVIXW6hCCRND48EedIX06k7conMkxIu/39ErDXOWWeoMAn
KIcR5TijQnviL//QxD1vQMXezuKIMHfDz2RGbClbWdD1lhtG7wvG515tDPJQXxia
0wzqOQmdoFF9S8hXAAT26vPjaAAkaEZXQaxG/4Au5elgNWu6b0wDXZN1VpkCgYAa
pLRim5RNTwkaZZdQngdMbLt4dKl7zXfhXxl1F3Wg5hgd7ZFfridzbmZ1Hbie6He6
rdDv/0AaxgYvkT/ICX4kP07pZSm8MTXL/BxJi3Lc6zQZF1Rvyvxn2SCYoNLo9r/F
NfDl4HCB8ps2VDrM/DOBsH7HWU/4ryVs0+eLfJsNHQKBgHKkH6ItLDOUrvGToc55
nVqwyEAdvB+nwSomm4GiCpG1mVeHtJ8Bby6rh4zzr7+AKPIhBlOXcob4NJ+QSmD4
vuJqejOjgNbMtDa06+xLCkmBsrtqQava8OKAGEzNi1yoOivmsIFFJZHOOdSMMRwQ
HteT1ggTN5Ly0HoozYndFc+S
-----END RSA PRIVATE KEY-----
2 changes: 2 additions & 0 deletions src/test/resources/hapi.properties
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ hibernate.hbm2ddl.auto=update
inferno.public_client_id=SAMPLE_PUBLIC_CLIENT_ID
inferno.confidential_client_id=SAMPLE_CONFIDENTIAL_CLIENT_ID
inferno.confidential_client_secret=SAMPLE_CONFIDENTIAL_CLIENT_SECRET
inferno.asymmetric_client_id=SAMPLE_ASYMMETRIC_CLIENT_ID
inferno.asymmetric_client_jwks=/inferno_client_jwks.json
inferno.resources_folder=./do_not_load_resources_in_test

0 comments on commit efcb5b1

Please sign in to comment.