Skip to content

Commit 8c0890c

Browse files
committed
Strip trailing dot when validating certificate
1 parent 2ac39b0 commit 8c0890c

File tree

4 files changed

+40
-7
lines changed

4 files changed

+40
-7
lines changed

hack/bin/selfsigned-kubernetes.sh

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,17 @@ else
4343
fi
4444

4545
# For federation end2end tests, only the
46-
# 'federation-test-helper.$NAMESPACE.svc.cluster.local' is necessary for
46+
# 'federation-test-helper.$FEDERATION_DOMAIN_BASE' is necessary for
4747
# ingress->federator traffic. For other potential traffic in the integration
4848
# tests of the future, we use a wildcard certificate here.
49-
HOSTNAME_DOMAIN="*.$FEDERATION_DOMAIN_BASE"
50-
5149
echo '{
5250
"key": {
5351
"algo": "rsa",
5452
"size": 2048
5553
}
5654
}' >"$CSR"
5755
# generate cert and key based on CA given comma-separated hostnames as SANs
58-
cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostname="$HOSTNAME_DOMAIN" "$CSR" | cfssljson -bare "$OUTPUTNAME_LEAF_CERT"
56+
cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostname="*.$FEDERATION_DOMAIN_BASE" "$CSR" | cfssljson -bare "$OUTPUTNAME_LEAF_CERT"
5957

6058
# the following yaml override file is needed as an override to
6159
# nginx-ingress-services helm chart

services/federator/federator.cabal

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ cabal-version: 1.12
44
--
55
-- see: https://github.com/sol/hpack
66
--
7-
-- hash: c0fa14fcc8de193a5a256a3c145a8b3a60196f151787ead79ba62bc48e948fee
7+
-- hash: f20104e2f4934b00d12efd33f7c7bfb2c138ddded9a8bede22e437d5ef8803d1
88

99
name: federator
1010
version: 1.0.0
@@ -78,8 +78,10 @@ library
7878
, wai-utilities
7979
, wire-api
8080
, wire-api-federation
81+
, x509
8182
, x509-store
8283
, x509-system
84+
, x509-validation
8385
default-language: Haskell2010
8486

8587
executable federator
@@ -134,8 +136,10 @@ executable federator
134136
, wai-utilities
135137
, wire-api
136138
, wire-api-federation
139+
, x509
137140
, x509-store
138141
, x509-system
142+
, x509-validation
139143
default-language: Haskell2010
140144

141145
executable federator-integration
@@ -199,8 +203,10 @@ executable federator-integration
199203
, wai-utilities
200204
, wire-api
201205
, wire-api-federation
206+
, x509
202207
, x509-store
203208
, x509-system
209+
, x509-validation
204210
, yaml
205211
default-language: Haskell2010
206212

@@ -264,7 +270,9 @@ test-suite federator-tests
264270
, wai-utilities
265271
, wire-api
266272
, wire-api-federation
273+
, x509
267274
, x509-store
268275
, x509-system
276+
, x509-validation
269277
, yaml
270278
default-language: Haskell2010

services/federator/package.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ dependencies:
5353
- wai-utilities
5454
- network-uri
5555
- uri-bytestring
56+
- x509
57+
- x509-validation
5658

5759
library:
5860
source-dirs: src

services/federator/src/Federator/Remote.hs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ module Federator.Remote where
2222
import Data.Default (def)
2323
import Data.Domain (Domain, domainText)
2424
import Data.String.Conversions (cs)
25+
import qualified Data.X509 as X509
2526
import Data.X509.CertificateStore
27+
import qualified Data.X509.Validation as X509
2628
import Federator.Discovery (DiscoverFederator, LookupError, discoverFederator)
2729
import Federator.Options
2830
import Imports
@@ -77,7 +79,9 @@ callInward :: MonadIO m => GrpcClient -> Request -> m (GRpcReply InwardResponse)
7779
callInward client request =
7880
liftIO $ gRpcCall @'MsgProtoBuf @Inward @"Inward" @"call" client request
7981

80-
-- FUTUREWORK(federation): Make this use TLS with real certificate validation
82+
-- FUTUREWORK(federation): Consider using HsOpenSSL instead of tls for better
83+
-- security and to avoid having to depend on cryptonite and override validation
84+
-- hooks. This might involve forking http2-client: https://github.com/lucasdicioccio/http2-client/issues/76
8185
-- FUTUREWORK(federation): Allow a configurable trust store to be used in TLS certificate validation
8286
-- See also https://github.com/lucasdicioccio/http2-client/issues/76
8387
-- FUTUREWORK(federation): Cache this client and use it for many requests
@@ -116,10 +120,31 @@ mkGrpcClient target@(SrvTarget host port) = Polysemy.runError $ do
116120

117121
let caStore = customCAStore <> systemCAStore
118122

123+
-- strip trailing dot to workaround issue in tls domain verification
124+
let stripDot hostname
125+
| isSuffixOf "." hostname = take (length hostname - 1) hostname
126+
| otherwise = hostname
127+
-- try validating the hostname without a trailing dot, and if that fails, try
128+
-- again with the original hostname
129+
let validateName hostname cert
130+
| null validation = []
131+
| isSuffixOf "." hostname = TLS.hookValidateName X509.defaultHooks hostname cert
132+
| otherwise = validation
133+
where
134+
validation = TLS.hookValidateName X509.defaultHooks (stripDot hostname) cert
135+
119136
let betterTLSConfig =
120137
(defaultParamsClient (cs host) (cs $ show port))
121138
{ TLS.clientSupported = def {TLS.supportedCiphers = blessed_ciphers},
122-
TLS.clientHooks = def, -- FUTUREWORK: use onCertificateRequest to provide client certificates
139+
TLS.clientHooks =
140+
def
141+
{ TLS.onServerCertificate =
142+
X509.validate
143+
X509.HashSHA256
144+
(X509.defaultHooks {TLS.hookValidateName = validateName})
145+
X509.defaultChecks
146+
},
147+
-- FUTUREWORK: use onCertificateRequest to provide client certificates
123148
TLS.clientShared = def {TLS.sharedCAStore = caStore}
124149
}
125150
let cfg' = cfg {_grpcClientConfigTLS = Just betterTLSConfig}

0 commit comments

Comments
 (0)