Skip to content

Commit

Permalink
Fetch JWKS certificates to verify signature
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-doubez committed Apr 18, 2024
1 parent fda44f0 commit 9442607
Show file tree
Hide file tree
Showing 6 changed files with 296 additions and 35 deletions.
18 changes: 8 additions & 10 deletions docs/configuration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ There are specifics instructions for well known providers:
* [Google Provider](GOOGLE.md)
* [Gitlab Provider](GITLAB.md)

This page contains the reference of plugin's configuration.

## Provider configuration

The OpenID Conenct spec describes a well known configuration location
The OpenID Connect spec describes a well known configuration location
which will also help discovering your settings
(<https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig>)

Expand All @@ -22,9 +23,8 @@ populate the configuration simplifying the configuration greatly.
The switch between modes is controled by the `automanualconfigure` field

| field | format | description |
| automanualconfigure | enum | Crontols endpoint configuration mode
- `auto`: activate automatic configuration
- `manual`: activate manual configuration |
| ----- | ------ | ----------- |
| automanualconfigure | enum | Crontols endpoint configuration mode<br />- `auto`: activate automatic configuration <br />- `manual`: activate manual configuration |
| clientId | string | Id of the openid client obtained from the provider |
| clientSecret | secret | Secret associated to the client |

Expand Down Expand Up @@ -59,10 +59,8 @@ If the JWKS endpoint is configured, JWS' signatures will be verified
| endSessionEndpoint | url | URL to logout from provider (used if activated) |
| jwksServerUrl | url | URL of provider's jws certificates (unused if disabled) |
| scopes | string | Space separated list of scopes to request (default: request all) |
| tokenAuthMethod | enum | method used for authenticating when requesting token(s)
- `client_secret_basic`: for client id/secret as basic authentication user/pass
- `client_secret_post`: for client id/secret sent in post request
|
| tokenAuthMethod | enum | method used for authenticating when requesting token(s)<br />- `client_secret_basic`: for client id/secret as basic authentication user/pass<br />- `client_secret_post`: for client id/secret sent in post request
| userInfoServerUrl | url | URL to get user's details |

### Advanced configuration

Expand All @@ -84,7 +82,7 @@ Most security feature are activated by default if possible.
| disableSslVerification | boolean | disable SSL verification (in case of self signed certificates by example) |
| nonceDisabled | boolean | Disable nonce verification |
| pkceEnable | boolean | Enable PKCE challenge |
| disableSignatureVerification | boolean | If JWKS uri is configured, ignore signature verifiaction |
| disableTokenVerification | boolean | Disable IdToken and UserInfo verification (not recommended) |
| tokenFieldToCheckKey | jmespath | field(s) to check to authorize user |
| tokenFieldToCheckValue | string | tokenFieldToCheckValue expected value |

Expand Down Expand Up @@ -136,7 +134,7 @@ jenkins:
disableSslVerification: <boolean>
nonceDisabled: <boolean>
pkceEnabled: <boolean>
disableSignatureVerification: <boolean>
disableTokenVerification: <boolean>
tokenFieldToCheckKey: <string:jmes path>
tokenFieldToCheckValue: <string>
# escape hatch
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*

Check warning on line 1 in src/main/java/org/jenkinsci/plugins/oic/OicJsonWebTokenVerifier.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

ERROR: (misc) NewlineAtEndOfFile: Expected line ending for file is LF(\n), but CRLF(\r\n) is detected.
* The MIT License
*
* Copyright (c) 2024 JenkinsCI oic-auth-plugin developers
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.oic;

import com.google.api.client.auth.openidconnect.IdToken;
import com.google.api.client.auth.openidconnect.IdTokenVerifier;
import com.google.api.client.json.webtoken.JsonWebSignature;
import hudson.Util;
import java.io.IOException;

