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

Feature/message signing #6

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 4 additions & 20 deletions approov-service/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@ plugins {
repositories {
mavenCentral()
google()
jcenter()
maven { url "https://jitpack.io" }
}

group = 'com.github.approov'

android {
compileSdkVersion 30

compileSdkVersion 34
namespace 'io.approov.service.okhttp'
defaultConfig {
minSdkVersion 21
targetSdkVersion 28
targetSdkVersion 34
}

buildTypes {
Expand All @@ -34,18 +30,6 @@ android {

dependencies {
implementation 'com.squareup.okhttp3:okhttp:4.12.0'
implementation 'com.github.approov:approov-android-sdk:3.2.2'
implementation 'io.approov:approov-android-sdk:3.3.0'
}

afterEvaluate {
publishing {
publications {
release(MavenPublication) {
from components.release
groupId = 'com.github.approov'
artifactId = 'approov-service-okhttp'
version = '3.2.2'
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package io.approov.service.okhttp;

import java.util.ArrayList;
import okhttp3.Request;

/* Add the following interfaces */
Copy link
Contributor

@jexh jexh Jan 14, 2025

Choose a reason for hiding this comment

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

I've added a bunch of changes here to indicate a new approach. The idea is to construct a message and a signature that can be used to correctly generate and verify the signature on the server side. To do this, while building the headers that will be added to the message we record the names of the headers and the counts of the values as a string. The generated string is then added to the message that will be signed.

The intention then is that this value is added to the header used to hold the signature along with the signature itself. The server side can then break apart the value and use it to reconstruct the message that should be used to verify the signature.

Unfortunately this implementation has a flaw, as headers not in a message but that may influence the behaviour of an endpoint can be freely added to the request and no detection would be made. Ultimately, I think the only legitimate defense against this is that the server side signature chieck also needs to know the set of headers that are expected to be included in the signature and their order. In essence, I don't think it is really possible to have the client side safely communicate to the server the set of headers that it should check in order to evaluate the legitimacy of a signature.

In summary, Ivo and Richard, I think we need some more discussions before we can fix on a nimplementation.

interface MessageSigningConfig {
String getSigningMessage();
String getTargetHeaderName();
String generateTargetHeaderValue(String messageSignature, String approovTokenHeader);
}

interface MessageSigningConfigFactory {
MessageSigningConfig generateMessageSigningConfig(Request request, String approovTokenHeader);
}

/* message signing configuration
* This class is used to configure the message signing feature. The message signature can be computed based on the
Copy link
Contributor

@jexh jexh Jan 11, 2025

Choose a reason for hiding this comment

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

This comment needs to be updated to reflect the changes to the class. It should also explain what the message signing behaviour is, which bits are included in the message, the order and any transformation that is performed (b64 encode the body).

* request URL, the headers specified in signedHeaders in the order in which they are listed and, optionally, the
* body of the message. The signature will be added to the request headers using the header name specified in header.
* You can have multiple configurations for different domains and a default '*' configuration that will be used if no
* specific configuration is found for a domain.
*/
public class DefaultMessageSigningConfigFactory implements MessageSigningConfigFactory {
// the name of the header that will be used to send the message signature
protected String targetHeader;
// the list of headers to include in the message to be signed, in the order they should be added
protected ArrayList<String> signedHeaders;

// constructor
public ApproovMessageSigningConfig(String targetHeader) {
if (targetHeader == null || targetHeader.isEmpty())
throw new IllegalArgumentException("The target header must be specified");
this.targetHeader = targetHeader;
this.signedHeaders = new ArrayList<>();
}

/* Get/set methods */

/* Get target header */
public String getTargetHeader() {
return targetHeader;
}

/* Add a header to the list of signed headers: NOTE the sequence of headers DOES matter */
public DefaultMessageSigningConfigFactory addSignedHeader(String header) {
if (header == null || header.isEmpty())
throw new IllegalArgumentException("The header must be specified");
signedHeaders.add(header);
return this;
}

MessageSigningConfig generateMessageSigningConfig(Request request, String approovTokenHeader){
StringBuilder usedHeadersSpec = new StringBuilder();
StringBuilder message = new StringBuilder();
// 1. Add the Method to the message
message.append(request.method());
message.append("\n");
// 2. add the URL to the message, followed by a newline
message.append(request.url()); // TODO make sure this includes all the URL params if there are any
message.append("\n");
// 3. add the Approov token header to the message
List<String> values = request.headers(approovTokenHeader); // make sure the okhtp lookup works whatever the case used on the header name
usedHeadersSpec.append(approovTokenHeader.toLowerCase() + "," + values.size())
for (String value : values) {
message.append(approovTokenHeader.toLowerCase()).append(":");
if (value != null) {
message.append(value);
}
message.append("\n");
}

// 4. add the required headers to the message as 'headername:headervalue', where the headername is in
// lowercase
if (messageSigningConfig.getSignedHeaders() != null) {
for (String header : messageSigningConfig.signedHeaders) {
// add one headername:headervalue\n entry for each header value to be included in the signature
List<String> values = request.headers(header);
if (values != null && values.length() > 0) {
usedHeadersSpec.append(approovTokenHeader.toLowerCase()+ "," + values.size())
for (String value : values) {
message.append(header.toLowerCase()).append(":");
if (value != null) {
message.append(value);
}
message.append("\n");
}
}
}
}
String finalUsedHeadersSpec = usedHeadersSpec.String()
message.append(finalUsedHeadersSpec)

// add the body to the message
okhttp3.RequestBody body = request.body();
if (body != null) {
Buffer buffer = new Buffer();
body.writeTo(buffer);
message.append(buffer.readUtf8());
}
return new DefaultMessageSigningConfig(
);
}
}

public class DefaultMessageSigningConfig implements MessageSigningConfig
// the name of the header that will be used to send the message signature
private String targetHeader;
// the list of headers with counts that are expected by the server and were also included in the message to be signed
private String usedHeadersSpec;
// the message to be signed
private String message;

DefaultMessageSigningConfig(String targetHeader, String usedHeadersSpec, String message) {
this.targeHeader = targetHeader;
this.usedHeaderSpec = usedHeaderSpec;
this.message = message;
}

public String getTargetHeaderName(){ return targetHeader }
public String getSigningMessage() { return usedHeadersSpec }
public String generateTargetHeaderValue(String messageSignature) {
return usedHeaderSpec + ";" + messageSignature;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okio.Buffer;

// ApproovService provides a mediation layer to the Approov SDK itself
public class ApproovService {
Expand Down Expand Up @@ -82,6 +83,9 @@ public class ApproovService {
// set of URL regexs that should be excluded from any Approov protection, mapped to the compiled Pattern
private static Map<String, Pattern> exclusionURLRegexs = null;

// message signing configurations mapped to a domain
private static Map<String,ApproovMessageSigningConfig> messageSigningConfigs = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Richard convinced me to remove the map - just have a single config. (@richardmtaylor : I'm now a bit concerned about the different headers that will be used for different endpoints / apis - if we include all of these header names in the message signing target header for all requests are we not potentially leaking information about other APIs that are in use in the app to 3rd parties that shouldn't have this information? - having a config per domain would hopefully avoid this.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good point, but I'm not sure it is worth the complexity to hide this. We will only send it on domains that have Approov tokens added anyway. We shouldn't be signing domains that are not Approov protected, since we rely on an Approov token to prevent replay attacks anyway. And all we are leaking is the names of headers, so don't think it is that big of a deal.


/**
* Construction is disallowed as this is a static only class.
*/
Expand Down Expand Up @@ -118,6 +122,31 @@ public static void initialize(Context context, String config) {
}
}

/**
* Sets the message signing configuration. If this is set, then a message signature will be computed based on the
* request URL, the headers specified in signedHeaders in the order in which they are listed and, optionally, the
* body of the message. The signature will be added to the request headers using the header name specified in header.
*
* To unset message signing, call this function as setMessageSigning(nil, nil, false)
*
* @param messageConfig is the ApproovMessageSingingConfig object that contains the message signing configuration
* @param domain is the domain for which the message signing configuration should be used; use '*' for the default configuration which is applied when no specific configuration is found for a domain
*/
public static synchronized void setMessageSigning(ApproovMessageSigningConfig messageConfig, String domain) throws ApproovException {
if ((messageConfig == null) || (domain == null) || (domain.isEmpty())) {
throw new ApproovException("The message signing configuration and domain must be specified");
} else {
// We check if this is the first message configuration that is created
if (messageSigningConfigs == null) {
messageSigningConfigs = new HashMap<>();
}
// Add the configuration and domain to the map
messageSigningConfigs.put(domain, messageConfig);
Log.d(TAG, "setMessageSigning for domain: " + domain);
}

}

/**
* Sets a flag indicating if the network interceptor should proceed anyway if it is
* not possible to obtain an Approov token due to a networking failure. If this is set
Expand Down Expand Up @@ -166,8 +195,12 @@ public static synchronized void setDevKey(String devKey) throws ApproovException
* @param header is the header to place the Approov token on
* @param prefix is any prefix String for the Approov token header
*/
public static synchronized void setApproovHeader(String header, String prefix) {
public static synchronized void setApproovHeader(String header, String prefix) throws ApproovException {
Log.d(TAG, "setApproovHeader " + header + ", " + prefix);
// We only allow this to proceed if message signing is not enabled since changing the header after configuring the message signing is not allowed
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to throw here anymore. The active approov header name is passed to the message signing config every time it processes a request.

if (messageSigningConfigs != null) {
throw new ApproovException("The Approov Token header cannot be changed after configuring the message signing");
}
approovTokenHeader = header;
approovTokenPrefix = prefix;
okHttpClient = null;
Expand Down Expand Up @@ -396,7 +429,7 @@ public static void setDataHashInToken(String data) throws ApproovException {
* is not possible to use the networking interception to add the token. This will
* likely require network access so may take some time to complete. If the attestation fails
* for any reason then an ApproovException is thrown. This will be ApproovNetworkException for
* networking issues wher a user initiated retry of the operation should be allowed. Note that
* networking issues where a user initiated retry of the operation should be allowed. Note that
* the returned token should NEVER be cached by your app, you should call this function when
* it is needed.
*
Expand Down Expand Up @@ -432,6 +465,7 @@ else if (approovResults.getStatus() != Approov.TokenFetchStatus.SUCCESS)
return approovResults.getToken();
}


/**
* Gets the signature for the given message. This uses an account specific message signing key that is
* transmitted to the SDK after a successful fetch if the facility is enabled for the account. Note
Expand Down Expand Up @@ -629,7 +663,7 @@ public static synchronized OkHttpClient getOkHttpClient() {
Log.d(TAG, "Building new Approov OkHttpClient");
ApproovTokenInterceptor interceptor = new ApproovTokenInterceptor(approovTokenHeader,
approovTokenPrefix, bindingHeader, proceedOnNetworkFail, substitutionHeaders,
substitutionQueryParams, exclusionURLRegexs);
substitutionQueryParams, exclusionURLRegexs, messageSigningConfigs);
okHttpClient = okHttpBuilder.certificatePinner(pinBuilder.build()).addInterceptor(interceptor).build();
} else {
// if the ApproovService was not initialized then we can't add Approov capabilities
Expand Down Expand Up @@ -688,6 +722,9 @@ class ApproovTokenInterceptor implements Interceptor {
// set of URL regexs that should be excluded from any Approov protection, mapped to the compiled Pattern
private Map<String, Pattern> exclusionURLRegexs;

// message signing configuration, if any
private Map<String,ApproovMessageSigningConfig> messageSigningConfigs;

/**
* Constructs a new interceptor that adds Approov tokens and substitute headers or query
* parameters.
Expand All @@ -702,7 +739,8 @@ class ApproovTokenInterceptor implements Interceptor {
*/
public ApproovTokenInterceptor(String approovTokenHeader, String approovTokenPrefix, String bindingHeader,
boolean proceedOnNetworkFail, Map<String, String> substitutionHeaders,
Set<String> substitutionQueryParams, Map<String, Pattern> exclusionURLRegexs) {
Set<String> substitutionQueryParams, Map<String, Pattern> exclusionURLRegexs,
Map<String,ApproovMessageSigningConfig> messageSigningConfigs) {
this.approovTokenHeader = approovTokenHeader;
this.approovTokenPrefix = approovTokenPrefix;
this.bindingHeader = bindingHeader;
Expand All @@ -719,6 +757,7 @@ public ApproovTokenInterceptor(String approovTokenHeader, String approovTokenPre
}
}
this.exclusionURLRegexs = new HashMap<>(exclusionURLRegexs);
this.messageSigningConfigs = messageSigningConfigs;
}

@Override
Expand All @@ -738,6 +777,8 @@ public Response intercept(Chain chain) throws IOException {

// request an Approov token for the domain
String host = request.url().host();

// fetch the Approov token for the domain
Approov.TokenFetchResult approovResults = Approov.fetchApproovTokenAndWait(host);

// provide information about the obtained token or error (note "approov token -check" can
Expand Down Expand Up @@ -766,16 +807,16 @@ public Response intercept(Chain chain) throws IOException {
// we successfully obtained a token so add it to the header for the request
request = request.newBuilder().header(approovTokenHeader, approovTokenPrefix + approovResults.getToken()).build();
else if ((approovResults.getStatus() == Approov.TokenFetchStatus.NO_NETWORK) ||
(approovResults.getStatus() == Approov.TokenFetchStatus.POOR_NETWORK) ||
(approovResults.getStatus() == Approov.TokenFetchStatus.MITM_DETECTED)) {
(approovResults.getStatus() == Approov.TokenFetchStatus.POOR_NETWORK) ||
(approovResults.getStatus() == Approov.TokenFetchStatus.MITM_DETECTED)) {
// we are unable to get an Approov token due to network conditions so the request can
// be retried by the user later - unless this is overridden
if (!proceedOnNetworkFail)
throw new ApproovNetworkException("Approov token fetch for " + host + ": " + approovResults.getStatus().toString());
}
else if ((approovResults.getStatus() != Approov.TokenFetchStatus.NO_APPROOV_SERVICE) &&
(approovResults.getStatus() != Approov.TokenFetchStatus.UNKNOWN_URL) &&
(approovResults.getStatus() != Approov.TokenFetchStatus.UNPROTECTED_URL))
(approovResults.getStatus() != Approov.TokenFetchStatus.UNKNOWN_URL) &&
(approovResults.getStatus() != Approov.TokenFetchStatus.UNPROTECTED_URL))
// we have failed to get an Approov token with a more serious permanent error
throw new ApproovException("Approov token fetch for " + host + ": " + approovResults.getStatus().toString());

Expand Down Expand Up @@ -862,6 +903,69 @@ else if (approovResults.getStatus() != Approov.TokenFetchStatus.UNKNOWN_KEY)
}
}

// if message signing is enabled, add the signature header to the request
if (messageSigningConfigs != null) {
// Match the domain of the request URL against the configured message signing domains
String domain = request.url().host();
ApproovMessageSigningConfig messageSigningConfig = messageSigningConfigs.get(domain);
if (messageSigningConfig == null) {
// If no specific configuration is found for the domain, use the default configuration
domain = "*";
messageSigningConfig = messageSigningConfigs.get(domain);
}
if (messageSigningConfig == null) {
// If no default configuration is found, do not sign the message
return chain.proceed(request);
}
Log.d(TAG, "Signing message with configuration for domain: " + domain + ": " + (messageSigningConfig.getSignApproovToken() ? ", " + approovTokenHeader + " **** " : "") + " headers " + messageSigningConfig.getSignedHeaders() +
Copy link
Contributor

Choose a reason for hiding this comment

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

This code should all move into the "public String generateMessage(request Request, String approovTokenHeader);" in the DefaultMessageSigningConfig implementation.

(messageSigningConfig.getSignBody() ? ", body" : ""));
// build the message to sign, consisting of the URL, the names and values of the included headers and
// the body, if enabled, where each entry is separated from the next by a newline character
StringBuilder message = new StringBuilder();
// 1. add the URL to the message, followed by a newline
message.append(request.url());
message.append("\n");
// 2. Add the Method to the message
message.append(request.method());
message.append("\n");
// 3. add the Approov token to the message, if required (TODO: Note we add the approov token and the prefix!?????)
if (messageSigningConfig.signApproovToken) {
if (approovResults.getToken() != null) {
message.append(approovTokenPrefix + approovResults.getToken());
Copy link
Contributor

Choose a reason for hiding this comment

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

You should read the approov header from the request instead of recreating the contents here - it is just a safer approach as the same process will occur on the server side -once you migrate the code, into the DefaultRequestSigningConfig you will have to do that anyway.

}
message.append("\n");
}
// 4. add the required headers to the message as 'headername:headervalue', where the headername is in
// lowercase
if (messageSigningConfig.getSignedHeaders() != null) {
for (String header : messageSigningConfig.signedHeaders) {
// add one headername:headervalue\n entry for each header value to be included in the signature
List<String> values = request.headers(header);
for (String value : values) {
message.append(header.toLowerCase()).append(":");
if (value != null) {
message.append(value);
}
message.append("\n");
}
}
}
// add the body to the message
if (messageSigningConfig.getSignBody()) {
okhttp3.RequestBody body = request.body();
if (body != null) {
Buffer buffer = new Buffer();
body.writeTo(buffer);
message.append(buffer.readUtf8());
Copy link
Contributor

@jexh jexh Jan 13, 2025

Choose a reason for hiding this comment

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

We need to b64 encode the body to make sure there are no control characters in the string when it is passed to the approov SDK signing method.

}
}
// compute the signature and add it to the request (passing on any exceptions that may occur)
if (messageSigningConfig.targetHeader != null && !messageSigningConfig.targetHeader.isEmpty()) {
String signature = ApproovService.getMessageSignature(message.toString());
request = request.newBuilder().header(messageSigningConfig.targetHeader, signature).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Once we have the signature, call the 'generateHeaderValue' method on the MessageSigningConfig to generate the string value for the header instead of just adding the signature directly.

}
}

// proceed with the rest of the chain
return chain.proceed(request);
}
Expand Down
Loading