-
Notifications
You must be signed in to change notification settings - Fork 33
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
BFD-3723: Determine SAMHSA authorization based on certificate identity #2486
Open
aschey-forpeople
wants to merge
29
commits into
master
Choose a base branch
from
BFD-3723
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+928
−91
Open
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
7dbff73
add request filter to check client cert
aschey-forpeople f99e668
Merge remote-tracking branch 'origin/master' into BFD-3723
aschey-forpeople f9367e5
allow samhsa based on cert alias
aschey-forpeople 0d659f7
rename SSM parameter, fix javadocs
aschey-forpeople 86bdf19
fix claim tests
aschey-forpeople e4b1679
fix javadocs
aschey-forpeople 0274892
Merge branch 'master' into BFD-3723
aschey-forpeople 8a29949
always forget the periods
aschey-forpeople 088f89e
fix missing parameter
aschey-forpeople 12acb0d
add test certs with samhsa access
aschey-forpeople e949844
make test cert script configurable
aschey-forpeople 0c5960c
add samhsa test certs for ephemeral envs
aschey-forpeople 555707b
Merge branch 'master' into BFD-3723
aschey-forpeople 6ec95ca
switch to jackson
aschey-forpeople 133a488
Merge branch 'master' into BFD-3723
aschey-forpeople 3775cd5
Merge branch 'master' into BFD-3723
aschey-forpeople 4f9ab8f
remove unused cert from allowlist
aschey-forpeople 6166f4b
add new cert for regression tests
aschey-forpeople 1aa9dd4
change cert for load tests
aschey-forpeople 17b1f77
fix copy/paste fail
aschey-forpeople ed0c83e
Update apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/serve…
aschey-forpeople b7bda3d
Update apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/serve…
aschey-forpeople 0fdf201
Update apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/serve…
aschey-forpeople 43d45f2
Update apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/serve…
aschey-forpeople 47e6999
Update apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/serve…
aschey-forpeople ace9eab
Update apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/serve…
aschey-forpeople b21543a
Update apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/serve…
aschey-forpeople b70ed60
Merge branch 'master' into BFD-3723
aschey-forpeople 053122d
fmt
aschey-forpeople File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
101 changes: 101 additions & 0 deletions
101
apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/AllowSamhsaFilter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
package gov.cms.bfd.server.war; | ||
|
||
import static gov.cms.bfd.server.war.SpringConfiguration.SSM_PATH_SAMHSA_ALLOWED_CERTIFICATE_ALIASES_JSON; | ||
import static gov.cms.bfd.server.war.commons.CommonTransformerUtils.SHOULD_FILTER_SAMHSA; | ||
|
||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import gov.cms.bfd.server.war.commons.ClientCertificateUtils; | ||
import jakarta.servlet.FilterChain; | ||
import jakarta.servlet.ServletException; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
import jakarta.servlet.http.HttpServletResponse; | ||
import java.io.IOException; | ||
import java.math.BigInteger; | ||
import java.security.KeyStore; | ||
import java.security.KeyStoreException; | ||
import java.security.cert.X509Certificate; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.stream.Collectors; | ||
import org.jetbrains.annotations.NotNull; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Qualifier; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.stereotype.Component; | ||
import org.springframework.web.filter.OncePerRequestFilter; | ||
|
||
/*** | ||
aschey-forpeople marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Filter class to add metadata to the request that says whether clients can see SAMHSA data or not. | ||
*/ | ||
@Component("AllowSamhsaFilterBean") | ||
public class AllowSamhsaFilter extends OncePerRequestFilter { | ||
|
||
/** The logger. */ | ||
private static final Logger LOGGER = LoggerFactory.getLogger(AllowSamhsaFilter.class); | ||
|
||
/** List of allowed certificate serial numbers. */ | ||
private final List<BigInteger> samhsaAllowedSerialNumbers; | ||
|
||
/** | ||
* Creates a new {@link AllowSamhsaFilter}. | ||
* | ||
* @param samhsaAllowedCertificateAliasesJson list of certificate aliases to identify clients that | ||
* are allowed to see SAMHSA data | ||
* @param keyStore server key store | ||
*/ | ||
public AllowSamhsaFilter( | ||
@Value("${" + SSM_PATH_SAMHSA_ALLOWED_CERTIFICATE_ALIASES_JSON + "}") | ||
String samhsaAllowedCertificateAliasesJson, | ||
@Qualifier("serverTrustStore") KeyStore keyStore) | ||
throws JsonProcessingException { | ||
super(); | ||
ObjectMapper mapper = new ObjectMapper(); | ||
String[] samhsaAllowedCertAliases = | ||
mapper.readValue(samhsaAllowedCertificateAliasesJson, String[].class); | ||
this.samhsaAllowedSerialNumbers = | ||
Arrays.stream(samhsaAllowedCertAliases) | ||
.map(allowedCert -> getCertSerialNumber(keyStore, allowedCert)) | ||
.filter(Objects::nonNull) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
/** | ||
* Gets the serial number from the certificate alias. | ||
* | ||
* @param keyStore server key store | ||
* @param allowedCertAlias certificate alias | ||
* @return serial number | ||
*/ | ||
private static BigInteger getCertSerialNumber(KeyStore keyStore, String allowedCertAlias) { | ||
try { | ||
X509Certificate cert = ((X509Certificate) keyStore.getCertificate(allowedCertAlias)); | ||
if (cert == null) { | ||
LOGGER.error( | ||
"Certificate {} was configured to allow SAMHSA, but was not found", allowedCertAlias); | ||
return null; | ||
} | ||
return cert.getSerialNumber(); | ||
} catch (KeyStoreException e) { | ||
LOGGER.error("Error loading keystore", e); | ||
return null; | ||
} | ||
} | ||
|
||
/** {@inheritDoc} */ | ||
@Override | ||
protected void doFilterInternal( | ||
@NotNull HttpServletRequest request, | ||
@NotNull HttpServletResponse response, | ||
@NotNull FilterChain chain) | ||
throws ServletException, IOException { | ||
BigInteger serialNumber = ClientCertificateUtils.getClientSslSerialNumber(request); | ||
// Set the attribute on the request so the transformers can check for this property | ||
request.setAttribute( | ||
SHOULD_FILTER_SAMHSA, | ||
serialNumber == null || !samhsaAllowedSerialNumbers.contains(serialNumber)); | ||
chain.doFilter(request, response); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
...r/bfd-server-war/src/main/java/gov/cms/bfd/server/war/commons/ClientCertificateUtils.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
package gov.cms.bfd.server.war.commons; | ||
|
||
import jakarta.servlet.http.HttpServletRequest; | ||
import java.math.BigInteger; | ||
import java.security.cert.X509Certificate; | ||
import javax.security.auth.x500.X500Principal; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/*** | ||
aschey-forpeople marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Utilities for parsing metadata from client certificates. | ||
*/ | ||
public class ClientCertificateUtils { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of this was moved here from |
||
/** The logger for this filter. */ | ||
private static final Logger LOGGER = LoggerFactory.getLogger(ClientCertificateUtils.class); | ||
|
||
/** | ||
* Gets the {@link X500Principal#getName()} for the client certificate if available. | ||
* | ||
* @param request the {@link HttpServletRequest} to get the client principal DN (if any) for | ||
* @return the {@link X500Principal#getName()} for the client certificate, or <code>null</code> if | ||
* that's not available | ||
*/ | ||
public static String getClientSslPrincipalDistinguishedName(HttpServletRequest request) { | ||
/* | ||
* Note: Now that Wildfly/JBoss is properly configured with a security realm, | ||
* this method is equivalent to calling `request.getRemoteUser()`. | ||
*/ | ||
X509Certificate clientCert = getClientCertificate(request); | ||
if (clientCert == null || clientCert.getSubjectX500Principal() == null) { | ||
LOGGER.debug("No client SSL principal available: {}", clientCert); | ||
return null; | ||
} | ||
|
||
return clientCert.getSubjectX500Principal().getName(); | ||
} | ||
|
||
/** | ||
* Gets the serial number for the client certificate if available. | ||
* | ||
* @param request the {@link HttpServletRequest} containing the certificate | ||
* @return the serial number | ||
*/ | ||
public static BigInteger getClientSslSerialNumber(HttpServletRequest request) { | ||
X509Certificate clientCert = getClientCertificate(request); | ||
if (clientCert == null) { | ||
LOGGER.debug("No client cert available"); | ||
return null; | ||
} | ||
|
||
return clientCert.getSerialNumber(); | ||
} | ||
|
||
/** | ||
* Gets the {@link X509Certificate} for the {@link HttpServletRequest}'s client SSL certificate if | ||
* available. | ||
* | ||
* @param request the {@link HttpServletRequest} to get the client SSL certificate for | ||
* @return the {@link X509Certificate} for the {@link HttpServletRequest}'s client SSL | ||
* certificate, or <code>null</code> if that's not available | ||
*/ | ||
private static X509Certificate getClientCertificate(HttpServletRequest request) { | ||
X509Certificate[] certs = | ||
(X509Certificate[]) request.getAttribute("jakarta.servlet.request.X509Certificate"); | ||
if (certs == null || certs.length == 0) { | ||
LOGGER.debug("No client certificate found for request."); | ||
return null; | ||
} | ||
return certs[certs.length - 1]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shellcheck was blowing up when trying to parse the certificate formats