HTTP Signature fix to work according to specification. (helidon-io#2884)

security/providers/http-sign/pom.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
 
-    Copyright (c) 2017, 2021 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 2017, 2021 Oracle and/or its affiliates.
 
     Licensed under the Apache License, Version 2.0 (the "License");
     you may not use this file except in compliance with the License.
@@ -42,6 +42,11 @@
             <groupId>io.helidon.common</groupId>
             <artifactId>helidon-common-key-util</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.security</groupId>
+            <artifactId>helidon-security-util</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>io.helidon.security.integration</groupId>
             <artifactId>helidon-security-integration-jersey</artifactId>

security/providers/http-sign/src/main/java/io/helidon/security/providers/httpsign/HttpSignHeader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,5 +28,9 @@ public enum HttpSignHeader {
      * Creates (or validates) an "Authorization" header, that contains "Signature" as the
      * beginning of its content (the rest of the header is the same as for {@link #SIGNATURE}.
      */
-    AUTHORIZATION
+    AUTHORIZATION,
+    /**
+     * Custom provided using a {@link io.helidon.security.util.TokenHandler}.
+     */
+    CUSTOM
 }

security/providers/http-sign/src/main/java/io/helidon/security/providers/httpsign/HttpSignProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -77,6 +77,7 @@ public final class HttpSignProvider implements AuthenticationProvider, OutboundS
     private final OutboundConfig outboundConfig;
     // cache of target name to a signature configuration for outbound calls
     private final Map<String, OutboundTargetDefinition> targetKeys = new HashMap<>();
+    private final boolean backwardCompatibleEol;
 
     private HttpSignProvider(Builder builder) {
         this.optional = builder.optional;
@@ -88,6 +89,7 @@ private HttpSignProvider(Builder builder) {
         this.inboundRequiredHeaders = builder.inboundRequiredHeaders;
         this.inboundKeys = builder.inboundKeys;
         this.outboundConfig = builder.outboundConfig;
+        this.backwardCompatibleEol = builder.backwardCompatibleEol;
 
         outboundConfig.targets().forEach(target -> target.getConfig().ifPresent(targetConfig -> {
             OutboundTargetDefinition outboundTargetDefinition = targetConfig.get("signature")
@@ -193,7 +195,7 @@ private AuthenticationResponse signatureHeader(List<String> signatures,
         String lastError = signatures.isEmpty() ? "No signature values for Signature header" : null;
 
         for (String signature : signatures) {
-            HttpSignature httpSignature = HttpSignature.fromHeader(signature);
+            HttpSignature httpSignature = HttpSignature.fromHeader(signature, backwardCompatibleEol);
             Optional<String> validate = httpSignature.validate();
             if (validate.isPresent()) {
                 lastError = validate.get();
@@ -272,7 +274,10 @@ private OutboundSecurityResponse signRequest(SecurityEnvironment outboundEnv) {
 
             Map<String, List<String>> newHeaders = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
             newHeaders.putAll(outboundEnv.headers());
-            HttpSignature signature = HttpSignature.sign(outboundEnv, targetConfig, newHeaders);
+            HttpSignature signature = HttpSignature.sign(outboundEnv,
+                                                         targetConfig,
+                                                         newHeaders,
+                                                         targetConfig.backwardCompatibleEol());
 
             OutboundSecurityResponse.Builder builder = OutboundSecurityResponse.builder()
                     .requestHeaders(newHeaders)
@@ -285,6 +290,12 @@ private OutboundSecurityResponse signRequest(SecurityEnvironment outboundEnv) {
             case AUTHORIZATION:
                 builder.requestHeader("Authorization", "Signature " + signature.toSignatureHeader());
                 break;
+            case CUSTOM:
+                Map<String, List<String>> headers = new HashMap<>();
+                targetConfig.tokenHandler()
+                        .addHeader(headers, signature.toSignatureHeader());
+                headers.forEach(builder::requestHeader);
+                break;
             default:
                 throw new HttpSignatureException("Invalid header configuration: " + targetConfig.header());
             }
@@ -312,6 +323,10 @@ public static final class Builder implements io.helidon.common.Builder<HttpSignP
         private SignedHeadersConfig inboundRequiredHeaders = SignedHeadersConfig.builder().build();
         private OutboundConfig outboundConfig = OutboundConfig.builder().build();
         private final Map<String, InboundClientDefinition> inboundKeys = new HashMap<>();
+        // not to self - we need to switch default to false in 3.0.0
+        // and probably remove this in 4.0.0
+        @Deprecated
+        private boolean backwardCompatibleEol = true;
 
         private Builder() {
         }
@@ -338,6 +353,8 @@ public Builder config(Config config) {
                     .asList(InboundClientDefinition::create)
                     .ifPresent(list -> list.forEach(inbound -> inboundKeys.put(inbound.keyId(), inbound)));
 
+            config.get("backward-compatible-eol").asBoolean().ifPresent(this::backwardCompatibleEol);
+
             return this;
         }
 
@@ -466,5 +483,18 @@ public Builder realm(String realm) {
             this.realm = realm;
             return this;
         }
+
+        /**
+         * Until version 3.0.0 (exclusive) there is a trailing end of line added to the signed
+         * data.
+         * To be able to communicate cross versions, we must configure this for newer versions
+         *
+         * @param backwardCompatible whether to run in backward compatible mode
+         * @return updated builder instance
+         */
+        public Builder backwardCompatibleEol(Boolean backwardCompatible) {
+            this.backwardCompatibleEol = backwardCompatible;
+            return this;
+        }
     }
 }

security/providers/http-sign/src/main/java/io/helidon/security/providers/httpsign/HttpSignature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
 import java.util.Arrays;
 import java.util.Base64;
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -53,27 +54,33 @@ class HttpSignature {
     private final String keyId;
     private final String algorithm;
     private final List<String> headers;
+    // Backward compatibility with Helidon versions until 3.0.0
+    // the signed string contained a trailing new line
+    private final boolean backwardCompatibleEol;
     private String base64Signature;
 
     private byte[] signatureBytes;
 
-    HttpSignature(String keyId, String algorithm, List<String> headers) {
+    HttpSignature(String keyId, String algorithm, List<String> headers, boolean backwardCompatibleEol) {
         this.keyId = keyId;
         this.algorithm = algorithm;
         this.headers = headers;
+        this.backwardCompatibleEol = backwardCompatibleEol;
     }
 
-    private HttpSignature(String header, String keyId,
+    private HttpSignature(String keyId,
                           String algorithm,
                           List<String> headers,
-                          String base64Signature) {
+                          String base64Signature,
+                          boolean backwardCompatibleEol) {
         this.keyId = keyId;
         this.algorithm = algorithm;
         this.headers = headers;
         this.base64Signature = base64Signature;
+        this.backwardCompatibleEol = backwardCompatibleEol;
     }
 
-    static HttpSignature fromHeader(String header) {
+    static HttpSignature fromHeader(String header, boolean backwardCompatibleEol) {
         /*keyId="rsa-key-1",algorithm="rsa-sha256",
                 headers="(request-target) host date digest content-length",
                 signature="Base64(RSA-SHA256(signing string))"*/
@@ -92,18 +99,18 @@ static HttpSignature fromHeader(String header) {
             int c = header.indexOf(',', b);
             int eq = header.indexOf('=', b);
             if (eq == -1) {
-                return new HttpSignature(header, keyId, algorithm, headers, signature);
+                return new HttpSignature(keyId, algorithm, headers, signature, backwardCompatibleEol);
             }
             if (eq > c) {
                 b = c + 1;
             }
             int qb = header.indexOf('"', eq);
             if (qb == -1) {
-                return new HttpSignature(header, keyId, algorithm, headers, signature);
+                return new HttpSignature(keyId, algorithm, headers, signature, backwardCompatibleEol);
             }
             int qe = header.indexOf('"', qb + 1);
             if (qe == -1) {
-                return new HttpSignature(header, keyId, algorithm, headers, signature);
+                return new HttpSignature(keyId, algorithm, headers, signature, backwardCompatibleEol);
             }
 
             String name = header.substring(b, eq).trim();
@@ -128,19 +135,21 @@ static HttpSignature fromHeader(String header) {
             }
             b = qe + 1;
             if (b >= header.length()) {
-                return new HttpSignature(header, keyId, algorithm, headers, signature);
+                return new HttpSignature(keyId, algorithm, headers, signature, backwardCompatibleEol);
             }
         }
     }
 
     static HttpSignature sign(SecurityEnvironment env,
                               OutboundTargetDefinition outboundDefinition,
-                              Map<String, List<String>> newHeaders) {
+                              Map<String, List<String>> newHeaders,
+                              boolean backwardCompatibleEol) {
 
         HttpSignature signature = new HttpSignature(outboundDefinition.keyId(),
                                                     outboundDefinition.algorithm(),
                                                     outboundDefinition.signedHeadersConfig()
-                                                            .headers(env.method(), env.headers()));
+                                                            .headers(env.method(), env.headers()),
+                                                    backwardCompatibleEol);
 
         // validate algorithm is OK
         //let's try to validate the signature
@@ -259,9 +268,9 @@ private byte[] signRsaSha256(SecurityEnvironment env, KeyConfig keyConfig, Map<S
         try {
             Signature signature = Signature.getInstance("SHA256withRSA");
             signature.initSign(keyConfig.privateKey().orElseThrow(() ->
-                                                                             new HttpSignatureException(
-                                                                                     "Private key is required, yet not "
-                                                                                             + "configured")));
+                                                                          new HttpSignatureException(
+                                                                                  "Private key is required, yet not "
+                                                                                          + "configured")));
             signature.update(getBytesToSign(env, newHeaders));
             return signature.sign();
         } catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) {
@@ -331,18 +340,15 @@ private byte[] getBytesToSign(SecurityEnvironment env, Map<String, List<String>>
     }
 
     String getSignedString(Map<String, List<String>> newHeaders, SecurityEnvironment env) {
-        StringBuilder toSign = new StringBuilder();
         Map<String, List<String>> requestHeaders = env.headers();
+        List<String> linesToSign = new LinkedList<>();
 
         for (String header : this.headers) {
             if ("(request-target)".equals(header)) {
                 //special case
-                toSign.append(header)
-                        .append(": ")
-                        .append(env.method().toLowerCase())
-                        .append(" ")
-                        .append(env.path().orElse("/"))
-                        .append('\n');
+                linesToSign.add(header
+                                        + ": " + env.method().toLowerCase()
+                                        + " " + env.path().orElse("/"));
             } else {
                 List<String> headerValues = requestHeaders.get(header);
                 if (null == headerValues && null == newHeaders) {
@@ -372,15 +378,22 @@ String getSignedString(Map<String, List<String>> newHeaders, SecurityEnvironment
                     }
                 }
 
-                toSign.append(header)
-                        .append(": ")
-                        .append(String.join(" ", headerValues))
-                        .append('\n');
+                linesToSign.add(header + ": " + String.join(" ", headerValues));
             }
         }
 
-        LOGGER.finest(() -> "Data to sign: " + toSign);
+        // 2.3.  Signature String Construction
+        // If value is not the last value then append an ASCII newline `\n`.
+        String toSign = String.join("\n", linesToSign);
 
-        return toSign.toString();
+        if (backwardCompatibleEol) {
+            toSign = toSign + "\n";
+        }
+
+        if (LOGGER.isLoggable(Level.FINEST)) {
+            LOGGER.finest("Data to sign: " + toSign);
+        }
+
+        return toSign;
     }
 }