/**
* Extend IdTokenVerifier to verify UserInfo webtoken
*/
public class OicJsonWebTokenVerifier extends IdTokenVerifier {

/** Bypass Signature verification if no JWKS url configured */
private final boolean hasNoJwksServerUrl;

/** Payload indicating userInfo */
private static final IdToken.Payload NO_PAYLOAD = new IdToken.Payload();

/**
* Default verifier
*/
public OicJsonWebTokenVerifier() {
super();
hasNoJwksServerUrl = true;
}

Check warning on line 49 in src/main/java/org/jenkinsci/plugins/oic/OicJsonWebTokenVerifier.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 47-49 are not covered by tests

Check warning on line 49 in src/main/java/org/jenkinsci/plugins/oic/OicJsonWebTokenVerifier.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicJsonWebTokenVerifier.java#L47-L49

Added lines #L47 - L49 were not covered by tests

/**
* Verifier with custom builder
*/
public OicJsonWebTokenVerifier(String jwksServerUrl, IdTokenVerifier.Builder builder) {
super(builder.setCertificatesLocation(jwksServerUrl));
hasNoJwksServerUrl = (Util.fixEmptyAndTrim(jwksServerUrl) == null);
}

/** Verify real idtoken */
public boolean verifyIdToken(IdToken idToken) throws IOException {
if (hasNoJwksServerUrl) {
/* avoid Google's certificate fallback mechanism */
return super.verifyPayload(idToken);
}
return verifyOrThrow(idToken);
}

/** Verify userinfo jwt token */
public boolean verifyUserInfo(JsonWebSignature userinfo) throws IOException {
if (hasNoJwksServerUrl) {
/* avoid Google's certificate fallback mechanism */
return true;
}
IdToken idToken = new IdToken(
userinfo.getHeader(),
NO_PAYLOAD, /* bypass verification of payload */
userinfo.getSignatureBytes(),
userinfo.getSignedContentBytes());
return verifyOrThrow(idToken);
}

/** hack: verify payload only if idtoken is not userinfo */
@Override
protected boolean verifyPayload(IdToken idToken) {
if (idToken.getPayload() == NO_PAYLOAD) {
return true;
}
return super.verifyPayload(idToken);
}
}
59 changes: 48 additions & 11 deletions src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.api.client.auth.oauth2.BearerToken;
import com.google.api.client.auth.oauth2.ClientParametersAuthentication;
import com.google.api.client.auth.oauth2.Credential.AccessMethod;
import com.google.api.client.auth.openidconnect.HttpTransportFactory;
import com.google.api.client.auth.openidconnect.IdToken;
import com.google.api.client.http.BasicAuthentication;
import com.google.api.client.http.GenericUrl;
Expand Down Expand Up @@ -191,7 +192,7 @@ public static enum TokenAuthMethod {

/** Flag to disable JWT signature verification
*/
private boolean disableSignatureVerification = false;
private boolean disableTokenVerification = false;

/** Flag to disable nonce security
*/
Expand All @@ -206,7 +207,11 @@ public static enum TokenAuthMethod {
* but it's still needed for backwards compatibility */
private transient String endSessionUrl;

private transient HttpTransport httpTransport;
/** Verification of IdToken and UserInfo (in jwt case)
*/
private transient OicJsonWebTokenVerifier jwtVerifier;

private transient HttpTransport httpTransport = null;

/** Random generator needed for robust random wait
*/
Expand Down Expand Up @@ -488,8 +493,8 @@ public boolean isPkceEnabled() {
return pkceEnabled;
}

public boolean isDisableSignatureVerification() {
return disableSignatureVerification;
public boolean isDisableTokenVerification() {
return disableTokenVerification;
}

public boolean isNonceDisabled() {
Expand Down Expand Up @@ -742,8 +747,8 @@ public void setPkceEnabled(boolean pkceEnabled) {
}

@DataBoundSetter
public void setDisableSignatureVerification(boolean disableSignatureVerification) {
this. disableSignatureVerification = disableSignatureVerification;
public void setDisableTokenVerification(boolean disableTokenVerification) {
this. disableTokenVerification = disableTokenVerification;
}

Check warning on line 752 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 751-752 are not covered by tests

Check warning on line 752 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L751-L752

Added lines #L751 - L752 were not covered by tests

@DataBoundSetter
Expand Down Expand Up @@ -904,7 +909,7 @@ public HttpResponse onSuccess(String authorizationCode, AuthorizationCodeFlow fl
} catch(IllegalArgumentException e) {
return HttpResponses.errorWithoutStack(403, Messages.OicSecurityRealm_IdTokenParseError());
}
if (!isDisableSignatureVerification() && !validateSignature(idToken)) {
if (!validateIdToken(idToken)) {

Check warning on line 912 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 912 is only partially covered, one branch is missing
return HttpResponses.errorWithoutStack(401, "Unauthorized");

Check warning on line 913 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 913 is not covered by tests

Check warning on line 913 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L913

Added line #L913 was not covered by tests
}
if (!isNonceDisabled() && !validateNonce(idToken)) {
Expand Down Expand Up @@ -945,9 +950,41 @@ public HttpResponse onSuccess(String authorizationCode, AuthorizationCodeFlow fl
.commenceLogin(buildAuthorizationCodeFlow());
}

/** Validate WebToken signature if available */
private boolean validateSignature(JsonWebSignature jws) {
return true;
/** Create OicJsonWebTokenVerifier if needed */
private OicJsonWebTokenVerifier getJwksVerifier() {
if (isDisableTokenVerification()) {

Check warning on line 955 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 955 is only partially covered, one branch is missing
return null;

Check warning on line 956 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 956 is not covered by tests

Check warning on line 956 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L956

Added line #L956 was not covered by tests
}
if (jwtVerifier == null) {
jwtVerifier = new OicJsonWebTokenVerifier(
jwksServerUrl,
new OicJsonWebTokenVerifier.Builder()
.setHttpTransportFactory(new HttpTransportFactory() {
@Override
public HttpTransport create() {
return httpTransport;
}
}));
}
return jwtVerifier;
}

/** Validate UserInfo signature if available */
private boolean validateUserInfo(JsonWebSignature userinfo) throws IOException {
OicJsonWebTokenVerifier verifier = getJwksVerifier();
if (verifier == null) {

Check warning on line 975 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 975 is only partially covered, one branch is missing
return true;

Check warning on line 976 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 976 is not covered by tests

Check warning on line 976 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L976

Added line #L976 was not covered by tests
}
return verifier.verifyUserInfo(userinfo);
}

/** Validate IdToken signature if available */
private boolean validateIdToken(IdToken idtoken) throws IOException {
OicJsonWebTokenVerifier verifier = getJwksVerifier();
if (verifier == null) {

Check warning on line 984 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 984 is only partially covered, one branch is missing
return true;

Check warning on line 985 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 985 is not covered by tests

Check warning on line 985 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L985

Added line #L985 was not covered by tests
}
return verifier.verifyIdToken(idtoken);
}

@SuppressFBWarnings(
Expand Down Expand Up @@ -976,7 +1013,7 @@ public void initialize(HttpRequest request) throws IOException {
if (response.getHeaders().getContentType().contains("application/jwt")) {
String token = response.parseAsString();
JsonWebSignature jws = JsonWebSignature.parse(flow.getJsonFactory(), token);
if (!isDisableSignatureVerification() && !validateSignature(jws)) {
if (!validateUserInfo(jws)) {

Check warning on line 1016 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1016 is only partially covered, one branch is missing
return null;

Check warning on line 1017 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1017 is not covered by tests

Check warning on line 1017 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L1017

Added line #L1017 was not covered by tests
}
return jws.getPayload();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
<f:entry title="${%Disable Nonce verification}" field="nonceDisabled">
<f:checkbox/>
</f:entry>
<f:entry title="${%Disable signature verification (JWKS)}" field="disableSignatureVerification">
<f:entry title="${%Disable token verification (JWKS)}" field="disableTokenVerification">
<f:checkbox/>
</f:entry>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void testConfig() {
assertEquals("userNameField", oicSecurityRealm.getUserNameField());
assertTrue(oicSecurityRealm.isRootURLFromRequest());
assertEquals("http://localhost/jwks", oicSecurityRealm.getJwksServerUrl());
assertFalse(oicSecurityRealm.isDisableSignatureVerification());
assertFalse(oicSecurityRealm.isDisableTokenVerification());
}

@Test
Expand Down Expand Up @@ -115,7 +115,7 @@ public void testMinimal() throws Exception {
assertTrue(oicSecurityRealm.isLogoutFromOpenidProvider());
assertFalse(oicSecurityRealm.isRootURLFromRequest());
assertEquals(null, oicSecurityRealm.getJwksServerUrl());
assertFalse(oicSecurityRealm.isDisableSignatureVerification());
assertFalse(oicSecurityRealm.isDisableTokenVerification());
}

@Rule(order = 0)
Expand Down Expand Up @@ -154,7 +154,7 @@ public void testMinimalWellKnown() throws Exception {
assertEquals(TokenAuthMethod.client_secret_post, oicSecurityRealm.getTokenAuthMethod());
assertEquals("sub", oicSecurityRealm.getUserNameField());
assertTrue(oicSecurityRealm.isLogoutFromOpenidProvider());
assertFalse(oicSecurityRealm.isDisableSignatureVerification());
assertFalse(oicSecurityRealm.isDisableTokenVerification());
}

/** Class to setup WireMockRule for well known with stub and setting port in env variable
Expand Down
Loading

0 comments on commit 9442607

Please sign in to comment.