Skip to content

Commit

Permalink
[Backport 2.x] Expanding Authentication with SecurityRequest Abstract…
Browse files Browse the repository at this point in the history 
…ion (#3430)

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 <[email protected]>
(cherry picked from commit f435c05)
  • Loading branch information
@peternied
peternied committed Oct 6, 2023
1 parent 84d9dd8 commit 106fbc6
Show file tree
Hide file tree
Showing 52 changed files with 928 additions and 475 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.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);

Check warning on line 156 in src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L156 

Added line #L156 was not covered by tests
} else {
// just consume to avoid "contains unrecognized parameter"
request.param(jwtUrlParameter);
request.params().get(jwtUrlParameter);

Check warning on line 159 in src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L159 

Added line #L159 was not covered by tests
}
}

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\""), "")

Check warning on line 245 in src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L244-L245 

Added lines #L244 - L245 were not covered by tests
);
}

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,6 +11,8 @@

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.Key;
Expand All @@ -21,6 +23,8 @@
import java.security.spec.InvalidKeySpecException;
import java.security.spec.X509EncodedKeySpec;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.Map.Entry;
import java.util.regex.Pattern;

Expand All @@ -30,18 +34,18 @@
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.io.Decoders;
import io.jsonwebtoken.security.WeakKeyException;
import org.apache.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;

public class HTTPJwtAuthenticator implements HTTPAuthenticator {
Expand Down Expand Up @@ -103,8 +107,8 @@ public HTTPJwtAuthenticator(final Settings settings, final 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");
requireAudience = settings.get("required_audience");
Expand Down Expand Up @@ -136,7 +140,8 @@ public JwtParser run() {

@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 @@ -153,7 +158,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 @@ -165,10 +170,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 @@ -223,18 +228,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\""), "")

Check warning on line 233 in src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java#L232-L233 

Added lines #L232 - L233 were not covered by tests
);
}

@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 @@ -258,7 +263,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();

Check warning on line 289 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L287-L289 

Added lines #L287 - L289 were not covered by tests
if (negotiateResponseBody != null) {
responseBody = negotiateResponseBody;
headers.putAll(SecurityResponse.CONTENT_TYPE_APP_JSON);

Check warning on line 292 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L291-L292 

Added lines #L291 - L292 were not covered by tests
}

if (credentials == null || credentials.getNativeCredentials() == null) {
wwwAuthenticateResponse.addHeader("WWW-Authenticate", "Negotiate");
if (creds == null || creds.getNativeCredentials() == null) {
headers.put("WWW-Authenticate", "Negotiate");

Check warning on line 296 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L296 

Added line #L296 was not covered by tests
} else {
wwwAuthenticateResponse.addHeader(
"WWW-Authenticate",
"Negotiate " + Base64.getEncoder().encodeToString((byte[]) credentials.getNativeCredentials())
);
headers.put("WWW-Authenticate", "Negotiate " + Base64.getEncoder().encodeToString((byte[]) creds.getNativeCredentials()));

Check warning on line 298 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L298 

Added line #L298 was not covered by tests
}
return wwwAuthenticateResponse;

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

Check warning on line 301 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L301 

Added line #L301 was not covered by tests
}

@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();

Check warning on line 385 in src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java#L385 

Added line #L385 was not covered by tests
} catch (Exception ex) {
log.error("Can't construct response body", ex);
return null;
Expand Down
Loading

0 comments on commit 106fbc6

Please sign in to comment.