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

Cluster Api security #125

Merged
merged 3 commits into from
Oct 31, 2022
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
20 changes: 20 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
<sonar-maven-plugin.version>3.9.1.2184</sonar-maven-plugin.version>
<spotless-maven-plugin.version>2.27.2</spotless-maven-plugin.version>
<swagger-annotations.version>2.1.0</swagger-annotations.version>
<jjwt-api.version>0.11.5</jjwt-api.version>
<jjwt-impl.version>0.11.5</jjwt-impl.version>
<jjwt-jackson.version>0.11.5</jjwt-jackson.version>
muralibasani marked this conversation as resolved.
Show resolved Hide resolved
</properties>

<dependencies>
Expand Down Expand Up @@ -234,6 +237,23 @@
<groupId>javax.mail</groupId>
<artifactId>javax.mail-api</artifactId>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-api</artifactId>
<version>${jjwt-api.version}</version>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-impl</artifactId>
<version>${jjwt-impl.version}</version>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-jackson</artifactId>
<version>${jjwt-jackson.version}</version>
<scope>runtime</scope>
</dependency>

</dependencies>

Expand Down
95 changes: 47 additions & 48 deletions src/main/java/io/aiven/klaw/service/ClusterApiService.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,36 +23,42 @@
import io.aiven.klaw.model.cluster.ClusterConnectorRequest;
import io.aiven.klaw.model.cluster.ClusterSchemaRequest;
import io.aiven.klaw.model.cluster.ClusterTopicRequest;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.Key;
import java.security.KeyManagementException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableKeyException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
import java.util.UUID;
import javax.annotation.PostConstruct;
import javax.crypto.spec.SecretKeySpec;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.codec.binary.Base64;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.conn.ssl.TrustStrategy;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.apache.tomcat.util.codec.binary.Base64;
import org.jasypt.util.text.BasicTextEncryptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.ParameterizedTypeReference;
Expand All @@ -72,15 +78,13 @@
@Slf4j
public class ClusterApiService {

public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
public static final String URL_DELIMITER = "/";
@Autowired ManageDatabase manageDatabase;
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final String URL_DELIMITER = "/";

@Value("${server.ssl.trust-store:null}")
private String trustStore;

@Value("${server.ssl.trust-store-password:null}")
private String trustStorePwd;
private static final String URI_CREATE_TOPICS = "/topics/createTopics";
private static final String URI_UPDATE_TOPICS = "/topics/updateTopics";
private static final String URI_DELETE_TOPICS = "/topics/deleteTopics";
@Autowired private ManageDatabase manageDatabase;

@Value("${server.ssl.key-store:null}")
private String keyStore;
Expand All @@ -91,29 +95,20 @@ public class ClusterApiService {
@Value("${server.ssl.key-store-type:JKS}")
private String keyStoreType;

protected static HttpComponentsClientHttpRequestFactory requestFactory;

static String URI_CREATE_TOPICS = "/topics/createTopics";
static String URI_UPDATE_TOPICS = "/topics/updateTopics";
static String URI_DELETE_TOPICS = "/topics/deleteTopics";

@Value("${klaw.jasypt.encryptor.secretkey}")
private String encryptorSecretKey;

@Value("${klaw.clusterapi.access.username}")
private String clusterApiUser;

@Value("${klaw.clusterapi.access.password}")
private String clusterApiPwd;
@Value("${klaw.clusterapi.access.base64.secret:null}")
muralibasani marked this conversation as resolved.
Show resolved Hide resolved
private String clusterApiAccessBase64Secret;

private static String clusterConnUrl;
protected static HttpComponentsClientHttpRequestFactory requestFactory;
RestTemplate httpRestTemplate, httpsRestTemplate;

public ClusterApiService(ManageDatabase manageDatabase) {
this.manageDatabase = manageDatabase;
}

RestTemplate httpRestTemplate, httpsRestTemplate;

private RestTemplate getRestTemplate() {
if (clusterConnUrl.toLowerCase().startsWith("https")) {
if (this.httpsRestTemplate == null) {
Expand Down Expand Up @@ -390,7 +385,7 @@ public String approveConnectorRequests(
uri = clusterConnUrl + uriGetTopics + "deleteConnector";
}

HttpHeaders headers = createHeaders(clusterApiUser, clusterApiPwd);
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_JSON);

HttpEntity<ClusterConnectorRequest> request =
Expand Down Expand Up @@ -457,7 +452,7 @@ public ResponseEntity<ApiResponse> approveTopicRequests(
uri = clusterConnUrl + URI_DELETE_TOPICS;
}

HttpHeaders headers = createHeaders(clusterApiUser, clusterApiPwd);
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_JSON);
HttpEntity<ClusterTopicRequest> request = new HttpEntity<>(clusterTopicRequest, headers);
response = getRestTemplate().postForEntity(uri, request, ApiResponse.class);
Expand Down Expand Up @@ -546,7 +541,7 @@ public ResponseEntity<ApiResponse> approveAclRequests(AclRequests aclReq, int te
clusterAclRequest.toBuilder().requestOperationType(RequestOperationType.DELETE).build();
}

HttpHeaders headers = createHeaders(clusterApiUser, clusterApiPwd);
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_JSON);

HttpEntity<ClusterAclRequest> request = new HttpEntity<>(clusterAclRequest, headers);
Expand Down Expand Up @@ -583,7 +578,7 @@ ResponseEntity<ApiResponse> postSchema(
.fullSchema(schemaRequest.getSchemafull())
.build();

HttpHeaders headers = createHeaders(clusterApiUser, clusterApiPwd);
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_JSON);

HttpEntity<ClusterSchemaRequest> request = new HttpEntity<>(clusterSchemaRequest, headers);
Expand Down Expand Up @@ -702,8 +697,7 @@ public Map<String, String> retrieveMetrics(String jmxUrl, String objectName)
String uriGetTopicsFull = clusterConnUrl + uriGetTopics;
RestTemplate restTemplate = getRestTemplate();

HttpHeaders headers =
createHeaders(clusterApiUser, clusterApiPwd); // createHeaders("user1", "pwd");
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
HttpEntity<MultiValueMap<String, String>> entity = new HttpEntity<>(params, headers);

Expand All @@ -719,16 +713,6 @@ public Map<String, String> retrieveMetrics(String jmxUrl, String objectName)
}
}

