Skip to content
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.

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

Digest auth support #10

Merged
merged 10 commits into from
Mar 10, 2019
Next Next commit
Digest-Auth-Support: hack the input buffer to support Digest Auth
mssfang committed Feb 21, 2019
commit 2e6b529e859dfe739ef9691d963215d9537e3887
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@

<groupId>com.microsoft.azure</groupId>
<artifactId>qpid-proton-j-extensions</artifactId>
<version>1.1.0</version>
<version>1.1.0-SNAPSHOT</version>

<url>https://github.com/Azure/qpid-proton-j-extensions</url>

Original file line number Diff line number Diff line change
@@ -10,8 +10,16 @@
import com.microsoft.azure.proton.transport.proxy.Proxy;
import com.microsoft.azure.proton.transport.proxy.ProxyHandler;

import java.net.Authenticator;
import java.net.PasswordAuthentication;

import java.nio.ByteBuffer;
import java.security.MessageDigest;
import java.security.SecureRandom;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;
import java.util.concurrent.atomic.AtomicInteger;

import org.apache.qpid.proton.engine.Transport;
import org.apache.qpid.proton.engine.TransportException;
@@ -21,8 +29,10 @@
import org.apache.qpid.proton.engine.impl.TransportOutput;
import org.apache.qpid.proton.engine.impl.TransportWrapper;

import javax.xml.bind.DatatypeConverter;

