zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 07:57:13 +02:00
Code Issues Projects Releases Packages Wiki Activity

Add fallback headers to GFE specific headers

Browse source 
Currently we exact the client certificate hash from header X-GFE-SSL-Certificate. This works because the proxy running on [] sends the request directly to the AFE via HttpOverRpc, bypassing the frontline GFE, which would strip away this header.

[]

After the proxy moves to GCP we can no longer use that header. Instead we should use X-SSL-Certificate, which does not get stripped by the GFE. In fact the open source build should never have contained X-GFE-SSL-Certificate because obviously external nomulus users have to go through the GFE to reach the registry app and that header would never have survived.

Without changing how the [] proxy works, this CL makes the registry first try to extract the hash from X-GFE-SSL-Certificate, and fallback to X-SSL-Certificate if necessary. This allows the current setup to continue to work, while the new proxy is being tested.

This should not open us up to attacks because even if an attacker uses a proxy that uses X-SSL-Certificate, it still needs to pass OAuth in order to talk to /_dr/epp.

Similarly, we use X-Requested-Servername-SNI as fallback to X-GFE-Requested-Servername-SNI. This can be eliminated altogether when the [] proxy is retired, because the only reason we check if the client request is SNI enabled (by checking the existence of that header) is because the GFE only requests client certificate when SNI is enabled. The GCP proxy does not have that limitation, and also will be only serving one certificate with all SAN listed in it.

Some formatting change is also introduced by the formatter. They seem to be better conforming to the style guide, so I left them there.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165378083
This commit is contained in:
jianglai 2017-08-15 16:43:50 -07:00 • committed by Ben McIlwain
parent f9a42457a8
commit 9e7c996081
1 changed files with 23 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

45
java/google/registry/flows/TlsCredentials.java
View file