private HttpHeaders createHeaders(String username, String password) {
HttpHeaders httpHeaders = new HttpHeaders();
String auth = username + ":" + decodePwd(password);
byte[] encodedAuth = Base64.encodeBase64(auth.getBytes(StandardCharsets.US_ASCII));
String authHeader = "Basic " + new String(encodedAuth);
httpHeaders.set("Authorization", authHeader);

return httpHeaders;
}

// to connect to cluster api if https
@PostConstruct
private void setKwSSLContext() {
Expand Down Expand Up @@ -770,18 +754,33 @@ protected KeyStore getStore(String secret, String storeLoc)
return store;
}

private String decodePwd(String pwd) {
if (pwd != null) {
BasicTextEncryptor textEncryptor = new BasicTextEncryptor();
textEncryptor.setPasswordCharArray(encryptorSecretKey.toCharArray());
private HttpHeaders createHeaders(String username) {
HttpHeaders httpHeaders = new HttpHeaders();
String authHeader = "Bearer " + generateToken(username);
httpHeaders.set("Authorization", authHeader);

return httpHeaders;
}

return textEncryptor.decrypt(pwd);
}
return "";
private String generateToken(String username) {
Key hmacKey =
new SecretKeySpec(
Base64.decodeBase64(clusterApiAccessBase64Secret),
SignatureAlgorithm.HS256.getJcaName());
Instant now = Instant.now();

return Jwts.builder()
.claim("name", username)
.setSubject(username)
.setId(UUID.randomUUID().toString())
.setIssuedAt(Date.from(now))
.setExpiration(Date.from(now.plus(3L, ChronoUnit.MINUTES))) // expiry in 3 minutes
.signWith(hmacKey)
.compact();
}

private HttpEntity<String> getHttpEntity() {
HttpHeaders headers = createHeaders(clusterApiUser, clusterApiPwd);
HttpHeaders headers = createHeaders(clusterApiUser);
headers.setContentType(MediaType.APPLICATION_JSON);

headers.add("Accept", MediaType.APPLICATION_JSON_VALUE);
Expand Down
24 changes: 16 additions & 8 deletions src/main/java/io/aiven/klaw/service/ServerConfigService.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ public class ServerConfigService {

@Autowired private ClusterApiService clusterApiService;

@Autowired private CommonUtilsService commonUtilsService;
private final CommonUtilsService commonUtilsService;

private static List<ServerConfigProperties> listProps;

public ServerConfigService(Environment env) {
public ServerConfigService(Environment env, CommonUtilsService commonUtilsService) {
this.env = env;
this.commonUtilsService = commonUtilsService;
}

@PostConstruct
Expand All @@ -58,18 +59,21 @@ public void getAllProperties() {
log.info("All server properties being loaded");

List<ServerConfigProperties> listProps = new ArrayList<>();
List<String> allowedKeys =
Arrays.asList(
"spring.", "java.", "klaw.", "server.", "logging.", "management.", "endpoints.");
List<String> allowedKeys = Arrays.asList("spring.", "klaw.");

if (env instanceof ConfigurableEnvironment) {
for (PropertySource propertySource : ((ConfigurableEnvironment) env).getPropertySources()) {
for (PropertySource<?> propertySource :
((ConfigurableEnvironment) env).getPropertySources()) {
if (propertySource instanceof EnumerablePropertySource) {
for (String key : ((EnumerablePropertySource) propertySource).getPropertyNames()) {
for (String key : ((EnumerablePropertySource<?>) propertySource).getPropertyNames()) {

ServerConfigProperties props = new ServerConfigProperties();
props.setKey(key);
if (key.contains("password") || key.contains("license")) {
if (key.contains("password")

Choose a reason for hiding this comment

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

If I'm not mistaken String.contains is case sensitive. Depending on what is returned for "key", should that be considered here?

Copy link
Contributor Author

@muralibasani muralibasani Oct 24, 2022

Choose a reason for hiding this comment

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

@reethi-kotti-aiven Yes, but it may not be considered here. All the keys are already lower case now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw, thank you for the review.

|| key.contains("license")
|| key.contains("pwd")
|| key.contains("cert")
|| key.contains("secret")) {
props.setValue("*******");
} else {
props.setValue(WordUtils.wrap(propertySource.getProperty(key) + "", 125, "\n", true));
Expand All @@ -91,6 +95,10 @@ public void getAllProperties() {
}

public List<ServerConfigProperties> getAllProps() {
if (commonUtilsService.isNotAuthorizedUser(
getPrincipal(), PermissionType.UPDATE_SERVERCONFIG)) {
return new ArrayList<>();
}
return listProps;
}

Expand Down
7 changes: 4 additions & 3 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,17 @@ spring.cache.type=NONE
spring.thymeleaf.cache=false

# application shutdown and health properties
management.endpoints.web.exposure.include=*
management.endpoints.web.exposure.include=health,info,metrics
management.endpoints.web.exposure.exclude=
management.health.ldap.enabled=false
management.endpoint.shutdown.enabled=true
management.endpoint.shutdown.enabled=false

#jasypt encryption pwd secret key
klaw.jasypt.encryptor.secretkey=kw2021secretkey

# ClusterApi access
klaw.clusterapi.access.username=kwclusterapiuser
klaw.clusterapi.access.password=d7DtnvRR7jq05ODBkvxLIGO6Qa/bVpkW
klaw.clusterapi.access.base64.secret=

# Monitoring
klaw.monitoring.metrics.enable=false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ public void setUp() {
this.env = new Env();
env.setName("DEV");
ReflectionTestUtils.setField(clusterApiService, "httpRestTemplate", restTemplate);
ReflectionTestUtils.setField(clusterApiService, "clusterApiUser", "testuser");
ReflectionTestUtils.setField(
clusterApiService,
"clusterApiAccessBase64Secret",
"dGhpcyBpcyBhIHNlY3JldCB0byBhY2Nlc3MgY2x1c3RlcmFwaQ=="); // any base64 string

when(manageDatabase.getHandleDbRequests()).thenReturn(handleDbRequests);
when(manageDatabase.getKwPropertyValue(anyString(), anyInt())).thenReturn("http://cluster");
}
Expand Down
25 changes: 23 additions & 2 deletions src/test/java/io/aiven/klaw/service/ServerConfigServiceTest.java
Original file line number Diff line number Diff line change
@@ -1,35 +1,56 @@
package io.aiven.klaw.service;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

import io.aiven.klaw.model.ServerConfigProperties;
import java.util.List;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.core.env.Environment;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.test.context.junit.jupiter.SpringExtension;

@ExtendWith(SpringExtension.class)
public class ServerConfigServiceTest {

@Mock private CommonUtilsService commonUtilsService;
ServerConfigService serverConfigService;

@Mock private UserDetails userDetails;

private Environment env;

@BeforeEach
public void setUp() {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
this.env = context.getEnvironment();
loginMock();

serverConfigService = new ServerConfigService(env);
serverConfigService = new ServerConfigService(env, commonUtilsService);
}

@Test
public void getAllProps() {
when(commonUtilsService.isNotAuthorizedUser(any(), any())).thenReturn(false);
serverConfigService.getAllProperties();
List<ServerConfigProperties> list = serverConfigService.getAllProps();
assertThat(list).isNotEmpty();
assertThat(list).isEmpty(); // filtering for spring. and klaw.
}

private void loginMock() {
Authentication authentication = Mockito.mock(Authentication.class);
SecurityContext securityContext = Mockito.mock(SecurityContext.class);
when(securityContext.getAuthentication()).thenReturn(authentication);
when(authentication.getPrincipal()).thenReturn(userDetails);
SecurityContextHolder.setContext(securityContext);
}
}