Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch JWT library implementations from cxf to nimbus #3421

Merged
merged 37 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1c75b4a
Replace JWT library with Nimbus Jose + JWT
peternied Aug 25, 2023
e9e5457
Merge remote-tracking branch 'peternied/nimbus-jose-jwt'
MaciejMierzwa Sep 26, 2023
68fb56c
test jwt content
MaciejMierzwa Sep 26, 2023
48658dd
swap cxf jwt to nimbus jwt
MaciejMierzwa Sep 28, 2023
93c1bce
remove all usages of cxf.rs.security.jose
MaciejMierzwa Sep 29, 2023
5fc3b8b
tests, encoding fixes
MaciejMierzwa Oct 2, 2023
1eb397c
naming, add padding to JwtVendor secret
MaciejMierzwa Oct 3, 2023
9a2ef33
small refactor, spotless, tests, use raw settings to create jwk
MaciejMierzwa Oct 3, 2023
0ee2de6
Merge remote-tracking branch 'origin/main' into nimbus-jose-jwt
MaciejMierzwa Oct 3, 2023
f52ca23
Merge remote-tracking branch 'origin/main' into nimbus-jose-jwt
MaciejMierzwa Oct 3, 2023
46eb723
test build after merge
MaciejMierzwa Oct 3, 2023
10fe305
revert misc changes
MaciejMierzwa Oct 4, 2023
da51bec
correct HMAC padding, escape chars in tests
MaciejMierzwa Oct 4, 2023
c151696
PR changes, style
MaciejMierzwa Oct 4, 2023
a597cf5
remove org.apache.cxf:cxf-rt-rs-security-jose import, add rule forbid…
MaciejMierzwa Oct 4, 2023
7e2c6ca
PR suggestions, null checks, java.util.Date
MaciejMierzwa Oct 4, 2023
88de2cc
Merge branch 'main_origin' into nimbus-jose-jwt
MaciejMierzwa Oct 5, 2023
48c3b5a
PR suggestions, spotless
MaciejMierzwa Oct 5, 2023
a245f58
Merge branch 'main_origin' into nimbus-jose-jwt
MaciejMierzwa Oct 5, 2023
3002f11
Exception -> IllegalArgumentException
MaciejMierzwa Oct 5, 2023
f0e19bd
Merge remote-tracking branch 'origin/main' into nimbus-jose-jwt
MaciejMierzwa Oct 6, 2023
0d21bd1
Merge remote-tracking branch 'origin/main' into nimbus-jose-jwt
MaciejMierzwa Oct 16, 2023
8d210a1
Class raw use fix
MaciejMierzwa Oct 17, 2023
480ba8a
Fix the seconds into milli seconds in jwt vendor
RyanL1997 Oct 20, 2023
f74edd9
Fixed obo integ test
RyanL1997 Oct 20, 2023
c899710
Refactor the matcher library
RyanL1997 Oct 20, 2023
1d5fcb4
Fix saml authenticator test
RyanL1997 Oct 20, 2023
2848560
Add padding back but not for obo
RyanL1997 Oct 23, 2023
c9fd75f
test cxf, nimbus compability
MaciejMierzwa Oct 24, 2023
4402b14
default encoding fix
MaciejMierzwa Oct 24, 2023
de31e00
default encoding fix
MaciejMierzwa Oct 24, 2023
2bdd1de
Add tests and relocate KeyPaddingUtil
RyanL1997 Oct 24, 2023
85d7eaa
Add comment for cxf code generation
RyanL1997 Oct 24, 2023
2c2560b
Reloacate the comment for cxf lib
RyanL1997 Oct 24, 2023
78e94fa
Fix testParsePrevGeneratedJwt()
RyanL1997 Oct 24, 2023
1004595
Fix the comment description
RyanL1997 Oct 24, 2023
9c667e4
Revert the changes for padding test config
RyanL1997 Oct 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Base64;
import java.util.Date;
import java.util.List;
import java.util.Optional;
Expand All @@ -31,12 +32,14 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.JWSHeader;
import com.nimbusds.jose.crypto.factories.DefaultJWSSignerFactory;
import com.nimbusds.jose.jwk.JWK;
import com.nimbusds.jose.jwk.KeyUse;
import com.nimbusds.jose.jwk.OctetSequenceKey;
import com.nimbusds.jose.util.ByteUtils;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.SignedJWT;
import com.onelogin.saml2.authn.SamlResponse;
Expand All @@ -60,10 +63,11 @@
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.authtoken.jwt.JwtVendor;
import org.opensearch.security.dlic.rest.api.AuthTokenProcessorAction;
import org.opensearch.security.filter.SecurityResponse;

import static com.nimbusds.jose.crypto.MACSigner.getMinRequiredSecretLength;