public class ProxyImpl implements Proxy, TransportLayer {
private final int proxyHandshakeBufferSize = 4 * 1024; // buffers used only for proxy-handshake
private final int proxyHandshakeBufferSize = 8 * 1024; // buffers used only for proxy-handshake
private final ByteBuffer inputBuffer;
private final ByteBuffer outputBuffer;

@@ -36,6 +46,8 @@ public class ProxyImpl implements Proxy, TransportLayer {

private ProxyHandler proxyHandler;

private final String PROXY_AUTH_DIGEST = "Proxy-Authenticate: Digest";
private final AtomicInteger nonceCounter = new AtomicInteger(0);
/**
* Create proxy transport layer - which, after configuring using
* the {@link #configure(String, Map, ProxyHandler, Transport)} API
@@ -167,9 +179,22 @@ public void process() throws TransportException {
final ProxyHandler.ProxyResponseResult responseResult = proxyHandler
.validateProxyResponse(inputBuffer);
inputBuffer.compact();

inputBuffer.clear();
if (responseResult.getIsSuccess()) {
proxyState = ProxyState.PN_PROXY_CONNECTED;
} else if (responseResult.getError() != null &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous implementation assumes that - we dont negotiate the AuthMechanism with the Proxy and always assumes Digest Auth. In your current design, you have a new state where you are reading proxy challenge and responding with the appropriate response. Can you indicate this by adding a new state for this?

In the case of Proxies with No Auth:

PN_PROXY_NOT_STARTED,
PN_PROXY_CONNECTING,
PN_PROXY_CONNECTED

In case of Proxies with Auth:

PN_PROXY_NOT_STARTED,
PN_PROXY_CONNECTING,
PN_PROXY_CHALLENGE,
PN_PROXY_CHALLENGE_RESPONDED,
PN_PROXY_CONNECTED

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new states added

responseResult.getError().contains(PROXY_AUTH_DIGEST)) {
proxyState = ProxyState.PN_PROXY_NOT_STARTED;
final Scanner responseScanner = new Scanner(responseResult.getError());
final Map<String, String> challengeQuestionValues = new HashMap<String, String>();
while (responseScanner.hasNextLine()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about the scope of this class as -a place holder for the state transistions and buffers. Please refactor processing the challenge headers into a different files.

I would Create an interface something like:

interface ProxyChallengeProcessor {
 string getHeader();
}

implement a basic auth processor and digest auth processor. Select processor based on the Challege response.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added ProxyChallengeProcessor and ProxyChallengeProcessorImpl classes

String line = responseScanner.nextLine();
if (line.contains(PROXY_AUTH_DIGEST)){
getChallengeQuestionHeaders(line, challengeQuestionValues);
break;
}
}
computeChallengeAnswer(challengeQuestionValues);
} else {
tailClosed = true;
underlyingTransport.closed(
@@ -274,5 +299,76 @@ public void close_head() {
headClosed = true;
underlyingOutput.close_head();
}

private void getChallengeQuestionHeaders(String line, Map<String, String> challengeQuestionValues) {
String context = line.substring(PROXY_AUTH_DIGEST.length());
String[] headerValues = context.split(",");

for (String headerValue : headerValues) {
if (headerValue.contains("=")) {
String key = headerValue.substring(0, headerValue.indexOf("="));
String value = headerValue.substring(headerValue.indexOf("=") + 1);
challengeQuestionValues.put(key.trim(), value.replaceAll("\"", "").trim());
}
}
}

private void computeChallengeAnswer(Map<String, String> challengeQuestionValues) {
String uri = host;
PasswordAuthentication passwordAuthentication = Authenticator.requestPasswordAuthentication(
"",
null,
0,
"https",
"Event Hubs client websocket proxy support",
"digest",
null,
Authenticator.RequestorType.PROXY);

String username = passwordAuthentication.getUserName();
String password = passwordAuthentication.getPassword() != null
? new String(passwordAuthentication.getPassword())
: null;

String digestValue;
try {
SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
random.setSeed(System.currentTimeMillis());
byte[] nonceBytes = new byte[16];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find that nonceBytes is ever used. It looks like these four lines can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's cnonceBytes, which is a separate array from nonceBytes. There's a similar duplication with random/secureRandom.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks. Just delete these 4 lines with a new commit.

random.nextBytes(nonceBytes);

String nonce = challengeQuestionValues.get("nonce");
String realm = challengeQuestionValues.get("realm");
String qop = challengeQuestionValues.get("qop");

MessageDigest md5 = MessageDigest.getInstance("md5");
SecureRandom secureRandom = new SecureRandom();
String a1 = DatatypeConverter.printHexBinary(md5.digest(String.format("%s:%s:%s", username, realm, password).getBytes("UTF-8"))).toLowerCase();
String a2 = DatatypeConverter.printHexBinary(md5.digest(String.format("%s:%s", "CONNECT", uri).getBytes("UTF-8"))).toLowerCase();

byte[] cnonceBytes = new byte[16];
secureRandom.nextBytes(cnonceBytes);
String cnonce = DatatypeConverter.printHexBinary(cnonceBytes).toLowerCase();
String response;
if (qop == null || qop.isEmpty()) {
response = DatatypeConverter.printHexBinary(md5.digest(String.format("%s:%s:%s", a1, nonce, a2).getBytes("UTF-8"))).toLowerCase();
digestValue = String.format("Digest username=\"%s\",realm=\"%s\",nonce=\"%s\",uri=\"%s\",cnonce=\"%s\",response=\"%s\"",
username, realm, nonce, uri, cnonce, response);
} else {
int nc = nonceCounter.incrementAndGet();
response = DatatypeConverter.printHexBinary(md5.digest(String.format("%s:%s:%08X:%s:%s:%s", a1, nonce, nc, cnonce, qop, a2).getBytes("UTF-8"))).toLowerCase();
digestValue = String.format("Digest username=\"%s\",realm=\"%s\",nonce=\"%s\",uri=\"%s\",cnonce=\"%s\",nc=%08X,response=\"%s\",qop=\"%s\"",
username, realm, nonce, uri, cnonce, nc, response, qop);
}

if (headers == null) {
headers = new HashMap<>();
}
headers.put("Proxy-Authorization", digestValue);

} catch(Exception ex) {
System.out.println(ex.getMessage());
}
}
}
}
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@
public class ProxyImplTest {

private String hostName = "test.host.name";
private int bufferSize = 4 * 1024;
private int bufferSize = 8 * 1024;
private Map<String, String> headers = new HashMap<>();
private int proxyConnectRequestLength = 132;