Skip to content

Commit c3bfc11

Browse files
authored
Merge pull request #6 from adamcin/feature/issue-5-request-target
Added "(request-target): " header prefix, with tests and deprecations
2 parents c1351c8 + b12cb3c commit c3bfc11

File tree

8 files changed

+110
-15
lines changed

8 files changed

+110
-15
lines changed

api/src/main/java/net/adamcin/httpsig/api/DefaultVerifier.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,32 @@ public final class DefaultVerifier implements Verifier {
4545
private final KeyId keyId;
4646
private final long skew;
4747

48+
// this parameter is not long for the world. Do not expose to API.
49+
private final boolean strictRequestTarget;
50+
4851
public DefaultVerifier(Keychain keychain) {
49-
this(keychain, null, DEFAULT_SKEW);
52+
this(keychain, null, DEFAULT_SKEW, false);
53+
}
54+
55+
// Package-private constructor to prevent external API usage of strictRequestTarget parameter.
56+
DefaultVerifier(Keychain keychain, boolean strictRequestTarget) {
57+
this(keychain, null, DEFAULT_SKEW, strictRequestTarget);
5058
}
5159

5260
public DefaultVerifier(Keychain keychain, KeyId keyId) {
53-
this(keychain, keyId, DEFAULT_SKEW);
61+
this(keychain, keyId, DEFAULT_SKEW, false);
5462
}
5563

5664
public DefaultVerifier(Keychain keychain, KeyId keyId, long skew) {
65+
this(keychain, keyId, skew, false);
66+
}
67+
68+
// private constructor to prevent external API usage of strictRequestTarget parameter.
69+
private DefaultVerifier(Keychain keychain, KeyId keyId, long skew, boolean strictRequestTarget) {
5770
this.keychain = keychain != null ? new KeychainGuard(keychain) : new KeychainGuard(new DefaultKeychain());
5871
this.keyId = new CanVerifyId(keyId != null ? keyId : Constants.DEFAULT_KEY_IDENTIFIER);
5972
this.skew = skew;
73+
this.strictRequestTarget = strictRequestTarget;
6074
}
6175

6276
public Keychain getKeychain() {
@@ -139,9 +153,18 @@ public VerifyResult verifyWithResult(Challenge challenge, RequestContent request
139153
}
140154

141155
if (key.verify(authorization.getAlgorithm(),
142-
requestContent.getContent(authorization.getHeaders(), Constants.CHARSET),
156+
requestContent.getBytesToSign(authorization.getHeaders(), Constants.CHARSET),
143157
authorization.getSignatureBytes())) {
144158
return VerifyResult.SUCCESS;
159+
} else if (!this.strictRequestTarget
160+
&& authorization.getHeaders().contains(Constants.HEADER_REQUEST_TARGET)) {
161+
if (key.verify(authorization.getAlgorithm(),
162+
requestContent.getContent(authorization.getHeaders(),
163+
Constants.CHARSET), authorization.getSignatureBytes())) {
164+
return VerifyResult.SUCCESS;
165+
} else {
166+
return VerifyResult.FAILED_KEY_VERIFY;
167+
}
145168
} else {
146169
return VerifyResult.FAILED_KEY_VERIFY;
147170
}

api/src/main/java/net/adamcin/httpsig/api/Key.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public interface Key {
5252
/**
5353
* Verifies the {@code signatureBytes} against the {@code challengeHash} using an underlying public key
5454
* @param algorithm the selected Signature {@link Algorithm}
55-
* @param contentBytes the result of {@link RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
55+
* @param contentBytes the result of {@link RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
5656
* @param signatureBytes the result of {@link net.adamcin.httpsig.api.Authorization#getSignatureBytes()}
5757
* @return true if signature is valid
5858
*/
@@ -66,7 +66,7 @@ public interface Key {
6666
/**
6767
* Signs the {@code challengeHash} using the specified signature {@link Algorithm}
6868
* @param algorithm the selected Signature {@link Algorithm}
69-
* @param contentBytes the result of {@link RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
69+
* @param contentBytes the result of {@link RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
7070
* @return byte array containing the challengeHash signature or null if a signature could not be generated.
7171
*/
7272
byte[] sign(Algorithm algorithm, byte[] contentBytes);

api/src/main/java/net/adamcin/httpsig/api/RequestContent.java

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public final class RequestContent implements Serializable {
5353

5454
private static final List<String> SUPPORTED_DATE_FORMATS = Arrays.asList(DATE_FORMAT_RFC1123, DATE_FORMAT);
5555

56-
private static final long serialVersionUID = -2968642080214687631L;
56+
private static final long serialVersionUID = -2968642080214687632L;
5757

5858
@Deprecated
5959
private final String requestLine;
@@ -181,7 +181,10 @@ public RequestContent build() {
181181
* @param headers the list of headers to be included in the signed content
182182
* @param charset charset for decoding the content
183183
* @return the result of {@link #getContentString(java.util.List)} encoded using the provided {@link Charset}
184+
* @deprecated use {@link #getBytesToSign(List, Charset)} to include the {@link Constants#HEADER_REQUEST_TARGET}
185+
* prefix in signature content
184186
*/
187+
@Deprecated
185188
public byte[] getContent(List<String> headers, Charset charset) {
186189
return getContentString(headers).getBytes(charset);
187190
}
@@ -191,8 +194,42 @@ public byte[] getContent(List<String> headers, Charset charset) {
191194
*
192195
* @param headers the list of headers to be included in the signed content
193196
* @return formatted content to be signed
197+
* @deprecated use {@link #getStringToSign(List)} to include the {@link Constants#HEADER_REQUEST_TARGET}
198+
* prefix in signature content
194199
*/
200+
@Deprecated
195201
public String getContentString(List<String> headers) {
202+
return getStringToSign(headers, true);
203+
}
204+
205+
/**
206+
* Returns the request content as a String for generating a signature.
207+
*
208+
* @param headers the list of headers to be included in the signed content
209+
* @return formatted content to be signed
210+
*/
211+
public String getStringToSign(List<String> headers) {
212+
return getStringToSign(headers, false);
213+
}
214+
215+
/**
216+
* Returns the request content as a byte array for generating a signature.
217+
*
218+
* @param headers the list of headers to be included in the signed content
219+
* @param charset charset for decoding the content
220+
* @return the result of {@link #getStringToSign(List)} encoded using the provided {@link Charset}
221+
*/
222+
public byte[] getBytesToSign(List<String> headers, Charset charset) {
223+
return getStringToSign(headers).getBytes(charset);
224+
}
225+
226+
/**
227+
* Method created for backwards compatibility.
228+
* @param headers the list of headers to be included in the signed content.
229+
* @param suppressRequestTargetPrefix true to remove "(request-target): " from the content.
230+
* @return formatted content String to be signed.
231+
*/
232+
private String getStringToSign(List<String> headers, boolean suppressRequestTargetPrefix) {
196233
StringBuilder hashBuilder = new StringBuilder();
197234
if (headers != null) {
198235
for (String header : headers) {
@@ -204,6 +241,9 @@ public String getContentString(List<String> headers) {
204241
}
205242
} else if (Constants.HEADER_REQUEST_TARGET.equals(_header)) {
206243
if (this.getRequestTarget() != null) {
244+
if (!suppressRequestTargetPrefix) {
245+
hashBuilder.append(Constants.HEADER_REQUEST_TARGET).append(": ");
246+
}
207247
hashBuilder.append(this.getRequestTarget()).append('\n');
208248
}
209249
} else {
@@ -219,7 +259,7 @@ public String getContentString(List<String> headers) {
219259

220260
@Override
221261
public String toString() {
222-
return getContentString(this.getHeaderNames());
262+
return getStringToSign(this.getHeaderNames());
223263
}
224264

225265
/**

api/src/main/java/net/adamcin/httpsig/api/Signer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public Authorization sign(RequestContent requestContent, List<String> electiveHe
163163

164164
List<String> headers = new ArrayList<String>(signHeaders);
165165

166-
byte[] signature = key.sign(algo, requestContent.getContent(headers, Constants.CHARSET));
166+
byte[] signature = key.sign(algo, requestContent.getBytesToSign(headers, Constants.CHARSET));
167167

168168
if (signature != null) {
169169
return new Authorization(this.keyId.getId(key), Base64.toBase64String(signature), headers, algo);

api/src/test/java/net/adamcin/httpsig/api/DefaultVerifierTest.java

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
package net.adamcin.httpsig.api;
2929

3030

31-
import net.adamcin.commons.testing.junit.TestBody;
32-
import org.junit.Test;
31+
import static org.junit.Assert.assertFalse;
32+
import static org.junit.Assert.assertTrue;
3333

3434
import java.util.Arrays;
35+
import java.util.List;
3536

36-
import static org.junit.Assert.*;
37+
import net.adamcin.commons.testing.junit.TestBody;
38+
import org.junit.Test;
3739

3840
public class DefaultVerifierTest {
3941

@@ -57,4 +59,33 @@ public void testVerify() {
5759
}
5860

5961

62+
@Test
63+
public void testStrictVerify() {
64+
TestBody.test(new TestBody() {
65+
@Override protected void execute() throws Exception {
66+
String fingerprint = "fingerprint";
67+
68+
Keychain identities = new MockKeychain(fingerprint);
69+
DefaultVerifier v = new DefaultVerifier(identities);
70+
DefaultVerifier strictV = new DefaultVerifier(identities, true);
71+
RequestContent requestContent = new RequestContent.Builder().setRequestTarget("get", "index.html").addDateNow().build();
72+
List<String> headers = Arrays.asList(Constants.HEADER_REQUEST_TARGET, Constants.HEADER_DATE);
73+
Challenge c = new Challenge(DefaultVerifierTest.class.getName(),
74+
headers,
75+
Arrays.asList( Algorithm.SSH_RSA ));
76+
77+
byte[] content = requestContent.getContent(headers, Constants.CHARSET);
78+
byte[] strictContent = requestContent.getBytesToSign(headers, Constants.CHARSET);
79+
Authorization authz = new Authorization(fingerprint, MockKey.mockSignBase64(content), headers, Algorithm.SSH_RSA);
80+
Authorization strictAuthz = new Authorization(fingerprint, MockKey.mockSignBase64(strictContent), headers, Algorithm.SSH_RSA);
81+
82+
assertTrue("default verifier should verify default mock signature ", v.verify(c, requestContent, authz));
83+
assertTrue("default verifier should verify strict mock signature ", v.verify(c, requestContent, strictAuthz));
84+
assertTrue("strict verifier should verify strict mock signature ", strictV.verify(c, requestContent, strictAuthz));
85+
assertFalse("strict verifier should not verify default mock signature ", strictV.verify(c, requestContent, authz));
86+
}
87+
});
88+
}
89+
90+
6091
}

hmac/src/main/java/net/adamcin/httpsig/hmac/HmacKey.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public boolean canVerify() {
9494
/**
9595
* Verifies the {@code signatureBytes} against the {@code challengeHash} using an underlying public key
9696
* @param algorithm the selected Signature {@link net.adamcin.httpsig.api.Algorithm}
97-
* @param contentBytes the result of {@link net.adamcin.httpsig.api.RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
97+
* @param contentBytes the result of {@link net.adamcin.httpsig.api.RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
9898
* @param signatureBytes the result of {@link net.adamcin.httpsig.api.Authorization#getSignatureBytes()}
9999
* @return true if signature is valid
100100
*/
@@ -133,7 +133,7 @@ public boolean canSign() {
133133
/**
134134
* Signs the {@code challengeHash} using the specified signature {@link net.adamcin.httpsig.api.Algorithm}
135135
* @param algorithm the selected Signature {@link net.adamcin.httpsig.api.Algorithm}
136-
* @param contentBytes the result of {@link net.adamcin.httpsig.api.RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
136+
* @param contentBytes the result of {@link net.adamcin.httpsig.api.RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
137137
* @return byte array containing the challengeHash signature or null if a signature could not be generated.
138138
*/
139139
public byte[] sign(Algorithm algorithm, byte[] contentBytes) {

mvnvm.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
mvn_version=3.3.9

ssh-jsch/src/main/java/net/adamcin/httpsig/ssh/jsch/JschKey.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public Set<Algorithm> getAlgorithms() {
7979
/**
8080
* {@inheritDoc}
8181
* @param algorithm the selected Signature {@link Algorithm}
82-
* @param challengeHash the result of {@link net.adamcin.httpsig.api.RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
82+
* @param challengeHash the result of {@link net.adamcin.httpsig.api.RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
8383
* @param signatureBytes the result of {@link net.adamcin.httpsig.api.Authorization#getSignatureBytes()}
8484
* @return true if verified
8585
*/
@@ -147,7 +147,7 @@ private Signature getSignature(byte[] challengeHash) throws Exception {
147147
/**
148148
* {@inheritDoc}
149149
* @param algorithm the selected Signature {@link Algorithm}
150-
* @param challengeHash the result of {@link net.adamcin.httpsig.api.RequestContent#getContent(java.util.List, java.nio.charset.Charset)}
150+
* @param challengeHash the result of {@link net.adamcin.httpsig.api.RequestContent#getBytesToSign(java.util.List, java.nio.charset.Charset)}
151151
* @return the generated signature
152152
*/
153153
public byte[] sign(Algorithm algorithm, byte[] challengeHash) {

0 commit comments

Comments
 (0)