[Feature/Extension] Add configuration of disable OBO (opensearch-proj…

…ect#3047) Signed-off-by: Ryan Liang <[email protected]>
peternied · Aug 2, 2023 · d634d60 · d634d60
1 parent df3dba3
commit d634d60
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 40 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
@@ -45,6 +45,7 @@ public class OnBehalfOfJwtAuthenticationTest {
 
     static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS);
 
+    private static Boolean oboEnabled = true;
     private static final String signingKey = Base64.getEncoder()
         .encodeToString(
             "jwt signing key for an on behalf of token authentication backend for testing of OBO authentication".getBytes(
@@ -70,7 +71,7 @@ public class OnBehalfOfJwtAuthenticationTest {
             )
         )
         .authc(AUTHC_HTTPBASIC_INTERNAL)
-        .onBehalfOf(new OnBehalfOfConfig().signing_key(signingKey).encryption_key(encryptionKey))
+        .onBehalfOf(new OnBehalfOfConfig().oboEnabled(oboEnabled).signing_key(signingKey).encryption_key(encryptionKey))
         .build();
 
     @Test

diff --git a/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java b/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java
@@ -18,9 +18,15 @@
 import org.opensearch.core.xcontent.XContentBuilder;
 
 public class OnBehalfOfConfig implements ToXContentObject {
+    private Boolean oboEnabled;
     private String signing_key;
     private String encryption_key;
 
+    public OnBehalfOfConfig oboEnabled(Boolean oboEnabled) {
+        this.oboEnabled = oboEnabled;
+        return this;
+    }
+
     public OnBehalfOfConfig signing_key(String signing_key) {
         this.signing_key = signing_key;
         return this;
@@ -34,6 +40,9 @@ public OnBehalfOfConfig encryption_key(String encryption_key) {
     @Override
     public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException {
         xContentBuilder.startObject();
+        if (oboEnabled || oboEnabled == null) {
+            xContentBuilder.field("enabled", oboEnabled);
+        }
         xContentBuilder.field("signing_key", signing_key);
         if (StringUtils.isNoneBlank(encryption_key)) {
             xContentBuilder.field("encryption_key", encryption_key);

diff --git a/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java b/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java
@@ -31,7 +31,7 @@
 import org.opensearch.rest.RestChannel;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestRequest.Method;
-import org.opensearch.rest.RestStatus;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.security.authtoken.jwt.JwtVendor;
 import org.opensearch.security.securityconf.ConfigModel;
 import org.opensearch.security.securityconf.DynamicConfigModel;
@@ -59,9 +59,18 @@ public void onConfigModelChanged(ConfigModel configModel) {
     @Subscribe
     public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
         this.dcm = dcm;
-        if (dcm.getDynamicOnBehalfOfSettings().get("signing_key") != null
-            && dcm.getDynamicOnBehalfOfSettings().get("encryption_key") != null) {
-            this.vendor = new JwtVendor(dcm.getDynamicOnBehalfOfSettings(), Optional.empty());
+
+        Settings settings = dcm.getDynamicOnBehalfOfSettings();
+        if (settings != null) {
+            Boolean enabled = Boolean.parseBoolean(settings.get("enabled"));
+            String signingKey = settings.get("signing_key");
+            String encryptionKey = settings.get("encryption_key");
+
+            if (!Boolean.FALSE.equals(enabled) && signingKey != null && encryptionKey != null) {
+                this.vendor = new JwtVendor(settings, Optional.empty());
+            } else {
+                this.vendor = null;
+            }
         } else {
             this.vendor = null;
         }

diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
@@ -49,19 +49,23 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator {
 
     private final JwtParser jwtParser;
     private final String encryptionKey;
+    private final Boolean oboEnabled;
 
     public OnBehalfOfAuthenticator(Settings settings) {
+        String oboEnabledSetting = settings.get("enabled");
+        oboEnabled = oboEnabledSetting == null ? Boolean.TRUE : Boolean.valueOf(oboEnabledSetting);
         encryptionKey = settings.get("encryption_key");
         jwtParser = initParser(settings.get("signing_key"));
     }
 
     private JwtParser initParser(final String signingKey) {
         JwtParser _jwtParser = keyUtil.keyAlgorithmCheck(signingKey, log);
-        if (_jwtParser != null) {
-            return _jwtParser;
-        } else {
+
+        if (_jwtParser == null) {
             throw new RuntimeException("Unable to find on behalf of authenticator signing key");
         }
+
+        return _jwtParser;
     }
 
     private List<String> extractSecurityRolesFromClaims(Claims claims) {
@@ -128,6 +132,11 @@ public AuthCredentials run() {
     }
 
     private AuthCredentials extractCredentials0(final RestRequest request) {
+        if (!oboEnabled) {
+            log.error("On-behalf-of authentication has been disabled");
+            return null;
+        }
+
         if (jwtParser == null) {
             log.error("Missing Signing Key. JWT authentication will not work");
             return null;

diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java
@@ -481,6 +481,8 @@ public String toString() {
     }
 
     public static class OnBehalfOf {
+        @JsonProperty("enabled")
+        private Boolean oboEnabled;
         @JsonProperty("signing_key")
         private String signingKey;
         @JsonProperty("encryption_key")
@@ -495,6 +497,14 @@ public String configAsJson() {
             }
         }
 
+        public Boolean getOboEnabled() {
+            return oboEnabled == null ? Boolean.TRUE : oboEnabled;
+        }
+
+        public void setOboEnabled(Boolean oboEnabled) {
+            this.oboEnabled = oboEnabled;
+        }
+
         public String getSigningKey() {
             return signingKey;
         }
@@ -513,7 +523,7 @@ public void setEncryptionKey(String encryptionKey) {
 
         @Override
         public String toString() {
-            return "OnBehalfOf [signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]";
+            return "OnBehalfOf [ enabled=" + oboEnabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]";
         }
     }
 

diff --git a/src/main/java/org/opensearch/security/util/keyUtil.java b/src/main/java/org/opensearch/security/util/keyUtil.java
@@ -14,10 +14,13 @@
 import io.jsonwebtoken.JwtParser;
 import io.jsonwebtoken.Jwts;
 import org.apache.logging.log4j.Logger;
+import org.opensearch.SpecialPermission;
 
+import java.security.AccessController;
 import java.security.Key;
 import java.security.KeyFactory;
 import java.security.NoSuchAlgorithmException;
+import java.security.PrivilegedAction;
 import java.security.PublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.X509EncodedKeySpec;
@@ -27,40 +30,55 @@
 public class keyUtil {
 
     public static JwtParser keyAlgorithmCheck(final String signingKey, final Logger log) {
-        if (signingKey == null || signingKey.length() == 0) {
-            log.error("Unable to find signing key");
-            return null;
-        } else {
-            try {
-                Key key = null;
-
-                final String minimalKeyFormat = signingKey.replace("-----BEGIN PUBLIC KEY-----\n", "")
-                    .replace("-----END PUBLIC KEY-----", "");
-
-                final byte[] decoded = Base64.getDecoder().decode(minimalKeyFormat);
-
-                try {
-                    key = getPublicKey(decoded, "RSA");
-                } catch (Exception e) {
-                    log.debug("No public RSA key, try other algos ({})", e.toString());
-                }
+        final SecurityManager sm = System.getSecurityManager();
 
-                try {
-                    key = getPublicKey(decoded, "EC");
-                } catch (final Exception e) {
-                    log.debug("No public ECDSA key, try other algos ({})", e.toString());
-                }
+        JwtParser jwtParser = null;
 
-                if (Objects.nonNull(key)) {
-                    return Jwts.parser().setSigningKey(key);
-                }
+        if (sm != null) {
+            sm.checkPermission(new SpecialPermission());
+        }
+
+        jwtParser = AccessController.doPrivileged(new PrivilegedAction<JwtParser>() {
+            @Override
+            public JwtParser run() {
+                if (signingKey == null || signingKey.length() == 0) {
+                    log.error("Unable to find signing key");
+                    return null;
+                } else {
+                    try {
+                        Key key = null;
+
+                        final String minimalKeyFormat = signingKey.replace("-----BEGIN PUBLIC KEY-----\n", "")
+                            .replace("-----END PUBLIC KEY-----", "");
+
+                        final byte[] decoded = Base64.getDecoder().decode(minimalKeyFormat);
 
-                return Jwts.parser().setSigningKey(decoded);
-            } catch (Throwable e) {
-                log.error("Error while creating JWT authenticator", e);
-                throw new RuntimeException(e);
+                        try {
+                            key = getPublicKey(decoded, "RSA");
+                        } catch (Exception e) {
+                            log.debug("No public RSA key, try other algos ({})", e.toString());
+                        }
+
+                        try {
+                            key = getPublicKey(decoded, "EC");
+                        } catch (final Exception e) {
+                            log.debug("No public ECDSA key, try other algos ({})", e.toString());
+                        }
+
+                        if (Objects.nonNull(key)) {
+                            return Jwts.parser().setSigningKey(key);
+                        }
+
+                        return Jwts.parser().setSigningKey(decoded);
+                    } catch (Throwable e) {
+                        log.error("Error while creating JWT authenticator", e);
+                        throw new RuntimeException(e);
+                    }
+                }
             }
-        }
+        });
+
+        return jwtParser;
     }
 
     private static PublicKey getPublicKey(final byte[] keyBytes, final String algo) throws NoSuchAlgorithmException,

diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
@@ -36,6 +36,8 @@
 import org.opensearch.security.util.FakeRestRequest;
 
 public class OnBehalfOfAuthenticatorTest {
+    final static String enableOBO = "true";
+    final static String disableOBO = "false";
     final static String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16);
 
     final static String signingKey =
@@ -115,6 +117,38 @@ public void testInvalid() throws Exception {
         Assert.assertNull(credentials);
     }
 
+    @Test
+    public void testDisabled() throws Exception {
+        String jwsToken = Jwts.builder()
+            .setSubject("Leonard McCoy")
+            .setAudience("ext_0")
+            .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
+            .compact();
+
+        OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(disableOBOSettings());
+        Map<String, String> headers = new HashMap<String, String>();
+        headers.put("Authorization", "Bearer " + jwsToken);
+
+        AuthCredentials credentials = jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap<String, String>()), null);
+        Assert.assertNull(credentials);
+    }
+
+    @Test
+    public void testNonSpecifyOBOSetting() throws Exception {
+        String jwsToken = Jwts.builder()
+            .setSubject("Leonard McCoy")
+            .setAudience("ext_0")
+            .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
+            .compact();
+
+        OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(nonSpecifyOBOSetting());
+        Map<String, String> headers = new HashMap<String, String>();
+        headers.put("Authorization", "Bearer " + jwsToken);
+
+        AuthCredentials credentials = jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap<String, String>()), null);
+        Assert.assertNotNull(credentials);
+    }
+
     @Test
     public void testBearer() throws Exception {
 
@@ -297,7 +331,11 @@ private AuthCredentials extractCredentialsFromJwtHeader(
         final Boolean bwcPluginCompatibilityMode
     ) {
         final OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(
-            Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", encryptionKey).build()
+            Settings.builder()
+                .put("enabled", enableOBO)
+                .put("signing_key", signingKeyB64Encoded)
+                .put("encryption_key", encryptionKey)
+                .build()
         );
 
         final String jwsToken = jwtBuilder.signWith(
@@ -309,6 +347,22 @@ private AuthCredentials extractCredentialsFromJwtHeader(
     }
 
     private Settings defaultSettings() {
+        return Settings.builder()
+            .put("enabled", enableOBO)
+            .put("signing_key", signingKeyB64Encoded)
+            .put("encryption_key", claimsEncryptionKey)
+            .build();
+    }
+
+    private Settings disableOBOSettings() {
+        return Settings.builder()
+            .put("enabled", disableOBO)
+            .put("signing_key", signingKeyB64Encoded)
+            .put("encryption_key", claimsEncryptionKey)
+            .build();
+    }
+
+    private Settings nonSpecifyOBOSetting() {
         return Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build();
     }
 }
diff --git a/src/test/resources/restapi/securityconfig_nondefault.json b/src/test/resources/restapi/securityconfig_nondefault.json
@@ -172,6 +172,7 @@
     "hosts_resolver_mode" : "ip-only",
     "do_not_fail_on_forbidden_empty" : false,
     "on_behalf_of": {
+      "enabled": true,
       "signing_key": "VGhpcyBpcyB0aGUgand0IHNpZ25pbmcga2V5IGZvciBhbiBvbiBiZWhhbGYgb2YgdG9rZW4gYXV0aGVudGljYXRpb24gYmFja2VuZCBmb3IgdGVzdGluZyBvZiBleHRlbnNpb25z",
       "encryption_key": "ZW5jcnlwdGlvbktleQ=="
     }