Skip to content

Commit

Permalink
Expanding Authentication with SecurityRequest Abstraction (#3430)
Browse files Browse the repository at this point in the history 
Introduced a new abstraction, SecurityRequest & SecurityRequestChannel,
to streamline and secure the authentication process in the OpenSearch
Security plugin. By isolating the essential request components needed
for authentication, we minimize potential risks associated with previous
designs and provide a more maintainable architecture.

Signed-off-by: Peter Nied <petern@amazon.com>
  • Loading branch information
@peternied
peternied authored Oct 6, 2023
1 parent bfba97a commit f435c05
Show file tree
Hide file tree
Showing 56 changed files with 978 additions and 499 deletions.
33 changes: 19 additions & 14 deletions src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@

package com.amazon.dlic.auth.http.jwt;

import static org.apache.http.HttpHeaders.AUTHORIZATION;

import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.Map.Entry;
import java.util.regex.Pattern;

import com.google.common.annotations.VisibleForTesting;
import org.apache.cxf.rs.security.jose.jwt.JwtClaims;
import org.apache.cxf.rs.security.jose.jwt.JwtToken;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.http.HttpStatus;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -35,10 +39,10 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.common.Strings;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.user.AuthCredentials;

public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator {
Expand All @@ -62,8 +66,8 @@ public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator

public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {
jwtUrlParameter = settings.get("jwt_url_parameter");
jwtHeaderName = settings.get("jwt_header", HttpHeaders.AUTHORIZATION);
isDefaultAuthHeader = HttpHeaders.AUTHORIZATION.equalsIgnoreCase(jwtHeaderName);
jwtHeaderName = settings.get("jwt_header", AUTHORIZATION);
isDefaultAuthHeader = AUTHORIZATION.equalsIgnoreCase(jwtHeaderName);
rolesKey = settings.get("roles_key");
subjectKey = settings.get("subject_key");
clockSkewToleranceSeconds = settings.getAsInt("jwt_clock_skew_tolerance_seconds", DEFAULT_CLOCK_SKEW_TOLERANCE_SECONDS);
Expand All @@ -82,7 +86,8 @@ public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {

@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context) throws OpenSearchSecurityException {
public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext context)
throws OpenSearchSecurityException {
final SecurityManager sm = System.getSecurityManager();

if (sm != null) {
Expand All @@ -99,7 +104,7 @@ public AuthCredentials run() {
return creds;
}

private AuthCredentials extractCredentials0(final RestRequest request) throws OpenSearchSecurityException {
private AuthCredentials extractCredentials0(final SecurityRequest request) throws OpenSearchSecurityException {

String jwtString = getJwtTokenString(request);

Expand Down Expand Up @@ -140,18 +145,18 @@ private AuthCredentials extractCredentials0(final RestRequest request) throws Op

}

protected String getJwtTokenString(RestRequest request) {
protected String getJwtTokenString(SecurityRequest request) {
String jwtToken = request.header(jwtHeaderName);
if (isDefaultAuthHeader && jwtToken != null && BASIC.matcher(jwtToken).matches()) {
jwtToken = null;
}

if (jwtUrlParameter != null) {
if (jwtToken == null || jwtToken.isEmpty()) {
jwtToken = request.param(jwtUrlParameter);
jwtToken = request.params().get(jwtUrlParameter);
} else {
// just consume to avoid "contains unrecognized parameter"
request.param(jwtUrlParameter);
request.params().get(jwtUrlParameter);
}
}

Expand Down Expand Up @@ -235,10 +240,10 @@ public String[] extractRoles(JwtClaims claims) {
protected abstract KeyProvider initKeyProvider(Settings settings, Path configPath) throws Exception;

@Override
public BytesRestResponse reRequestAuthentication(RestRequest request, AuthCredentials authCredentials) {
final BytesRestResponse wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, "");
wwwAuthenticateResponse.addHeader("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\"");
return wwwAuthenticateResponse;
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials authCredentials) {
return Optional.of(
new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, Map.of("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\""), "")
);
}

public String getRequiredAudience() {
Expand Down
37 changes: 21 additions & 16 deletions src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,33 @@

package com.amazon.dlic.auth.http.jwt;

import static org.apache.http.HttpHeaders.AUTHORIZATION;

import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.Map.Entry;
import java.util.regex.Pattern;

import io.jsonwebtoken.Claims;
import io.jsonwebtoken.JwtParser;
import io.jsonwebtoken.JwtParserBuilder;
import io.jsonwebtoken.security.WeakKeyException;
import org.apache.hc.core5.http.HttpHeaders;

import org.apache.http.HttpStatus;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.OpenSearchSecurityException;
import org.opensearch.SpecialPermission;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.user.AuthCredentials;
import org.opensearch.security.util.KeyUtils;

Expand All @@ -58,8 +62,8 @@ public HTTPJwtAuthenticator(final Settings settings, final Path configPath) {

String signingKey = settings.get("signing_key");
jwtUrlParameter = settings.get("jwt_url_parameter");
jwtHeaderName = settings.get("jwt_header", HttpHeaders.AUTHORIZATION);
isDefaultAuthHeader = HttpHeaders.AUTHORIZATION.equalsIgnoreCase(jwtHeaderName);
jwtHeaderName = settings.get("jwt_header", AUTHORIZATION);
isDefaultAuthHeader = AUTHORIZATION.equalsIgnoreCase(jwtHeaderName);
rolesKey = settings.get("roles_key");
subjectKey = settings.get("subject_key");
requireAudience = settings.get("required_audience");
Expand All @@ -83,7 +87,8 @@ public HTTPJwtAuthenticator(final Settings settings, final Path configPath) {

@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context) throws OpenSearchSecurityException {
public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext context)
throws OpenSearchSecurityException {
final SecurityManager sm = System.getSecurityManager();

if (sm != null) {
Expand All @@ -100,7 +105,7 @@ public AuthCredentials run() {
return creds;
}

private AuthCredentials extractCredentials0(final RestRequest request) {
private AuthCredentials extractCredentials0(final SecurityRequest request) {
if (jwtParser == null) {
log.error("Missing Signing Key. JWT authentication will not work");
return null;
Expand All @@ -112,10 +117,10 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
}

if ((jwtToken == null || jwtToken.isEmpty()) && jwtUrlParameter != null) {
jwtToken = request.param(jwtUrlParameter);
jwtToken = request.params().get(jwtUrlParameter);
} else {
// just consume to avoid "contains unrecognized parameter"
request.param(jwtUrlParameter);
request.params().get(jwtUrlParameter);
}

if (jwtToken == null || jwtToken.length() == 0) {
Expand Down Expand Up @@ -170,18 +175,18 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
}

@Override
public BytesRestResponse reRequestAuthentication(RestRequest request, AuthCredentials credentials) {
final BytesRestResponse wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, "");
wwwAuthenticateResponse.addHeader("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\"");
return wwwAuthenticateResponse;
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest channel, AuthCredentials creds) {
return Optional.of(
new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, Map.of("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\""), "")
);
}

@Override
public String getType() {
return "jwt";
}

protected String extractSubject(final Claims claims, final RestRequest request) {
protected String extractSubject(final Claims claims, final SecurityRequest request) {
String subject = claims.getSubject();
if (subjectKey != null) {
// try to get roles from claims, first as Object to avoid having to catch the ExpectedTypeException
Expand All @@ -205,7 +210,7 @@ protected String extractSubject(final Claims claims, final RestRequest request)
}

@SuppressWarnings("unchecked")
protected String[] extractRoles(final Claims claims, final RestRequest request) {
protected String[] extractRoles(final Claims claims, final SecurityRequest request) {
// no roles key specified
if (rolesKey == null) {
return new String[0];
Expand Down
47 changes: 24 additions & 23 deletions src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

package com.amazon.dlic.auth.http.kerberos;

import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;

import java.io.Serializable;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -22,13 +24,17 @@
import java.security.PrivilegedExceptionAction;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import javax.security.auth.Subject;
import javax.security.auth.login.LoginException;

import com.google.common.base.Strings;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.ietf.jgss.GSSContext;
Expand All @@ -48,15 +54,13 @@
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.env.Environment;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.user.AuthCredentials;

public class HTTPSpnegoAuthenticator implements HTTPAuthenticator {

private static final String EMPTY_STRING = "";
private static final Oid[] KRB_OIDS = new Oid[] { KrbConstants.SPNEGO, KrbConstants.KRB5MECH };

protected final Logger log = LogManager.getLogger(this.getClass());
Expand Down Expand Up @@ -170,7 +174,7 @@ public Void run() {

@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(final RestRequest request, ThreadContext threadContext) {
public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext threadContext) {
final SecurityManager sm = System.getSecurityManager();

if (sm != null) {
Expand All @@ -187,7 +191,7 @@ public AuthCredentials run() {
return creds;
}

private AuthCredentials extractCredentials0(final RestRequest request) {
private AuthCredentials extractCredentials0(final SecurityRequest request) {

if (acceptorPrincipal == null || acceptorKeyTabPath == null) {
log.error("Missing acceptor principal or keytab configuration. Kerberos authentication will not work");
Expand Down Expand Up @@ -279,25 +283,22 @@ public GSSCredential run() throws GSSException {
}

@Override
public BytesRestResponse reRequestAuthentication(RestRequest request, AuthCredentials credentials) {
final BytesRestResponse wwwAuthenticateResponse;
XContentBuilder response = getNegotiateResponseBody();

if (response != null) {
wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, response);
} else {
wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, EMPTY_STRING);
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) {
final Map<String, String> headers = new HashMap<>();
String responseBody = "";
final String negotiateResponseBody = getNegotiateResponseBody();
if (negotiateResponseBody != null) {
responseBody = negotiateResponseBody;
headers.putAll(SecurityResponse.CONTENT_TYPE_APP_JSON);
}

if (credentials == null || credentials.getNativeCredentials() == null) {
wwwAuthenticateResponse.addHeader("WWW-Authenticate", "Negotiate");
if (creds == null || creds.getNativeCredentials() == null) {
headers.put("WWW-Authenticate", "Negotiate");
} else {
wwwAuthenticateResponse.addHeader(
"WWW-Authenticate",
"Negotiate " + Base64.getEncoder().encodeToString((byte[]) credentials.getNativeCredentials())
);
headers.put("WWW-Authenticate", "Negotiate " + Base64.getEncoder().encodeToString((byte[]) creds.getNativeCredentials()));
}
return wwwAuthenticateResponse;

return Optional.of(new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody));
}

@Override
Expand Down Expand Up @@ -369,7 +370,7 @@ private static String getUsernameFromGSSContext(final GSSContext gssContext, fin
return null;
}

private XContentBuilder getNegotiateResponseBody() {
private String getNegotiateResponseBody() {
try {
XContentBuilder negotiateResponseBody = XContentFactory.jsonBuilder();
negotiateResponseBody.startObject();
Expand All @@ -381,7 +382,7 @@ private XContentBuilder getNegotiateResponseBody() {
negotiateResponseBody.endObject();
negotiateResponseBody.endObject();
negotiateResponseBody.endObject();
return negotiateResponseBody;
return negotiateResponseBody.toString();
} catch (Exception ex) {
log.error("Can't construct response body", ex);
return null;
Expand Down
Loading

0 comments on commit f435c05

Please sign in to comment.