Skip to content

Commit

Permalink
Client cert authentication (#637)
Browse files Browse the repository at this point in the history
* Add client certificate authentication.

* Fix tests.

* Don't provide client certificate for mail updates.
Pick up (optional) client certificate for syncupdates.

* Added test for client certificate authentication.

* More audit logging.

* Properly handle PEM certificate.

* Tidy up code, use X509CertificateWrapper for cert operations, added tests, handle Apache's SSL_CLIENT_CERT PEM format.

* Added feature toggle for client certificate authentication.

* Enable client certificate authentication in tests.

* Client cert authentication should not be enabled by default.

* Also accept FAILED SSL verify header (for self signed).
  • Loading branch information
sbusk authored Jun 22, 2020
1 parent 477248d commit c9883e5
Show file tree
Hide file tree
Showing 25 changed files with 620 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import net.ripe.db.whois.common.Latin1Conversion;
import net.ripe.db.whois.update.domain.ClientCertificateCredential;
import net.ripe.db.whois.update.domain.ContentWithCredentials;
import net.ripe.db.whois.update.domain.Credential;
import net.ripe.db.whois.update.domain.Credentials;
Expand Down Expand Up @@ -61,6 +62,8 @@ public List<Paragraph> createParagraphs(final ContentWithCredentials contentWith
baseCredentials.add(SsoCredential.createOfferedCredential(updateContext.getUserSession()));
}

updateContext.getClientCertificate().ifPresent(x509 -> baseCredentials.add(ClientCertificateCredential.createOfferedCredential(x509)));

final List<Paragraph> paragraphs = Lists.newArrayList();

int offset = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import javax.mail.internet.MimeMessage;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -220,6 +221,7 @@ private void handleMessageInContext(final String messageId, final MimeMessage me
mailMessageDao.setStatus(messageId, DequeueStatus.LOGGED);

final UpdateContext updateContext = new UpdateContext(loggerContext);
updateContext.setClientCertificate(Optional.empty());
final MailMessage mailMessage = messageParser.parse(message, updateContext);
mailMessageDao.setStatus(messageId, DequeueStatus.PARSED);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public Response update(final WhoisResources whoisResources,

try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);
updateContext.setBatchUpdate();

if(isQueryParamSet(dryRun)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package net.ripe.db.whois.api.rest;

import net.ripe.db.whois.common.DateTimeProvider;
import net.ripe.db.whois.update.keycert.X509CertificateWrapper;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.net.util.Base64;
import org.bouncycastle.x509.util.StreamParsingException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.http.HttpServletRequest;
import java.util.Optional;

public class ClientCertificateExtractor {

private static final Logger LOGGER = LoggerFactory.getLogger(ClientCertificateExtractor.class);

final static String HEADER_SSL_CLIENT_CERT = "SSL_CLIENT_CERT";
final static String HEADER_SSL_CLIENT_VERIFY = "SSL_CLIENT_VERIFY";

public static Optional<X509CertificateWrapper> getClientCertificate(final HttpServletRequest request,
final DateTimeProvider dateTimeProvider) {
final String sslClientCert = request.getHeader(HEADER_SSL_CLIENT_CERT);

if (StringUtils.isBlank(sslClientCert)) {
return Optional.empty();
}

if (!hasAcceptableVerifyHeader(request)) {
return Optional.empty();
}

return getX509Certificate(sslClientCert, dateTimeProvider);
}

private static boolean hasAcceptableVerifyHeader(final HttpServletRequest request) {
final String sslClientVerify = request.getHeader(HEADER_SSL_CLIENT_VERIFY);

return StringUtils.isNotEmpty(sslClientVerify) &&
("GENEROUS".equals(sslClientVerify) ||
"SUCCESS".equals(sslClientVerify) ||
sslClientVerify.startsWith("FAILED"));
}

private static Optional<X509CertificateWrapper> getX509Certificate(final String certificate,
final DateTimeProvider dateTimeProvider) {
String fingerprint;
try {
// the PEM cert provided by Apache in SSL_CLIENT_CERT has spaces that JCA doesn't like so we decode it ourselves:
final byte[] bytes = Base64.decodeBase64(
certificate
.replace(X509CertificateWrapper.X509_HEADER, "")
.replace(X509CertificateWrapper.X509_FOOTER, "")
.replaceAll(" ", "")
);
// TODO we should probably let the servlet container handle this for us and just use javax.servlet.request.X509Certificate

final X509CertificateWrapper x509CertificateWrapper = X509CertificateWrapper.parse(bytes);
fingerprint = x509CertificateWrapper.getFingerprint();

if (x509CertificateWrapper.isNotYetValid(dateTimeProvider)) {
LOGGER.info("Client certificate {} is not yet valid", fingerprint);
return Optional.empty();
}

if (x509CertificateWrapper.isExpired(dateTimeProvider)) {
LOGGER.info("Client certificate {} has expired", fingerprint);
return Optional.empty();
}

return Optional.of(x509CertificateWrapper);
} catch (StreamParsingException e) {
LOGGER.info("Invalid X.509 certificate");
}

return Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import net.ripe.db.whois.api.rest.mapper.WhoisObjectMapper;
import net.ripe.db.whois.common.rpsl.ObjectType;
import net.ripe.db.whois.common.rpsl.RpslObject;
import net.ripe.db.whois.common.sso.UserSession;
import net.ripe.db.whois.update.domain.ClientCertificateCredential;
import net.ripe.db.whois.update.domain.Credential;
import net.ripe.db.whois.update.domain.Credentials;
import net.ripe.db.whois.update.domain.Keyword;
Expand Down Expand Up @@ -90,12 +90,12 @@ public Response create(
try {
final Origin origin = updatePerformer.createOrigin(request);

final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);
updateContext.setBatchUpdate();

auditlogRequest(request);

final Credentials credentials = createCredentials(updateContext.getUserSession(), passwords);
final Credentials credentials = createCredentials(updateContext, passwords);

final List<Update> updates = extractUpdates(resources, credentials);

Expand Down Expand Up @@ -186,17 +186,20 @@ private List<Update> extractUpdates(final WhoisResources whoisResources, final C
return result;
}

private Credentials createCredentials(final UserSession userSession, final List<String> passwords) {
private Credentials createCredentials(final UpdateContext updateContext, final List<String> passwords) {

final Set<Credential> credentials = Sets.newHashSet();

for (String password : passwords) {
credentials.add(new PasswordCredential(password));
}

if (userSession != null) {
credentials.add(SsoCredential.createOfferedCredential(userSession));
if (updateContext.getUserSession() != null) {
credentials.add(SsoCredential.createOfferedCredential(updateContext.getUserSession()));
}

updateContext.getClientCertificate().ifPresent(x509 -> credentials.add(ClientCertificateCredential.createOfferedCredential(x509)));

return new Credentials(credentials);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import net.ripe.db.whois.common.sso.CrowdClientException;
import net.ripe.db.whois.common.sso.SsoTokenTranslator;
import net.ripe.db.whois.update.domain.Action;
import net.ripe.db.whois.update.domain.ClientCertificateCredential;
import net.ripe.db.whois.update.domain.Credential;
import net.ripe.db.whois.update.domain.Credentials;
import net.ripe.db.whois.update.domain.Keyword;
Expand Down Expand Up @@ -73,10 +74,11 @@ public InternalUpdatePerformer(final UpdateRequestHandler updateRequestHandler,
this.ssoTokenTranslator = ssoTokenTranslator;
}

public UpdateContext initContext(final Origin origin, final String ssoToken) {
public UpdateContext initContext(final Origin origin, final String ssoToken, final HttpServletRequest request) {
loggerContext.init(getRequestId(origin.getFrom()));
final UpdateContext updateContext = new UpdateContext(loggerContext);
setSsoSessionToContext(updateContext, ssoToken);
updateContext.setClientCertificate(ClientCertificateExtractor.getClientCertificate(request, dateTimeProvider));
return updateContext;
}

Expand Down Expand Up @@ -202,6 +204,8 @@ private static Paragraph createParagraph(final UpdateContext updateContext, fina
credentials.add(SsoCredential.createOfferedCredential(updateContext.getUserSession()));
}

updateContext.getClientCertificate().ifPresent(x509 -> credentials.add(ClientCertificateCredential.createOfferedCredential(x509)));

return new Paragraph(rpslObject.toString(), new Credentials(credentials));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public WhoisResources performUpdates(

try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);
updateContext.setBatchUpdate();
auditlogRequest(request);

Expand Down Expand Up @@ -493,7 +493,7 @@ private Response performUpdate(
final String crowdTokenKey) {
try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);

auditlogRequest(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ private Response doSyncUpdate(final HttpServletRequest httpServletRequest, final
final UpdateContext updateContext = new UpdateContext(loggerContext);

setSsoSessionToContext(updateContext, request.getSsoToken());
updateContext.setClientCertificate(ClientCertificateExtractor.getClientCertificate(httpServletRequest, dateTimeProvider));

final String content = request.hasParam("DATA") ? request.getParam("DATA") : "";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public Response delete(

try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);

if(requiresNonAuthRedirect(source, objectType, key)) {
return redirectNonAuthOrRequiresRipeRedirect(sourceContext.getNonauthSource().getName().toString(), objectType, key, request.getQueryString());
Expand Down Expand Up @@ -165,7 +165,7 @@ public Response update(

try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);

if(requiresNonAuthRedirect(source, objectType, key)) {
return redirectNonAuthOrRequiresRipeRedirect(sourceContext.getNonauthSource().getName().toString(), objectType, key, request.getQueryString());
Expand Down Expand Up @@ -220,7 +220,7 @@ public Response create(

try {
final Origin origin = updatePerformer.createOrigin(request);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey);
final UpdateContext updateContext = updatePerformer.initContext(origin, crowdTokenKey, request);

auditLogRequest(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import net.ripe.db.whois.update.domain.UpdateContext;
import net.ripe.db.whois.update.domain.UpdateMessages;
import org.apache.commons.io.IOUtils;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
Expand All @@ -21,6 +22,7 @@
import org.springframework.core.io.ClassPathResource;

import java.util.List;
import java.util.Optional;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
Expand All @@ -33,6 +35,7 @@
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
public class UpdatesParserTest {
Expand All @@ -43,6 +46,11 @@ public class UpdatesParserTest {

@InjectMocks UpdatesParser subject = new UpdatesParser(1000000);

@Before
public void setup() {
when(updateContext.getClientCertificate()).thenReturn(Optional.empty());
}

@Test
public void no_paragraphs() {
final List<Update> updates = subject.parse(updateContext, Lists.newArrayList());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package net.ripe.db.whois.api.rest;

import net.ripe.db.whois.common.ClockDateTimeProvider;
import net.ripe.db.whois.common.TestDateTimeProvider;
import net.ripe.db.whois.update.keycert.X509CertificateTestUtil;
import org.junit.Test;

import javax.servlet.http.HttpServletRequest;

import java.time.LocalDateTime;

import static net.ripe.db.whois.api.rest.ClientCertificateExtractor.HEADER_SSL_CLIENT_CERT;
import static net.ripe.db.whois.api.rest.ClientCertificateExtractor.HEADER_SSL_CLIENT_VERIFY;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ClientCertificateExtractorTest {

static String cert;

static {
try {
cert = X509CertificateTestUtil.asPem(X509CertificateTestUtil.generate("test-cn", new ClockDateTimeProvider()));
} catch (Exception e) {
throw new RuntimeException(e);
}
}

@Test
public void testValidCertificate() {
final HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader(HEADER_SSL_CLIENT_CERT)).thenReturn(cert);
when(request.getHeader(HEADER_SSL_CLIENT_VERIFY)).thenReturn("GENEROUS");

assertThat(ClientCertificateExtractor.getClientCertificate(request, new ClockDateTimeProvider()).isPresent(), is(true));
}

@Test
public void testInvalidVerifyHeader() {
final HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader(HEADER_SSL_CLIENT_CERT)).thenReturn(cert);
when(request.getHeader(HEADER_SSL_CLIENT_VERIFY)).thenReturn("NOT-ACCEPTED");

assertThat(ClientCertificateExtractor.getClientCertificate(request, new ClockDateTimeProvider()).isPresent(), is(false));
}

@Test
public void testNoCertificate() {
final HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader(HEADER_SSL_CLIENT_VERIFY)).thenReturn("GENEROUS");

assertThat(ClientCertificateExtractor.getClientCertificate(request, new ClockDateTimeProvider()).isPresent(), is(false));
}

@Test
public void testCertificateExpired() {
final HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader(HEADER_SSL_CLIENT_CERT)).thenReturn(cert);
when(request.getHeader(HEADER_SSL_CLIENT_VERIFY)).thenReturn("GENEROUS");

final TestDateTimeProvider testDateTimeProvider = new TestDateTimeProvider();
testDateTimeProvider.setTime(LocalDateTime.now().plusYears(5));

assertThat(ClientCertificateExtractor.getClientCertificate(request, testDateTimeProvider).isPresent(), is(false));
}

@Test
public void testCertificateNotYetValid() {
final HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader(HEADER_SSL_CLIENT_CERT)).thenReturn(cert);
when(request.getHeader(HEADER_SSL_CLIENT_VERIFY)).thenReturn("GENEROUS");

final TestDateTimeProvider testDateTimeProvider = new TestDateTimeProvider();
testDateTimeProvider.setTime(LocalDateTime.now().minusMonths(1));

assertThat(ClientCertificateExtractor.getClientCertificate(request, testDateTimeProvider).isPresent(), is(false));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import net.ripe.db.whois.update.domain.UpdateContext;
import net.ripe.db.whois.update.handler.UpdateRequestHandler;
import net.ripe.db.whois.update.log.LoggerContext;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
Expand All @@ -28,6 +29,7 @@
import javax.servlet.http.HttpServletRequest;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.Optional;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
Expand All @@ -51,6 +53,11 @@ public class InternalUpdatePerformerTest {
@Mock private UpdateContext updateContextMock;
@InjectMocks private InternalUpdatePerformer subject;

@Before
public void setup() {
when(updateContextMock.getClientCertificate()).thenReturn(Optional.empty());
}

@Test
public void create_update_with_override_no_passwords() {
final RpslObject object = RpslObject.parse(
Expand Down
Loading

0 comments on commit c9883e5

Please sign in to comment.