@ -39,20 +39,18 @@ import javax.servlet.http.HttpServletRequest;
* Container and validation for TLS certificate and ip-whitelisting. * Container and validation for TLS certificate and ip-whitelisting.
* *
* <p>Credentials are based on the following headers: * <p>Credentials are based on the following headers:
*
* <dl> * <dl>
* <dt>X-GFE-SSL-Certificate * <dt>X-SSL-Certificate
* <dd> * <dd>This field should contain a base64 encoded digest of the client's TLS certificate. It is
* This field should contain a base64 encoded digest of the client's TLS certificate. It is * validated during an EPP login command against a known good value that is transmitted out of
* validated during an EPP login command against a known good value that is transmitted out of * band.
* band.
* <dt>X-Forwarded-For * <dt>X-Forwarded-For
* <dd> * <dd>This field should contain the host and port of the connecting client. It is validated
* This field should contain the host and port of the connecting client. It is validated during * during an EPP login command against an IP whitelist that is transmitted out of band.
* an EPP login command against an IP whitelist that is transmitted out of band.
* <dt>X-GFE-Requested-Servername-SNI * <dt>X-GFE-Requested-Servername-SNI
* <dd> * <dd>This field should contain the servername that the client requested during the TLS
* This field should contain the servername that the client requested during the TLS handshake. * handshake. It is unused, but expected to be present in the GFE-proxied configuration.
* It is unused, but expected to be present in the GFE-proxied configuration.
* </dl> * </dl>
*/ */
public class TlsCredentials implements TransportCredentials { public class TlsCredentials implements TransportCredentials {
@ -66,9 +64,9 @@ public class TlsCredentials implements TransportCredentials {
@Inject @Inject
@VisibleForTesting @VisibleForTesting
public TlsCredentials( public TlsCredentials(
@Header("X-GFE-SSL-Certificate") String clientCertificateHash, @Header("X-SSL-Certificate") String clientCertificateHash,
@Header("X-Forwarded-For") Optional<String> clientAddress, @Header("X-Forwarded-For") Optional<String> clientAddress,
@Header("X-GFE-Requested-Servername-SNI") String sni) { @Header("X-Requested-Servername-SNI") String sni) {
this.clientCertificateHash = clientCertificateHash; this.clientCertificateHash = clientCertificateHash;
this.clientInetAddr = clientAddress.isPresent() ? parseInetAddress(clientAddress.get()) : null; this.clientInetAddr = clientAddress.isPresent() ? parseInetAddress(clientAddress.get()) : null;
this.sni = sni; this.sni = sni;
@ -102,7 +100,8 @@ public class TlsCredentials implements TransportCredentials {
private void validateIp(Registrar registrar) throws AuthenticationErrorException { private void validateIp(Registrar registrar) throws AuthenticationErrorException {
ImmutableList<CidrAddressBlock> ipWhitelist = registrar.getIpAddressWhitelist(); ImmutableList<CidrAddressBlock> ipWhitelist = registrar.getIpAddressWhitelist();
if (ipWhitelist.isEmpty()) { if (ipWhitelist.isEmpty()) {
logger.infofmt("Skipping IP whitelist check because %s doesn't have an IP whitelist", logger.infofmt(
"Skipping IP whitelist check because %s doesn't have an IP whitelist",
registrar.getClientId()); registrar.getClientId());
return; return;
} }
@ -112,8 +111,9 @@ public class TlsCredentials implements TransportCredentials {
return; return;
} }
} }
logger.infofmt("%s not in %s's CIDR whitelist: %s", logger.infofmt(
clientInetAddr, registrar.getClientId(), ipWhitelist); "Authentication error: IP address %s is not whitelisted for registrar %s.",
clientInetAddr, registrar.getClientId());
throw new BadRegistrarIpAddressException(); throw new BadRegistrarIpAddressException();
} }
@ -138,12 +138,13 @@ public class TlsCredentials implements TransportCredentials {
if (!hasSni()) { if (!hasSni()) {
throw new NoSniException(); throw new NoSniException();
} }
logger.infofmt("Request did not include %s", "X-GFE-SSL-Certificate"); logger.info("Request did not include X-SSL-Certificate");
throw new MissingRegistrarCertificateException(); throw new MissingRegistrarCertificateException();
} }
if (!clientCertificateHash.equals(registrar.getClientCertificateHash()) if (!clientCertificateHash.equals(registrar.getClientCertificateHash())
&& !clientCertificateHash.equals(registrar.getFailoverClientCertificateHash())) { && !clientCertificateHash.equals(registrar.getFailoverClientCertificateHash())) {
logger.warningfmt("bad certificate hash (%s) for %s, wanted either %s or %s", logger.warningfmt(
"bad certificate hash (%s) for %s, wanted either %s or %s",
clientCertificateHash, clientCertificateHash,
registrar.getClientId(), registrar.getClientId(),
registrar.getClientCertificateHash(), registrar.getClientCertificateHash(),
@ -200,9 +201,9 @@ public class TlsCredentials implements TransportCredentials {
@Module @Module
public static final class EppTlsModule { public static final class EppTlsModule {
@Provides @Provides
@Header("X-GFE-SSL-Certificate") @Header("X-SSL-Certificate")
static String provideClientCertificateHash(HttpServletRequest req) { static String provideClientCertificateHash(HttpServletRequest req) {
return extractRequiredHeader(req, "X-GFE-SSL-Certificate"); return extractRequiredHeader(req, "X-SSL-Certificate");
} }
@Provides @Provides
@ -212,9 +213,9 @@ public class TlsCredentials implements TransportCredentials {
} }
@Provides @Provides
@Header("X-GFE-Requested-Servername-SNI") @Header("X-Requested-Servername-SNI")
static String provideRequestedServername(HttpServletRequest req) { static String provideRequestedServername(HttpServletRequest req) {
return extractRequiredHeader(req, "X-GFE-Requested-Servername-SNI"); return extractRequiredHeader(req, "X-Requested-Servername-SNI");
} }
} }
} }