Fix CloudStack URL signing for fields with [ chars

Commit 69a8304 caused the CloudStack QuerySigner to generate invalid
signatures where key names contained square brackets, such as in the
"iptonetworklist[N]" field to deployVirtualMachine. The commit changed
the whole query string being URL-encoded, whereas previously the field
values were encoded but the field names were not. The CloudStack API
guide says that values that should be encoded for signing but not field
names, and indeed the commit does cause signatures to be rejected.

This commit reverses the change to QuerySigner.createStringToSign() and
adds a unit test for this case.
This commit is contained in:
Richard Downer 2013-01-23 10:38:58 +00:00
parent d1299c9311
commit 343da68502
2 changed files with 21 additions and 5 deletions

View File

@ -25,18 +25,19 @@ import static com.google.common.io.ByteStreams.readBytes;
import static org.jclouds.Constants.LOGGER_SIGNATURE;
import static org.jclouds.crypto.Macs.asByteProcessor;
import static org.jclouds.http.Uris.uriBuilder;
import static org.jclouds.http.utils.Queries.encodeQueryLine;
import static org.jclouds.http.utils.Queries.queryParser;
import static org.jclouds.util.Strings2.toInputStream;
import java.io.IOException;
import java.security.InvalidKeyException;
import java.util.Map;
import javax.annotation.Resource;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import com.google.common.base.Joiner;
import org.jclouds.crypto.Crypto;
import org.jclouds.domain.Credentials;
import org.jclouds.http.HttpException;
@ -46,12 +47,13 @@ import org.jclouds.http.internal.SignatureWire;
import org.jclouds.location.Provider;
import org.jclouds.logging.Logger;
import org.jclouds.rest.RequestSigner;
import org.jclouds.util.Strings2;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Multimap;
import com.google.common.collect.TreeMultimap;
import com.google.common.io.ByteProcessor;
/**
@ -116,10 +118,12 @@ public class QuerySigner implements AuthenticationFilter, RequestSigner {
@VisibleForTesting
public String createStringToSign(HttpRequest request, Multimap<String, String> decodedParams) {
utils.logRequest(signatureLog, request, ">>");
// like aws, percent encode the canonicalized string without skipping '/' and '?'
String queryLine = encodeQueryLine(TreeMultimap.create(decodedParams), ImmutableList.<Character> of());
// encode each parameter value first,
ImmutableSortedSet.Builder<String> builder = ImmutableSortedSet.naturalOrder();
for (Map.Entry<String, String> entry : decodedParams.entries())
builder.add(entry.getKey() + "=" + Strings2.urlEncode(entry.getValue()));
// then, lower case the entire query string
String stringToSign = queryLine.toLowerCase();
String stringToSign = Joiner.on('&').join(builder.build()).toLowerCase();
if (signatureWire.enabled())
signatureWire.output(stringToSign);

View File

@ -61,6 +61,18 @@ public class QuerySignerTest {
"apikey=apikey&command=listzones");
}
@Test
void testCreateStringToSignWithBrackets() {
// This test asserts that key *names* are not URL-encoded - only values
// should be encoded, according to "CloudStack API Developers Guide".
QuerySigner filter = INJECTOR.getInstance(QuerySigner.class);
assertEquals(
filter.createStringToSign(HttpRequest.builder().method("GET")
.endpoint("http://localhost:8080/client/api?command=deployVirtualMachine&iptonetworklist[0].ip=127.0.0.1&iptonetworklist[0].networkid=1").build()),
"apikey=apikey&command=deployvirtualmachine&iptonetworklist[0].ip=127.0.0.1&iptonetworklist[0].networkid=1");
}
@Test
void testFilter() {
QuerySigner filter = INJECTOR.getInstance(QuerySigner.class);