Skip to content

Revisit MODCLUSTER-337 & MODCLUSTER-338 #58

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 29, 2014
Merged

Revisit MODCLUSTER-337 & MODCLUSTER-338 #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
870eefa
Revert "MODCLUSTER-338 Advertize adds a message digest even if securi…
rhusar Jan 10, 2014
1c24545
[MODCLUSTER-337] Revisit: if security key is not used, verify using z…
rhusar Jan 23, 2014
2cf2670
[MODCLUSTER-338] Fix tests to send properly formed advertise message …
rhusar Jan 23, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions core/src/main/java/org/jboss/modcluster/advertise/impl/AdvertiseListenerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public AdvertiseListenerImpl(MCMPHandler commHandler, AdvertiseConfiguration con
this.handler = commHandler;
this.socketFactory = socketFactory;
this.config = config;
this.md = (config.getAdvertiseSecurityKey() != null) ? this.getMessageDigest() : null;
this.md = this.getMessageDigest();
}

private MessageDigest getMessageDigest() throws IOException {
Expand Down Expand Up @@ -261,14 +261,22 @@ boolean verifyDigest(String digest, String server, String date, String sequence)
// Neither side is configured to use digest -- pass verification
if (this.md == null && digest == null) return true;

// If either the digest is missing or security key is not set -- fail verification
String securityKey = this.config.getAdvertiseSecurityKey();
if (securityKey == null || digest == null) return false;
byte[] salt;

if (securityKey == null) {
// Security key is not configured, so the result hash was zero bytes
salt = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
} else {
// Use security key hash to calculate the final hash
this.md.reset();
digestString(this.md, securityKey);
salt = this.md.digest();
}

this.md.reset();
digestString(this.md, securityKey);
byte[] ssalt = this.md.digest();
this.md.update(ssalt);
this.md.update(salt);

digestString(this.md, date);
digestString(this.md, sequence);
digestString(this.md, server);
Expand Down Expand Up @@ -416,8 +424,10 @@ public void run() {
if (server != null && status > 0) {
/* We need a digest to match */
if (!AdvertiseListenerImpl.this.verifyDigest(digest, server_name, date_str, sequence)) {
log.tracef("Advertise message digest verification failed for server %s", server_name);
continue;
}
log.tracef("Advertise message digest verification passed for server %s", server_name);

server.setDate(date);
boolean rc = server.setStatus(status, status_desc);
Expand Down
112 changes: 112 additions & 0 deletions core/src/test/java/org/jboss/modcluster/TestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* JBoss, Home of Professional Open Source.
* Copyright 2014, Red Hat, Inc., and individual contributors
* as indicated by the @author tags. See the copyright.txt file in the
* distribution for a full listing of individual contributors.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/

package org.jboss.modcluster;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;
import javax.xml.bind.DatatypeConverter;

/**
* Utility class to be used in tests.
*
* @author Radoslav Husar
* @since 1.3.0
*/
public class TestUtils {

public static final String RFC_822_FMT = "EEE, d MMM yyyy HH:mm:ss Z";
public static final DateFormat df = new SimpleDateFormat(RFC_822_FMT, Locale.US);
public static final byte[] zeroMd5Sum = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

/**
* Generates datagram packet content buffer including all fields as sent by native code.
*
* @param date
* @param sequence
* @param server
* @param serverAddress
* @return byte buffer
* @throws NoSuchAlgorithmException
*/
public static byte[] generateAdvertisePacketData(Date date, int sequence, String server, String serverAddress) throws NoSuchAlgorithmException {
MessageDigest md = MessageDigest.getInstance("MD5");

String rfcDate = df.format(date);

md.update(zeroMd5Sum);
digestString(md, rfcDate);
digestString(md, String.valueOf(sequence));
digestString(md, server);

// Convert to hex
String digestHex = DatatypeConverter.printHexBinary(md.digest());

StringBuilder data = new StringBuilder("HTTP/1.1 200 OK\r\n");
data.append("Date: ");
data.append(rfcDate);
data.append("\r\n");
data.append("Sequence: ");
data.append(sequence);
data.append("\r\n");
data.append("Digest: ");
data.append(digestHex);
data.append("\r\n");
data.append("Server: ");
data.append(server);
data.append("\r\n");
data.append("X-Manager-Address: ");
data.append(serverAddress);
data.append("\r\n");

byte[] buf = data.toString().getBytes();
return buf;
}

/**
* Utility method to digest {@link String}s.
*
* @param md {@link MessageDigest}
* @param s {@link String} to update the digest with
*/
private static void digestString(MessageDigest md, String s) {
int len = s.length();
byte[] b = new byte[len];
for (int i = 0; i < len; i++) {
char c = s.charAt(i);
if (c < 127) {
b[i] = (byte) c;
} else {
b[i] = '?';
}
}
md.update(b);
}

// Utility class
private TestUtils() {
}
}
56 changes: 19 additions & 37 deletions core/src/test/java/org/jboss/modcluster/advertise/AdvertiseListenerImplTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,25 @@
*/
package org.jboss.modcluster.advertise;

import static org.mockito.Mockito.*;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.net.DatagramPacket;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.MulticastSocket;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.security.NoSuchAlgorithmException;
import java.util.Date;
import java.util.Locale;
import java.util.concurrent.Executors;

import org.jboss.modcluster.TestUtils;
import org.jboss.modcluster.advertise.impl.AdvertiseListenerImpl;
import org.jboss.modcluster.advertise.impl.MulticastSocketFactoryImpl;
import org.jboss.modcluster.config.AdvertiseConfiguration;
Expand All @@ -45,9 +50,10 @@
import org.mockito.ArgumentCaptor;

/**
* Tests of {@link AdvertiseListener}.
*
* Tests of {@link AdvertiseListener} / {@link AdvertiseListenerImpl}.
*
* @author Brian Stansberry
* @author Radoslav Husar
*/
@SuppressWarnings("boxing")
public class AdvertiseListenerImplTestCase {
Expand All @@ -56,8 +62,6 @@ public class AdvertiseListenerImplTestCase {
}
private static final String ADVERTISE_GROUP = "224.0.1.106";
private static final int ADVERTISE_PORT = 23364;
private static final String RFC_822_FMT = "EEE, d MMM yyyy HH:mm:ss Z";
private static final DateFormat df = new SimpleDateFormat(RFC_822_FMT, Locale.US);
private static final String SERVER1 = "127.0.0.1";
private static final String SERVER2 = "127.0.1.1";
private static final int SERVER_PORT = 8888;
Expand Down Expand Up @@ -93,7 +97,7 @@ public void tearDown() {
}

@Test
public void testBasicOperation() throws IOException {
public void testBasicOperation() throws IOException, NoSuchAlgorithmException {
ArgumentCaptor<InetAddress> capturedAddress = ArgumentCaptor.forClass(InetAddress.class);

when(this.socketFactory.createMulticastSocket(capturedAddress.capture(), eq(ADVERTISE_PORT))).thenReturn(this.socket);
Expand All @@ -104,20 +108,8 @@ public void testBasicOperation() throws IOException {
assertFalse(this.socket.isClosed());

ArgumentCaptor<InetSocketAddress> capturedSocketAddress = ArgumentCaptor.forClass(InetSocketAddress.class);
String date = df.format(new Date());

StringBuilder data = new StringBuilder("HTTP/1.1 200 OK\r\n");
data.append("Date: ");
data.append(date);
data.append("\r\n");
data.append("Server: ");
data.append(SERVER1);
data.append("\r\n");
data.append("X-Manager-Address: ");
data.append(SERVER1_ADDRESS);
data.append("\r\n");

byte[] buf = data.toString().getBytes();

byte[] buf = TestUtils.generateAdvertisePacketData(new Date(), 0, SERVER1, SERVER1_ADDRESS);
DatagramPacket packet = new DatagramPacket(buf, buf.length, this.groupAddress, ADVERTISE_PORT);

if (!System.getProperty("os.name").startsWith("Windows")) {
Expand All @@ -139,7 +131,7 @@ public void testBasicOperation() throws IOException {

verify(this.mcmpHandler).addProxy(capturedSocketAddress.capture());
reset(this.mcmpHandler);

InetSocketAddress socketAddress = capturedSocketAddress.getValue();
assertEquals(SERVER1, socketAddress.getAddress().getHostAddress());
assertEquals(SERVER_PORT, socketAddress.getPort());
Expand All @@ -163,18 +155,7 @@ public void testBasicOperation() throws IOException {

capturedSocketAddress = ArgumentCaptor.forClass(InetSocketAddress.class);

data = new StringBuilder("HTTP/1.1 200 OK\r\n");
data.append("Date: ");
data.append(date);
data.append("\r\n");
data.append("Server: ");
data.append(SERVER2);
data.append("\r\n");
data.append("X-Manager-Address: ");
data.append(SERVER2_ADDRESS);
data.append("\r\n");

buf = data.toString().getBytes();
buf = TestUtils.generateAdvertisePacketData(new Date(), 0, SERVER2, SERVER2_ADDRESS);
packet = new DatagramPacket(buf, buf.length, this.groupAddress, ADVERTISE_PORT);

this.socket.send(packet);
Expand Down Expand Up @@ -214,4 +195,5 @@ public void testBasicOperation() throws IOException {

assertTrue(this.socket.isClosed());
}

}
46 changes: 19 additions & 27 deletions native/advertise/mod_advertise.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,6 @@ static const char *cmd_advertise_h(cmd_parms *cmd, void *dummy,
"Digest: %s" CRLF \
"Server: %s" CRLF

#define MA_ADVERTISE_SERVER_FMT_NO_DIGEST \
"HTTP/1.0 %s" CRLF \
"Date: %s" CRLF \
"Sequence: %" APR_INT64_T_FMT CRLF \
"Server: %s" CRLF

static const char *hex = "0123456789abcdef";

apr_status_t ma_advertise_server(server_rec *server, int type)
Expand All @@ -313,28 +307,23 @@ apr_status_t ma_advertise_server(server_rec *server, int type)
ap_recent_rfc822_date(dat, apr_time_now());
asl = ap_get_status_line(ma_advertise_stat);

if (mconf->ma_advertise_skey) {
/* Create MD5 digest: salt + date + sequence + srvid */
apr_md5_init(&md);
apr_md5_update(&md, magd->ssalt, APR_MD5_DIGESTSIZE);
apr_md5_update(&md, dat, strlen(dat));
apr_md5_update(&md, buf, strlen(buf));
apr_md5_update(&md, magd->srvid + 1, strlen(magd->srvid) - 1);
apr_md5_final(msig, &md);
/* Convert MD5 digest to hex string */
for (i = 0; i < APR_MD5_DIGESTSIZE; i++) {
ssig[c++] = hex[msig[i] >> 4];
ssig[c++] = hex[msig[i] & 0x0F];
}
ssig[c] = '\0';

n = apr_snprintf(p, l, MA_ADVERTISE_SERVER_FMT,
asl, dat, ma_sequence, ssig, magd->srvid + 1);
} else {
n = apr_snprintf(p, l, MA_ADVERTISE_SERVER_FMT_NO_DIGEST,
asl, dat, ma_sequence, magd->srvid + 1);
/* Create MD5 digest
* salt + date + sequence + srvid
*/
apr_md5_init(&md);
apr_md5_update(&md, magd->ssalt, APR_MD5_DIGESTSIZE);
apr_md5_update(&md, dat, strlen(dat));
apr_md5_update(&md, buf, strlen(buf));
apr_md5_update(&md, magd->srvid + 1, strlen(magd->srvid) - 1);
apr_md5_final(msig, &md);
/* Convert MD5 digest to hex string */
for (i = 0; i < APR_MD5_DIGESTSIZE; i++) {
ssig[c++] = hex[msig[i] >> 4];
ssig[c++] = hex[msig[i] & 0x0F];
}

ssig[c] = '\0';
n = apr_snprintf(p, l, MA_ADVERTISE_SERVER_FMT,
asl, dat, ma_sequence, ssig, magd->srvid + 1);
if (type == MA_ADVERTISE_SERVER) {
char *ma_advertise_srvs = mconf->ma_advertise_srvs;
if (strchr(ma_advertise_srvs, ':') != NULL) {
Expand Down Expand Up @@ -611,6 +600,9 @@ static int post_config_hook(apr_pool_t *pconf, apr_pool_t *plog,
apr_md5_init(&mc);
apr_md5_update(&mc, mconf->ma_advertise_skey, strlen(mconf->ma_advertise_skey));
apr_md5_final(magd->ssalt, &mc);
} else {
/* If security key is not configured, the digest is calculated from zero bytes */
memset(magd->ssalt, '\0', APR_MD5_DIGESTSIZE);
}
apr_uuid_get(&magd->suuid);
magd->srvid[0] = '/';
Expand Down