class AuthTokenProcessorHandler {
private static final Logger log = LogManager.getLogger(AuthTokenProcessorHandler.class);
private static final Logger token_log = LogManager.getLogger("com.amazon.dlic.auth.http.saml.Token");
Expand Down Expand Up @@ -117,6 +121,18 @@
this.jwsHeader = this.createJwsHeaderFromSettings();
}

public static String padSecret(String signingKey, JWSAlgorithm jwsAlgorithm) {
RyanL1997 marked this conversation as resolved.
Show resolved Hide resolved
int requiredSecretLength;
try {
requiredSecretLength = getMinRequiredSecretLength(jwsAlgorithm);
} catch (JOSEException e) {
throw new RuntimeException(e);

Check warning on line 129 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L128-L129

Added lines #L128 - L129 were not covered by tests
}
int requiredByteLength = ByteUtils.byteLength(requiredSecretLength);
// padding the signing key with 0s to meet the minimum required length
return StringUtils.rightPad(signingKey, requiredByteLength, "\0");
}

@SuppressWarnings("removal")
Optional<SecurityResponse> handle(RestRequest restRequest) throws Exception {
try {
Expand Down Expand Up @@ -247,9 +263,10 @@
}

JWK createJwkFromSettings(Settings settings, Settings jwtSettings) throws Exception {
String exchangeKey = JwtVendor.padSecret(settings.get("exchange_key"), JWSAlgorithm.HS512);
String exchangeKey = settings.get("exchange_key");

if (!Strings.isNullOrEmpty(exchangeKey)) {
exchangeKey = padSecret(new String(Base64.getDecoder().decode(exchangeKey)), JWSAlgorithm.HS512);

return new OctetSequenceKey.Builder(exchangeKey.getBytes(StandardCharsets.UTF_8)).algorithm(JWSAlgorithm.HS512)
.keyUse(KeyUse.SIGNATURE)
Expand All @@ -263,7 +280,7 @@
);
}

String k = JwtVendor.padSecret(jwkSettings.get("k"), JWSAlgorithm.HS512);
String k = padSecret(new String(Base64.getDecoder().decode(jwkSettings.get("k"))), JWSAlgorithm.HS512);

return new OctetSequenceKey.Builder(k.getBytes(StandardCharsets.UTF_8)).algorithm(JWSAlgorithm.HS512)
.keyUse(KeyUse.SIGNATURE)
Expand All @@ -277,7 +294,7 @@
.claim(this.jwtSubjectKey, this.extractSubject(samlResponse));

if (this.samlSubjectKey != null) {
jwtClaimsBuilder.claim("saml_ni", samlResponse.getNameId());

Check warning on line 297 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L297

Added line #L297 was not covered by tests
}
if (samlResponse.getNameIdFormat() != null) {
jwtClaimsBuilder.claim("saml_nif", SamlNameIdFormat.getByUri(samlResponse.getNameIdFormat()).getShortName());
Expand All @@ -301,7 +318,7 @@
String encodedJwt = jwt.serialize();

if (token_log.isDebugEnabled()) {
token_log.debug("Created JWT: " + encodedJwt + "\n" + jwt.getHeader().toString() + "\n" + jwt.getJWTClaimsSet().toString());

Check warning on line 321 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L321

Added line #L321 was not covered by tests
willyborankin marked this conversation as resolved.
Show resolved Hide resolved
}

return encodedJwt;
Expand All @@ -311,10 +328,10 @@
DateTime sessionNotOnOrAfter = samlResponse.getSessionNotOnOrAfter();

if (this.expiryBaseValue == ExpiryBaseValue.NOW) {
return System.currentTimeMillis() + this.expiryOffset * 1000;

Check warning on line 331 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L331

Added line #L331 was not covered by tests
MaciejMierzwa marked this conversation as resolved.
Show resolved Hide resolved
peternied marked this conversation as resolved.
Show resolved Hide resolved
} else if (this.expiryBaseValue == ExpiryBaseValue.SESSION) {
if (sessionNotOnOrAfter != null) {
return sessionNotOnOrAfter.getMillis() + this.expiryOffset * 1000;

Check warning on line 334 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L334

Added line #L334 was not covered by tests
MaciejMierzwa marked this conversation as resolved.
Show resolved Hide resolved
} else {
throw new Exception("Error while determining JWT expiration time: SamlResponse did not contain sessionNotOnOrAfter value");
}
Expand All @@ -322,7 +339,7 @@
// AUTO

if (sessionNotOnOrAfter != null) {
return sessionNotOnOrAfter.getMillis();

Check warning on line 342 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L342

Added line #L342 was not covered by tests
} else {
return System.currentTimeMillis() + (this.expiryOffset > 0 ? this.expiryOffset * 1000 : 60 * 60_000);
MaciejMierzwa marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import java.util.function.LongSupplier;

import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.util.ByteUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -39,7 +37,6 @@
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.Settings;

import static com.nimbusds.jose.crypto.MACSigner.getMinRequiredSecretLength;
import static org.opensearch.security.util.AuthTokenUtils.isKeyNull;

public class JwtVendor {
Expand Down Expand Up @@ -91,10 +88,10 @@
);
}

String signingKey = jwkSettings.get("k");
key = new OctetSequenceKey.Builder(Base64.getDecoder().decode(signingKey)).algorithm(JWSAlgorithm.HS512)
.keyUse(KeyUse.SIGNATURE)
.build();

Check warning on line 94 in src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java#L91-L94

Added lines #L91 - L94 were not covered by tests
}

try {
Expand All @@ -104,18 +101,6 @@
}
}

public static String padSecret(String signingKey, JWSAlgorithm jwsAlgorithm) {
int requiredSecretLength;
try {
requiredSecretLength = getMinRequiredSecretLength(jwsAlgorithm);
} catch (JOSEException e) {
throw new RuntimeException(e);
}
int requiredByteLength = ByteUtils.byteLength(requiredSecretLength);
// padding the signing key with 0s to meet the minimum required length
return StringUtils.rightPad(signingKey, requiredByteLength, "\0");
}

RyanL1997 marked this conversation as resolved.
Show resolved Hide resolved
public String createJwt(
String issuer,
String subject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,65 @@ public void testInvalid() throws Exception {
Assert.assertNull(credentials);
}

@Test
public void testParsePrevGeneratedJwt() {
peternied marked this conversation as resolved.
Show resolved Hide resolved
String jwsToken =
"eyJhbGciOiJIUzUxMiJ9.eyJuYmYiOjE2OTgxNTE4ODQsImV4cCI6MTY5ODE1NTQ4NCwic3ViIjoiaG9yc3QiLCJzYW1sX25pZiI6InUiLCJzYW1sX3NpIjoiTU9DS1NBTUxfMyIsInJvbGVzIjpudWxsfQ.E_MP8wVVu1P7_RATtjhnCvPft2gQTFdY5NlmRTCsrjdDXTUfxkswktWCB_k_wXDKCuNukNlSL2FSo3EV2VtUEQ";
Settings settings = Settings.builder()
.put(
"signing_key",
BaseEncoding.base64()
.encode(
"thisIsSecretThatIsVeryHardToCrackItsPracticallyImpossibleToDothisIsSecretThatIsVeryHardToCrackItsPracticallyImpossibleToDo"
.getBytes(StandardCharsets.UTF_8)
)
)
.build();

HTTPJwtAuthenticator jwtAuth = new HTTPJwtAuthenticator(settings, null);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

AuthCredentials credentials = jwtAuth.extractCredentials(
new FakeRestRequest(headers, new HashMap<String, String>()).asSecurityRequest(),
null
);

Assert.assertNotNull(credentials);
Assert.assertEquals("horst", credentials.getUsername());
Assert.assertEquals(0, credentials.getBackendRoles().size());
Assert.assertEquals(5, credentials.getAttributes().size());
Assert.assertEquals("1698151884", credentials.getAttributes().get("attr.jwt.nbf"));
Assert.assertEquals("1698155484", credentials.getAttributes().get("attr.jwt.exp"));
}

@Test
public void testFailToParsePrevGeneratedJwt() {
String jwsToken =
"eyJhbGciOiJIUzUxMiJ9.eyJuYmYiOjE2OTgxNTE4ODQsImV4cCI6MTY5ODE1NTQ4NCwic3ViIjoiaG9yc3QiLCJzYW1sX25pZiI6InUiLCJzYW1sX3NpIjoiTU9DS1NBTUxfMyIsInJvbGVzIjpudWxsfQ.E_MP8wVVu1P7_RATtjhnCvPft2gQTFdY5NlmRTCsrjdDXTUfxkswktWCB_k_wXDKCuNukNlSL2FSo3EV2VtUEQ";
Settings settings = Settings.builder()
.put(
"signing_key",
BaseEncoding.base64()
.encode(
"additionalDatathisIsSecretThatIsVeryHardToCrackItsPracticallyImpossibleToDothisIsSecretThatIsVeryHardToCrackItsPracticallyImpossibleToDo"
.getBytes(StandardCharsets.UTF_8)
)
)
.build();

HTTPJwtAuthenticator jwtAuth = new HTTPJwtAuthenticator(settings, null);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

AuthCredentials credentials = jwtAuth.extractCredentials(
new FakeRestRequest(headers, new HashMap<String, String>()).asSecurityRequest(),
null
);

Assert.assertNull(credentials);
}

@Test
public void testBearer() throws Exception {

Expand Down
Loading