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

[#1099] adjusting new logic and configuration for security #1104

Merged
merged 2 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@ -25,4 +25,6 @@ public class Const {
public static final String AUTH_SERVICE_NAME = "serviceName";

public static final String AUTH_TOKEN_CHECK_ENABLED = "spring.cloud.servicecomb.webmvc.publicKey.tokenCheckEnabled";

public static final String AUTH_TOKEN_HEADER_KEY = "spring.cloud.servicecomb.webmvc.publicKey.headerTokenKey";
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,24 @@ public RSAProviderTokenManager(List<AccessController> accessControllers, Environ
this.authenticationAdapter = authenticationAdapter;
}

/**
* 1.tokenCheckEnabled is true or request headers has no serviceName, use serviceId and instanceId for authentication
* in token.
* 2.tokenCheckEnabled is false and request headers has serviceName, use serviceName for authentication in request
* header.
*
* @param request
* @throws Exception
*/
public void valid(HttpServletRequest request) throws Exception {
try {
AuthRequestExtractor extractor;
if (environment.getProperty(Const.AUTH_TOKEN_CHECK_ENABLED, boolean.class, true)
|| StringUtils.isEmpty(request.getHeader(Const.AUTH_SERVICE_NAME))) {
RsaAuthenticationToken rsaToken = RSATokenCheckUtils.checkTokenInfo(request, authenticationAdapter);
String headerTokenKey = environment.getProperty(Const.AUTH_TOKEN_HEADER_KEY, String.class,
"X-SM-Token");
RsaAuthenticationToken rsaToken = RSATokenCheckUtils.checkTokenInfo(request, authenticationAdapter,
headerTokenKey);
extractor = AuthRequestExtractorUtils.createAuthRequestExtractor(request, rsaToken.getServiceId(),
rsaToken.getInstanceId());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public class RSATokenCheckUtils {
.build();

public static RsaAuthenticationToken checkTokenInfo(HttpServletRequest request,
AuthenticationAdapter authenticationAdapter) throws Exception {
String token = request.getHeader(Const.AUTH_TOKEN);
AuthenticationAdapter authenticationAdapter, String headerTokenKey) throws Exception {
String token = request.getHeader(headerTokenKey);
if (StringUtils.isEmpty(token)) {
token = InvocationContextHolder.getOrCreateInvocationContext().getContext(Const.AUTH_TOKEN);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't fallback to X-Auth-Token, throws error here directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

microservice call microservice is X-Auth-Token, no affected with header token.

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,12 @@ public boolean isAllowed(AuthRequestExtractor extractor) throws Exception {

private boolean checkDeny(String serviceName, AuthRequestExtractor extractor) {
if (securityPolicyProperties.matchDeny(serviceName, extractor.uri(), extractor.method())) {
// both permissive and enforcing model need print logs(send alarm info).
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logging level should be WARN if denied in permissive

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change "unauthorized request" to "[autoauthz] request denied. consumer={} ....."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use security team context

serviceName, securityPolicyProperties.getProvider(), extractor.uri(), extractor.method(),
System.currentTimeMillis());
// permissive mode, black policy match allow passing
if ("permissive".equals(securityPolicyProperties.getMode())) {
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
serviceName, securityPolicyProperties.getProvider(), extractor.uri(), extractor.method(),
System.currentTimeMillis());
return false;
} else {
return true;
}
return !"permissive".equals(securityPolicyProperties.getMode());
} else {
return false;
}
Expand All @@ -72,15 +69,12 @@ private boolean checkAllowAndDeny(String serviceName, AuthRequestExtractor extra
if (securityPolicyProperties.matchAllow(serviceName, extractor.uri(), extractor.method())) {
return !checkDeny(serviceName, extractor);
} else {
// both permissive and enforcing model need print logs(send alarm info).
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above. Logging level shoud be WARN and message should be "request denied ...."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

serviceName, securityPolicyProperties.getProvider(), extractor.uri(), extractor.method(),
System.currentTimeMillis());
// permissive mode, white policy not match allow passing
if ("permissive".equals(securityPolicyProperties.getMode())) {
LOGGER.info("[autoauthz unauthorized request] consumer={}, provider={}, path={}, method={}, timestamp={}",
serviceName, securityPolicyProperties.getProvider(), extractor.uri(), extractor.method(),
System.currentTimeMillis());
return true;
} else {
return false;
}
return "permissive".equals(securityPolicyProperties.getMode());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void setAction(Action action) {

public boolean matchAllow(String serviceName, String uri, String method) {
if (action == null || action.allow.isEmpty()) {
return true;
return false;
}

for (ConfigurationItem item : action.allow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ public void testAllowEnforcingNotMatch() throws Exception {

@Test
public void testDenyEnforcingNotMatch() throws Exception {
AuthRequestExtractor extractor = createAuthRequestExtractor("/checkToken");
Assertions.assertTrue(getDenyAccessController("enforcing")
AuthRequestExtractor extractor = createAuthRequestExtractor("/checkTokenSecurityAllow");
Assertions.assertTrue(getBothAccessController("enforcing")
.isAllowed(extractor));
}

Expand Down
Loading