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 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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,87 @@
package io.approov.service.okhttp;

import java.util.ArrayList;

/* 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 ApproovMessageSigningConfig {
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.

We should add a new interface as follows:

interface MessageSigningConfig {
public String getTargetHeader();
public String generateSigningMessage(Request request, String approovTokenHeader);
public String generateTargetHeaderValue(String messageSignature, String approovTokenHeader);
}

then, this class should instead be called DefaultMessageSigningConfig and it should implement MessageSigningConfig.

// 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;
// true if the message body should also be signed
protected Boolean signBody;
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.

After chatting with Richard - remove the following: signBody, signApproovToken, signURL, and signMethod

// true if the Approov token header is included in the signature
protected Boolean signApproovToken;
// true if target URL should be included in the signature
protected Boolean signURL;
// true if the network request method should be included in the signature
protected Boolean signMethod;
// 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<>();
this.signBody = false;
this.signApproovToken = true;
this.signURL = true;
this.signMethod = true;
}

/* Get/set methods */

/* Get target header */
public String getTargetHeader() {
return targetHeader;
}
/* Get signed headers */
public ArrayList<String> getSignedHeaders() {
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.

I don't think we need the getter, I can't think of any reason that would be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debugging purposes perhaps only?

return signedHeaders;
}
/* Add a header to the list of signed headers: NOTE the sequence of headers DOES matter */
public void addSignedHeader(String header) {
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.

It makes sense to make addSignedHeader chainable to simplify the configuration code. So it would be:
public DefaultMessageSigningConfig addSignedHeader(String header); (returning 'this' at the end of the method)

It would make configuration work like this:
aConfig = new DefaultMessageSigningConfig("header").
addSignedHeader("header1").
addSignedHeader("header2");

if (header == null || header.isEmpty())
throw new IllegalArgumentException("The header must be specified");
signedHeaders.add(header);
}
/* Get signBody flag */
public Boolean getSignBody() {
return signBody;
}
/* Set signBody flag */
public void setSignBody(Boolean signBody) {
this.signBody = signBody;
}
/* Get signApproovToken flag */
public Boolean getSignApproovToken() {
return signApproovToken;
}
/* Set signApproovToken flag */
public void setSignApproovToken(Boolean signApproovToken) {
this.signApproovToken = signApproovToken;
}
/* Get signURL flag */
public Boolean getSignURL() {
return signURL;
}
/* Set signURL flag */
public void setSignURL(Boolean signURL) {
this.signURL = signURL;
}
/* Get signMethod flag */
public Boolean getSignMethod() {
return signMethod;
}
/* Set signMethod flag */
public void setSignMethod(Boolean signMethod) {
this.signMethod = signMethod;
}


}
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
3 changes: 1 addition & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ buildscript {
mavenCentral()
}
dependencies {
classpath 'com.android.tools.build:gradle:7.1.1'
classpath 'com.android.tools.build:gradle:8.1.1' // Update to the latest version
}
}

allprojects {
repositories {
mavenCentral()
google()
maven { url "https://jitpack.io" }
jcenter()
}
}
Expand Down
Binary file modified gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
5 changes: 3 additions & 2 deletions gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.2-bin.zip

Loading