From 30cf5b1138503c8af7b2818e6a478b58b6f17c23 Mon Sep 17 00:00:00 2001 From: Ryan Liang <109499885+RyanL1997@users.noreply.github.com> Date: Wed, 9 Aug 2023 10:10:24 -0700 Subject: [PATCH] [Feature/Extension] Add cluster id check for OBO Authenticator (#3117) --------- Signed-off-by: Ryan Liang --- .../security/OpenSearchSecurityPlugin.java | 10 +- .../http/OnBehalfOfAuthenticator.java | 8 ++ .../http/OnBehalfOfAuthenticatorTest.java | 106 +++++++++++++++--- 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index d64a07b732..8e9b71cc54 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -212,7 +212,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile ThreadPool threadPool; private volatile ConfigurationRepository cr; private volatile AdminDNs adminDns; - private volatile ClusterService cs; + private static volatile ClusterService cs; private volatile AtomicReference localNode = new AtomicReference<>(); private volatile AuditLog auditLog; private volatile BackendRegistry backendRegistry; @@ -1959,4 +1959,12 @@ public void start() {} public void stop() {} } + + public static void setClusterService(ClusterService clusterService) { + cs = clusterService; + } + + public static ClusterService getClusterNameString() { + return cs; + } } diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index f812cc97ee..3ac8b14eb4 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -36,6 +36,7 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; +import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.auth.HTTPAuthenticator; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; import org.opensearch.security.ssl.util.ExceptionUtils; @@ -209,6 +210,13 @@ private AuthCredentials extractCredentials0(final RestRequest request) { return null; } + final String issuer = claims.getIssuer(); + final String clusterID = OpenSearchSecurityPlugin.getClusterNameString().getClusterName().value(); + if (!issuer.equals(clusterID)) { + log.error("This issuer of this OBO does not match the current cluster identifier"); + return null; + } + List roles = extractSecurityRolesFromClaims(claims); String[] backendRoles = extractBackendRolesFromClaims(claims); diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index e9208ccac8..b3b1ef1a81 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -27,14 +27,23 @@ import io.jsonwebtoken.security.Keys; import org.apache.commons.lang3.RandomStringUtils; import org.apache.hc.core5.http.HttpHeaders; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; +import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class OnBehalfOfAuthenticatorTest { + final static String clusterNameString = "cluster_0"; final static String enableOBO = "true"; final static String disableOBO = "false"; final static String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); @@ -44,6 +53,20 @@ public class OnBehalfOfAuthenticatorTest { final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8)); final static SecretKey secretKey = Keys.hmacShaKeyFor(signingKeyB64Encoded.getBytes(StandardCharsets.UTF_8)); + @Before + public void setupMocks() { + ClusterService mockedClusterService = mock(ClusterService.class); + ClusterName mockedClusterName = new ClusterName(clusterNameString); + when(mockedClusterService.getClusterName()).thenReturn(mockedClusterName); + + OpenSearchSecurityPlugin.setClusterService(mockedClusterService); + } + + @After + public void tearDown() { + OpenSearchSecurityPlugin.setClusterService(null); + } + @Test public void testNoKey() throws Exception { @@ -51,7 +74,7 @@ public void testNoKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( null, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), + Jwts.builder().setIssuer(clusterNameString).claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a RuntimeException"); @@ -67,7 +90,7 @@ public void testEmptyKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( null, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), + Jwts.builder().setIssuer(clusterNameString).claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a RuntimeException"); @@ -83,7 +106,7 @@ public void testBadKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( BaseEncoding.base64().encode(new byte[] { 1, 3, 3, 4, 3, 6, 7, 8, 3, 10 }), claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), + Jwts.builder().setIssuer(clusterNameString).claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a WeakKeyException"); @@ -119,6 +142,7 @@ public void testInvalid() throws Exception { @Test public void testDisabled() throws Exception { String jwsToken = Jwts.builder() + .setIssuer(clusterNameString) .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") @@ -136,6 +160,7 @@ public void testDisabled() throws Exception { @Test public void testNonSpecifyOBOSetting() throws Exception { String jwsToken = Jwts.builder() + .setIssuer(clusterNameString) .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") @@ -154,6 +179,7 @@ public void testNonSpecifyOBOSetting() throws Exception { public void testBearer() throws Exception { String jwsToken = Jwts.builder() + .setIssuer(clusterNameString) .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") @@ -170,13 +196,14 @@ public void testBearer() throws Exception { Assert.assertEquals("Leonard McCoy", credentials.getUsername()); Assert.assertEquals(0, credentials.getSecurityRoles().size()); Assert.assertEquals(0, credentials.getBackendRoles().size()); - Assert.assertEquals(3, credentials.getAttributes().size()); + Assert.assertEquals(4, credentials.getAttributes().size()); } @Test public void testBearerWrongPosition() throws Exception { String jwsToken = Jwts.builder() + .setIssuer(clusterNameString) .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") @@ -195,6 +222,7 @@ public void testBearerWrongPosition() throws Exception { @Test public void testBasicAuthHeader() throws Exception { String jwsToken = Jwts.builder() + .setIssuer(clusterNameString) .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") @@ -214,7 +242,12 @@ public void testRoles() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), + Jwts.builder() + .setIssuer(clusterNameString) + .claim("typ", "obo") + .setSubject("Leonard McCoy") + .claim("dr", "role1,role2") + .setAudience("svc1"), true ); @@ -230,7 +263,7 @@ public void testNoTokenType() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), + Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), true ); @@ -243,7 +276,12 @@ public void testNullClaim() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", null).setAudience("svc1"), + Jwts.builder() + .setIssuer(clusterNameString) + .claim("typ", "obo") + .setSubject("Leonard McCoy") + .claim("dr", null) + .setAudience("svc1"), false ); @@ -258,7 +296,12 @@ public void testNonStringClaim() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", 123L).setAudience("svc1"), + Jwts.builder() + .setIssuer(clusterNameString) + .claim("typ", "obo") + .setSubject("Leonard McCoy") + .claim("dr", 123L) + .setAudience("svc1"), true ); @@ -274,7 +317,7 @@ public void testRolesMissing() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").setAudience("svc1"), + Jwts.builder().setIssuer(clusterNameString).claim("typ", "obo").setSubject("Leonard McCoy").setAudience("svc1"), false ); @@ -290,7 +333,12 @@ public void testWrongSubjectKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").claim("roles", "role1,role2").claim("asub", "Dr. Who").setAudience("svc1"), + Jwts.builder() + .setIssuer(clusterNameString) + .claim("typ", "obo") + .claim("roles", "role1,role2") + .claim("asub", "Dr. Who") + .setAudience("svc1"), false ); @@ -303,7 +351,7 @@ public void testExp() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Expired").setExpiration(new Date(100)), + Jwts.builder().setIssuer(clusterNameString).claim("typ", "obo").setSubject("Expired").setExpiration(new Date(100)), false ); @@ -316,7 +364,11 @@ public void testNbf() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("typ", "obo").setSubject("Expired").setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))), + Jwts.builder() + .setIssuer(clusterNameString) + .claim("typ", "obo") + .setSubject("Expired") + .setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))), false ); @@ -327,7 +379,15 @@ public void testNbf() throws Exception { public void testRolesArray() throws Exception { JwtBuilder builder = Jwts.builder() - .setPayload("{" + "\"typ\": \"obo\"," + "\"sub\": \"Cluster_0\"," + "\"aud\": \"ext_0\"," + "\"dr\": \"a,b,3rd\"" + "}"); + .setPayload( + "{" + + "\"iss\": \"cluster_0\"," + + "\"typ\": \"obo\"," + + "\"sub\": \"Cluster_0\"," + + "\"aud\": \"ext_0\"," + + "\"dr\": \"a,b,3rd\"" + + "}" + ); final AuthCredentials credentials = extractCredentialsFromJwtHeader(signingKeyB64Encoded, claimsEncryptionKey, builder, true); @@ -339,6 +399,26 @@ public void testRolesArray() throws Exception { Assert.assertTrue(credentials.getSecurityRoles().contains("3rd")); } + @Test + public void testDifferentIssuer() throws Exception { + + String jwsToken = Jwts.builder() + .setIssuer("Wrong Cluster Identifier") + .claim("typ", "obo") + .setSubject("Leonard McCoy") + .setAudience("ext_0") + .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512) + .compact(); + + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings()); + Map headers = new HashMap(); + headers.put("Authorization", "Bearer " + jwsToken); + + AuthCredentials credentials = jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap()), null); + + Assert.assertNull(credentials); + } + /** extracts a default user credential from a request header */ private AuthCredentials extractCredentialsFromJwtHeader( final String signingKeyB64Encoded,