Skip to content

Commit

Permalink
[fix] Fix hostname verification (#126)
Browse files Browse the repository at this point in the history
### Motivation

If `ValidateHostName` is set to true, handshake always fails.
```
INFO  [] ClientConnection:375 | [ -> ] Connected to broker
ERROR [] ClientConnection:463 | [ -> ] Handshake failed: certificate verify failed
INFO  [] ClientConnection:1560 | [ -> ] Connection closed
```

### Modifications

- Verify `serviceUrl.host()`, not `physicalAddress`.
  - `physicalAddress` is serviceUrl, which contains protocol (e.g. pulsar+ssl) and port number.
- Use `ssl::stream::set_verify_callback` instead of `ssl::context::set_verify_callback`.
  - Verification should work with `ssl::context::set_verify_callback`, but somehow it doesn't work.

Co-authored-by: hoguni <[email protected]>
  • Loading branch information
2 people authored and merlimat committed Jan 19, 2023
1 parent e6a8a64 commit 9350d1b
Show file tree
Hide file tree
Showing 10 changed files with 624 additions and 14 deletions.
36 changes: 36 additions & 0 deletions build-support/setup-test-service-container.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#

set -e -x

if [ $# -ne 2 ]; then
echo "Usage: $0 \$CONTAINER_ID \$START_TEST_SERVICE_INSIDE_CONTAINER"
exit 1
fi

CONTAINER_ID=$1
START_TEST_SERVICE_INSIDE_CONTAINER=$2

echo $CONTAINER_ID >> .tests-container-id.txt

docker cp test-conf $CONTAINER_ID:/pulsar/test-conf
docker cp build-support/$START_TEST_SERVICE_INSIDE_CONTAINER $CONTAINER_ID:$START_TEST_SERVICE_INSIDE_CONTAINER

docker exec -i $CONTAINER_ID /$START_TEST_SERVICE_INSIDE_CONTAINER
66 changes: 66 additions & 0 deletions build-support/start-mim-test-service-inside-container.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#!/usr/bin/env bash
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#

set -e -x

export PULSAR_EXTRA_OPTS=-Dpulsar.auth.basic.conf=test-conf/.htpasswd

# Generate secret key and token
mkdir -p data/tokens
bin/pulsar tokens create-secret-key --output data/tokens/secret.key

bin/pulsar tokens create \
--subject token-principal \
--secret-key file:///pulsar/data/tokens/secret.key \
> /pulsar/data/tokens/token.txt

export PULSAR_STANDALONE_CONF=test-conf/standalone-ssl-mim.conf
export PULSAR_PID_DIR=/tmp
bin/pulsar-daemon start standalone \
--no-functions-worker --no-stream-storage \
--bookkeeper-dir data/bookkeeper

echo "-- Wait for Pulsar service to be ready"
until curl http://localhost:8081/metrics > /dev/null 2>&1 ; do sleep 1; done

echo "-- Pulsar service is ready -- Configure permissions"

export PULSAR_CLIENT_CONF=test-conf/client-ssl-mim.conf

# Create "standalone" cluster if it does not exist
bin/pulsar-admin clusters list | grep -q '^standalone$' ||
bin/pulsar-admin clusters create \
standalone \
--url http://localhost:8081/ \
--url-secure https://localhost:8444/ \
--broker-url pulsar://localhost:6652/ \
--broker-url-secure pulsar+ssl://localhost:6653/

# Create "private" tenant
bin/pulsar-admin tenants create private -r "" -c "standalone"

# Create "private/auth" with required authentication
bin/pulsar-admin namespaces create private/auth --clusters standalone

bin/pulsar-admin namespaces grant-permission private/auth \
--actions produce,consume \
--role "token-principal"

echo "-- Ready to start tests"
10 changes: 5 additions & 5 deletions lib/ClientConnection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,6 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std:
} else {
ctx.set_verify_mode(boost::asio::ssl::context::verify_peer);

if (clientConfiguration.isValidateHostName()) {
LOG_DEBUG("Validating hostname for " << serviceUrl.host() << ":" << serviceUrl.port());
ctx.set_verify_callback(boost::asio::ssl::rfc2818_verification(physicalAddress));
}

std::string trustCertFilePath = clientConfiguration.getTlsTrustCertsFilePath();
if (!trustCertFilePath.empty()) {
if (file_exists(trustCertFilePath)) {
Expand Down Expand Up @@ -254,6 +249,11 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std:

tlsSocket_ = ExecutorService::createTlsSocket(socket_, ctx);

if (!clientConfiguration.isTlsAllowInsecureConnection() && clientConfiguration.isValidateHostName()) {
LOG_DEBUG("Validating hostname for " << serviceUrl.host() << ":" << serviceUrl.port());
tlsSocket_->set_verify_callback(boost::asio::ssl::rfc2818_verification(serviceUrl.host()));
}

LOG_DEBUG("TLS SNI Host: " << serviceUrl.host());
if (!SSL_set_tlsext_host_name(tlsSocket_->native_handle(), serviceUrl.host().c_str())) {
boost::system::error_code ec{static_cast<int>(::ERR_get_error()),
Expand Down
10 changes: 4 additions & 6 deletions pulsar-test-service-start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@ cd $SRC_DIR
./pulsar-test-service-stop.sh

CONTAINER_ID=$(docker run -i -p 8080:8080 -p 6650:6650 -p 8443:8443 -p 6651:6651 --rm --detach apachepulsar/pulsar:latest sleep 3600)
echo $CONTAINER_ID > .tests-container-id.txt

docker cp test-conf $CONTAINER_ID:/pulsar/test-conf
docker cp build-support/start-test-service-inside-container.sh $CONTAINER_ID:start-test-service-inside-container.sh

docker exec -i $CONTAINER_ID /start-test-service-inside-container.sh
build-support/setup-test-service-container.sh $CONTAINER_ID start-test-service-inside-container.sh

docker cp $CONTAINER_ID:/pulsar/data/tokens/token.txt .test-token.txt

CONTAINER_ID=$(docker run -i -p 8081:8081 -p 6652:6652 -p 8444:8444 -p 6653:6653 --rm --detach apachepulsar/pulsar:latest sleep 3600)
build-support/setup-test-service-container.sh $CONTAINER_ID start-mim-test-service-inside-container.sh

echo "-- Ready to start tests"
26 changes: 26 additions & 0 deletions test-conf/client-ssl-mim.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#

# Pulsar Client configuration
webServiceUrl=https://localhost:8444/
brokerServiceUrl=pulsar+ssl://localhost:6653/
tlsAllowInsecureConnection=false
tlsTrustCertsFilePath=test-conf/hn-verification/cacert.pem
authPlugin=org.apache.pulsar.client.impl.auth.AuthenticationTls
authParams=tlsCertFile:test-conf/client-cert.pem,tlsKeyFile:test-conf/client-key.pem
27 changes: 27 additions & 0 deletions test-conf/hn-verification/broker-cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
-----BEGIN CERTIFICATE-----
MIIEkDCCAnigAwIBAgICEAAwDQYJKoZIhvcNAQELBQAwETEPMA0GA1UEAwwGZm9v
YmFyMCAXDTE4MDYyMjA4NTUzMloYDzIyOTIwNDA2MDg1NTMyWjAjMSEwHwYDVQQD
DBhicm9rZXIucHVsc2FyLmFwYWNoZS5vcmcwggEiMA0GCSqGSIb3DQEBAQUAA4IB
DwAwggEKAoIBAQDQouKhZah4hMCqmg4aS5RhQG/Y1gA+yP9DGF9mlw35tfhfWs63
EvNjEK4L/ZWSEV45L/wc6YV14RmM6bJ0V/0vXo4xmISbqptND/2kRIspkLZQ5F0O
OQXVicqZLOc6igZQhRg8ANDYdTJUTF65DqauX4OJt3YMhF2FSt7jQtlj06IQBa01
+ARO9OotMJtBY+vIU5bV6JydfgkhQH9rIDI7AMeY5j02gGkJJrelfm+WoOsUez+X
aqTN3/tF8+MBcFB3G04s1qc2CJPJM3YGxvxEtHqTGI14t9J8p5O7X9JHpcY8X00s
bxa4FGbKgfDobbkJ+GgblWCkAcLN95sKTqtHAgMBAAGjgd0wgdowCQYDVR0TBAIw
ADARBglghkgBhvhCAQEEBAMCBkAwMwYJYIZIAYb4QgENBCYWJE9wZW5TU0wgR2Vu
ZXJhdGVkIFNlcnZlciBDZXJ0aWZpY2F0ZTAdBgNVHQ4EFgQUaxFvJrkEGqk8azTA
DyVyTyTbJAIwQQYDVR0jBDowOIAUVwvpyyPov0c+UHo/RX6hGEOdFSehFaQTMBEx
DzANBgNVBAMMBmZvb2JhcoIJANfih0+geeIMMA4GA1UdDwEB/wQEAwIFoDATBgNV
HSUEDDAKBggrBgEFBQcDATANBgkqhkiG9w0BAQsFAAOCAgEA35QDGclHzQtHs3yQ
ZzNOSKisg5srTiIoQgRzfHrXfkthNFCnBzhKjBxqk3EIasVtvyGuk0ThneC1ai3y
ZK3BivnMZfm1SfyvieFoqWetsxohWfcpOSVkpvO37P6v/NmmaTIGkBN3gxKCx0QN
zqApLQyNTM++X3wxetYH/afAGUrRmBGWZuJheQpB9yZ+FB6BRp8YuYIYBzANJyW9
spvXW03TpqX2AIoRBoGMLzK72vbhAbLWiCIfEYREhbZVRkP+yvD338cWrILlOEur
x/n8L/FTmbf7mXzHg4xaQ3zg/5+0OCPMDPUBE4xWDBAbZ82hgOcTqfVjwoPgo2V0
fbbx6redq44J3Vn5d9Xhi59fkpqEjHpX4xebr5iMikZsNTJMeLh0h3uf7DstuO9d
mfnF5j+yDXCKb9XzCsTSvGCN+spmUh6RfSrbkw8/LrRvBUpKVEM0GfKSnaFpOaSS
efM4UEi72FRjszzHEkdvpiLhYvihINLJmDXszhc3fCi42be/DGmUhuhTZWynOPmp
0N0V/8/sGT5gh4fGEtGzS/8xEvZwO9uDlccJiG8Pi+aO0/K9urB9nppd/xKWXv3C
cib/QrW0Qow4TADWC1fnGYCpFzzaZ2esPL2MvzOYXnW4/AbEqmb6Weatluai64ZK
3N2cGJWRyvpvvmbP2hKCa4eLgEc=
-----END CERTIFICATE-----
28 changes: 28 additions & 0 deletions test-conf/hn-verification/broker-key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDQouKhZah4hMCq
mg4aS5RhQG/Y1gA+yP9DGF9mlw35tfhfWs63EvNjEK4L/ZWSEV45L/wc6YV14RmM
6bJ0V/0vXo4xmISbqptND/2kRIspkLZQ5F0OOQXVicqZLOc6igZQhRg8ANDYdTJU
TF65DqauX4OJt3YMhF2FSt7jQtlj06IQBa01+ARO9OotMJtBY+vIU5bV6Jydfgkh
QH9rIDI7AMeY5j02gGkJJrelfm+WoOsUez+XaqTN3/tF8+MBcFB3G04s1qc2CJPJ
M3YGxvxEtHqTGI14t9J8p5O7X9JHpcY8X00sbxa4FGbKgfDobbkJ+GgblWCkAcLN
95sKTqtHAgMBAAECggEBALE1eMtfnk3nbAI74bih84D7C0Ug14p8jJv/qqBnsx4j
WrgbWDMVrJa7Rym2FQHBMMfgIwKnso0iSeJvaPz683j1lk833YKe0VQOPgD1m0IN
wV1J6mQ3OOZcKDIcerY1IBHqSmBEzR7dxIbnaxlCAX9gb0hdBK6zCwA5TMG5OQ5Y
3cGOmevK5i2PiejhpruA8h7E48P1ATaGHUZif9YD724oi6AcilQ8H/DlOjZTvlmK
r4aJ30f72NwGM8Ecet5CE2wyflAGtY0k+nChYkPRfy54u64Z/T9B53AvneFaj8jv
yFepZgRTs2cWhEl0KQGuBHQ4+IeOfMt2LebhvjWW8YkCgYEA7BXVsnqPHKRDd8wP
eNkolY4Fjdq4wu9ad+DaFiZcJuv7ugr+Kplltq6e4aU36zEdBYdPp/6KM/HGE/Xj
bo0CELNUKs/Ny9H/UJc8DDbVEmoF3XGiIbKKq1T8NTXTETFnwrGkBFD8nl7YTsOF
M4FZmSok0MhhkpEULAqxBS6YpQsCgYEA4jxM1egTVSWjTreg2UdYo2507jKa7maP
PRtoPsNJzWNbOpfj26l3/8pd6oYKWck6se6RxIUxUrk3ywhNJIIOvWEC7TaOH1c9
T4NQNcweqBW9+A1x5gyzT14gDaBfl45gs82vI+kcpVv/w2N3HZOQZX3yAUqWpfw2
yw1uQDXtgDUCgYEAiYPWbBXTkp1j5z3nrT7g0uxc89n5USLWkYlZvxktCEbg4+dP
UUT06EoipdD1F3wOKZA9p98uZT9pX2sUxOpBz7SFTEKq3xQ9IZZWFc9CoW08aVat
V++FsnLYTa5CeXtLsy6CGTmLTDx2xrpAtlWb+QmBVFPD8fmrxFOd9STFKS0CgYAt
6ztVN3OlFqyc75yQPXD6SxMkvdTAisSMDKIOCylRrNb5f5baIP2gR3zkeyxiqPtm
3htsHfSy67EtXpP50wQW4Dft2eLi7ZweJXMEWFfomfEjBeeWYAGNHHe5DFIauuVZ
2WexDEGqNpAlIm0s7aSjVPrn1DHbouOkNyenlMqN+QKBgQDVYVhk9widShSnCmUA
G30moXDgj3eRqCf5T7NEr9GXD1QBD/rQSPh5agnDV7IYLpV7/wkYLI7l9x7mDwu+
I9mRXkyAmTVEctLTdXQHt0jdJa5SfUaVEDUzQbr0fUjkmythTvqZ809+d3ELPeLI
5qJ7jxgksHWji4lYfL4r4J6Zaw==
-----END PRIVATE KEY-----
29 changes: 29 additions & 0 deletions test-conf/hn-verification/cacert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
-----BEGIN CERTIFICATE-----
MIIFCDCCAvCgAwIBAgIJANfih0+geeIMMA0GCSqGSIb3DQEBCwUAMBExDzANBgNV
BAMMBmZvb2JhcjAeFw0xODA2MjIwODQ2MjFaFw0zODA2MTcwODQ2MjFaMBExDzAN
BgNVBAMMBmZvb2JhcjCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAOVU
UpTPeXCeyfUiQS824l9s9krZd4R6TA4D97eQ9EWm2D7ppV4gPApHO8j5f+joo/b6
Iso4aFlHpJ8VV2a5Ol7rjQw43MJHaBgwDxB1XWgsNdfoI7ebtp/BWg2nM3r8wm+Z
gKenf9d1/1Ol+6yFUehkLkIXUvldiVegmmje8FnwhcDNE1eTrh66XqSJXEXqgBKu
NqsoYcVak72OyOO1/N8CESoSdyBkbSiH5vJyo0AUCjn7tULga7fxojmqBZDog9Pg
e5Fi/hbCrdinbxBrMgIxQ7wqXw2sw6iOWu4FU8Ih/CuF4xaQy2YP7MEk4Ff0LCY0
KMhFMWU7550r/fz/C2l7fKhREyCQPa/bVE+dfxgZ/gCZ+p7vQ154hCCjpd+5bECv
SN1bcVIPG6ngQu4vMXa7QRBi/Od40jSVGVJXYY6kXvrYatad7035w2GGGGkvMsQm
y53yh4tqQfH7ulHqB0J5LebTQRp6nRizWigVCLjNkxJYI+Dj51qvT1zdyWEegKr1
CthBfYzXlfjeH3xri1f0UABeC12n24Wkacd9af7zs7S3rYntEK444w/3fB0F62Lh
SESfMLAmUH0dF5plRShrFUXz23nUeS8EYgWmnGkpf/HDzB67vdfAK0tfJEtmmY78
q06OSgMr+AOOqaomh4Ez2ZQG592bS71G8MrE7r2/AgMBAAGjYzBhMB0GA1UdDgQW
BBRXC+nLI+i/Rz5Qej9FfqEYQ50VJzAfBgNVHSMEGDAWgBRXC+nLI+i/Rz5Qej9F
fqEYQ50VJzAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIBhjANBgkqhkiG
9w0BAQsFAAOCAgEAYd2PxdV+YOaWcmMG1fK7CGwSzDOGsgC7hi4gWPiNsVbz6fwQ
m5Ac7Zw76dzin8gzOPKST7B8WIoc7ZWrMnyh3G6A3u29Ec8iWahqGa91NPA3bOIl
0ldXnXfa416+JL/Q5utpiV6W2XDaB53v9GqpMk4rOTS9kCFOiuH5ZU8P69jp9mq6
7pI/+hWFr+21ibmXH6ANxRLd/5+AqojRUYowAu2997Z+xmbpwx/2Svciq3LNY/Vz
s9DudUHCBHj/DPgNxsEUt8QNohjQkRbFTY0a1aXodJ/pm0Ehk2kf9KwYYYduR7ak
6UmPIPrZg6FePNahxwMZ0RtgX7EXmpiiIH1q9BsulddWkrFQclevsWO3ONQVrDs2
gwY0HQuCRCJ+xgS2cyGiGohW5MkIsg1aI0i0j5GIUSppCIYgirAGCairARbCjhcx
pbMe8RTuBhCqO3R2wZ0wXu7P7/ArI/Ltm1dU6IeHUAUmeneVj5ie0SdA19mHTS2o
lG77N0jy6eq2zyEwJE6tuS/tyP1xrxdzXCYY7f6X9aNfsuPVQTcnrFajvDv8R6uD
YnRStVCdS6fZEP0JzsLrqp9bgLIRRsiqsVVBCgJdK1I/X59qk2EyCLXWSgk8T9XZ
iux8LlPpskt30YYt1KhlWB9zVz7k0uYAwits5foU6RfCRDPAyOa1q/QOXk0=
-----END CERTIFICATE-----
Loading

0 comments on commit 9350d1b

Please sign in to comment.