Fixed Signature Check in Inbox

src/main/java/net/javahippie/fitpub/controller/ActivityPubController.java

@@ -1,15 +1,21 @@
 package net.javahippie.fitpub.controller;
 
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.servlet.http.HttpServletRequest;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import net.javahippie.fitpub.model.activitypub.Actor;
 import net.javahippie.fitpub.model.activitypub.OrderedCollection;
 import net.javahippie.fitpub.model.entity.Activity;
+import net.javahippie.fitpub.model.entity.RemoteActor;
 import net.javahippie.fitpub.model.entity.User;
 import net.javahippie.fitpub.repository.FollowRepository;
 import net.javahippie.fitpub.repository.ActivityRepository;
 import net.javahippie.fitpub.repository.UserRepository;
+import net.javahippie.fitpub.security.HttpSignatureValidator;
 import net.javahippie.fitpub.service.ActivityImageService;
+import net.javahippie.fitpub.service.FederationService;
 import net.javahippie.fitpub.service.InboxProcessor;
 import net.javahippie.fitpub.util.ActivityFormatter;
 import org.springframework.beans.factory.annotation.Value;
@@ -19,7 +25,12 @@ import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
 
 import java.io.File;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.*;
+import java.util.regex.Pattern;
 
 /**
  * ActivityPub protocol controller.
@@ -37,6 +48,9 @@ public class ActivityPubController {
     private final ActivityImageService activityImageService;
     private final InboxProcessor inboxProcessor;
     private final FollowRepository followRepository;
+    private final HttpSignatureValidator signatureValidator;
+    private final FederationService federationService;
+    private final ObjectMapper objectMapper;
 
     @Value("${fitpub.base-url}")
     private String baseUrl;
@@ -44,6 +58,9 @@ public class ActivityPubController {
     private static final String ACTIVITY_JSON = "application/activity+json";
     private static final String LD_JSON = "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"";
 
+    /** Matches the keyId field of an HTTP Signature header. */
+    private static final Pattern SIGNATURE_KEY_ID_PATTERN = Pattern.compile("keyId=\"([^\"]+)\"");
+
     /**
      * Actor profile endpoint.
      * Returns the ActivityPub Actor object for a user.
@@ -74,9 +91,23 @@ public class ActivityPubController {
      * Inbox endpoint for receiving ActivityPub activities.
      * POST /users/{username}/inbox
      *
-     * @param username the username
+     * <p>Performs full HTTP-Signature validation on every incoming request:
-     * @param activity the incoming activity
+     * <ol>
-     * @return accepted response
+     *   <li>Reject if {@code Signature} or {@code Digest} headers are missing.</li>
+     *   <li>Verify the {@code Digest} header actually matches the body's SHA-256 hash.</li>
+     *   <li>Resolve the signing key by fetching the actor referenced in {@code keyId}.</li>
+     *   <li>Verify the request signature with the actor's public key.</li>
+     *   <li>Bind the activity's {@code actor} field to the signing key's host so a federated
+     *       server cannot deliver activities on behalf of users on a different host.</li>
+     * </ol>
+     * Any failure of steps 1–5 produces a 401. Transient upstream failures (cannot fetch the
+     * actor) produce 502 so the sender will retry per ActivityPub spec.
+     *
+     * @param username the local recipient username
+     * @param body the raw request body bytes (preserved for digest verification)
+     * @param request the inbound request, used for header collection and request-target
+     * @return 202 Accepted on success, 401 Unauthorized on signature failures, 502 on upstream
+     *         failures, 400 on malformed JSON
      */
     @PostMapping(
         value = "/users/{username}/inbox",
@@ -84,24 +115,168 @@ public class ActivityPubController {
     )
     public ResponseEntity<Void> inbox(
         @PathVariable String username,
-        @RequestBody Map<String, Object> activity,
+        @RequestBody byte[] body,
-        @RequestHeader(value = "Signature", required = false) String signature
+        HttpServletRequest request
     ) {
-        log.info("Received ActivityPub activity for user {}: {}", username, activity.get("type"));
+        // 1. Signature header required
+        String signatureHeader = request.getHeader("Signature");
+        if (signatureHeader == null || signatureHeader.isBlank()) {
+            log.warn("Inbox request for user {} missing Signature header", username);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
 
-        // TODO: Validate HTTP signature (signature validation can be added later)
+        // 2. Digest header required and must actually match the body.
+        // The signature covers the digest header value, but verifying the digest against the
+        // real body closes the loop: an attacker who lifted a signed envelope cannot swap the
+        // payload underneath it.
+        String digestHeader = request.getHeader("Digest");
+        if (digestHeader == null || digestHeader.isBlank()) {
+            log.warn("Inbox request for user {} missing Digest header", username);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+        String expectedDigest = computeSha256Digest(body);
+        if (!expectedDigest.equals(digestHeader.trim())) {
+            log.warn("Inbox request for user {}: Digest header does not match body hash", username);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
 
-        // Process activity asynchronously to avoid blocking the sender
+        // 3. Extract the keyId from the signature header
+        String keyId = extractKeyId(signatureHeader);
+        if (keyId == null) {
+            log.warn("Inbox request for user {}: Signature header has no keyId", username);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
+        // The keyId is conventionally "https://example.com/users/alice#main-key"; strip
+        // the fragment to get the actor URI we should fetch.
+        String actorUriFromKey = keyId.contains("#") ? keyId.substring(0, keyId.indexOf('#')) : keyId;
+        URI keyHostUri;
+        try {
+            keyHostUri = new URI(actorUriFromKey);
+            if (keyHostUri.getHost() == null) {
+                throw new URISyntaxException(actorUriFromKey, "missing host");
+            }
+        } catch (URISyntaxException e) {
+            log.warn("Inbox request for user {}: keyId is not a valid URI: {}", username, keyId);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
+        // 4. Fetch the actor and obtain their public key
+        String publicKeyPem;
+        try {
+            RemoteActor remoteActor = federationService.fetchRemoteActor(actorUriFromKey);
+            publicKeyPem = remoteActor.getPublicKey();
+        } catch (Exception e) {
+            // Couldn't reach upstream — treat as transient and let them retry
+            log.warn("Inbox request for user {}: failed to fetch remote actor {} for signature verification",
+                username, actorUriFromKey, e);
+            return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build();
+        }
+        if (publicKeyPem == null || publicKeyPem.isBlank()) {
+            log.warn("Inbox request for user {}: remote actor {} has no public key",
+                username, actorUriFromKey);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
+        // 5. Verify the signature against all the headers it claims to cover
+        Map<String, String> headers = collectHeaders(request);
+        if (!signatureValidator.validate(signatureHeader, headers, publicKeyPem)) {
+            log.warn("Inbox request for user {}: HTTP signature verification failed (signed by {})",
+                username, actorUriFromKey);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
+        // 6. Parse the JSON payload (only after the signature is verified, so we don't
+        // waste cycles on unauthenticated input)
+        Map<String, Object> activity;
+        try {
+            activity = objectMapper.readValue(body, new TypeReference<Map<String, Object>>() {});
+        } catch (Exception e) {
+            log.warn("Inbox request for user {}: malformed JSON payload", username, e);
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
+        }
+
+        // 7. Bind the activity's actor field to the signing key host. Without this check,
+        // a federated server signing as one of its own actors could deliver activities
+        // claiming to be from any other server's user.
+        Object actorField = activity.get("actor");
+        String activityActorUri = actorField instanceof String ? (String) actorField : null;
+        if (activityActorUri == null || activityActorUri.isBlank()) {
+            log.warn("Inbox request for user {}: activity is missing a string actor field", username);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+        try {
+            URI actorUri = new URI(activityActorUri);
+            String activityHost = actorUri.getHost();
+            if (activityHost == null
+                || !activityHost.equalsIgnoreCase(keyHostUri.getHost())) {
+                log.warn("Inbox request for user {}: activity actor host '{}' does not match signing key host '{}'",
+                    username, activityHost, keyHostUri.getHost());
+                return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+            }
+        } catch (URISyntaxException e) {
+            log.warn("Inbox request for user {}: activity actor URI is invalid: {}",
+                username, activityActorUri);
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
+        log.info("Received signed ActivityPub activity for user {}: {} from {}",
+            username, activity.get("type"), activityActorUri);
+
+        // 8. Hand off to the processor. Errors here are logged but do not affect the
+        // ack we send back to the federated server (per ActivityPub spec, the inbox
+        // returns 202 Accepted once the message is queued for processing).
         try {
             inboxProcessor.processActivity(username, activity);
         } catch (Exception e) {
             log.error("Error processing inbox activity", e);
         }
 
-        // Always return 202 Accepted per ActivityPub spec
         return ResponseEntity.status(HttpStatus.ACCEPTED).build();
     }
 
+    /**
+     * Computes the SHA-256 digest of the request body in the format used by the HTTP
+     * Signatures spec ({@code "SHA-256=" + base64(sha256(body))}).
+     */
+    private static String computeSha256Digest(byte[] body) {
+        try {
+            MessageDigest md = MessageDigest.getInstance("SHA-256");
+            return "SHA-256=" + Base64.getEncoder().encodeToString(md.digest(body));
+        } catch (NoSuchAlgorithmException e) {
+            // SHA-256 is required by every JRE; this branch is unreachable.
+            throw new IllegalStateException("SHA-256 algorithm not available", e);
+        }
+    }
+
+    /** Extracts the {@code keyId} value from a Signature header. */
+    private static String extractKeyId(String signatureHeader) {
+        var matcher = SIGNATURE_KEY_ID_PATTERN.matcher(signatureHeader);
+        return matcher.find() ? matcher.group(1) : null;
+    }
+
+    /**
+     * Collects request headers (lowercased) plus the synthetic {@code (request-target)}
+     * pseudo-header used by HTTP Signatures.
+     */
+    private static Map<String, String> collectHeaders(HttpServletRequest request) {
+        Map<String, String> headers = new HashMap<>();
+        Enumeration<String> names = request.getHeaderNames();
+        while (names.hasMoreElements()) {
+            String name = names.nextElement();
+            headers.put(name.toLowerCase(Locale.ROOT), request.getHeader(name));
+        }
+        // Synthetic pseudo-header: "<method-lowercase> <path-with-query>"
+        String path = request.getRequestURI();
+        String query = request.getQueryString();
+        if (query != null && !query.isEmpty()) {
+            path = path + "?" + query;
+        }
+        headers.put("(request-target)",
+            request.getMethod().toLowerCase(Locale.ROOT) + " " + path);
+        return headers;
+    }
+
     /**
      * Outbox endpoint for user's activities.
      * GET /users/{username}/outbox

src/test/java/net/javahippie/fitpub/integration/FederationFollowFlowIntegrationTest.java

@@ -12,6 +12,7 @@ import net.javahippie.fitpub.model.entity.User;
 import net.javahippie.fitpub.repository.FollowRepository;
 import net.javahippie.fitpub.repository.RemoteActorRepository;
 import net.javahippie.fitpub.repository.UserRepository;
+import net.javahippie.fitpub.security.HttpSignatureValidator;
 import net.javahippie.fitpub.security.JwtTokenProvider;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
@@ -68,6 +69,9 @@ class FederationFollowFlowIntegrationTest {
     @Autowired
     private JwtTokenProvider jwtTokenProvider;
 
+    @Autowired
+    private HttpSignatureValidator signatureValidator;
+
     @Value("${fitpub.base-url}")
     private String baseUrl;
 
@@ -113,6 +117,68 @@ class FederationFollowFlowIntegrationTest {
         return "-----BEGIN PRIVATE KEY-----\n" + base64 + "\n-----END PRIVATE KEY-----";
     }
 
+    /**
+     * Test fixture pairing a persisted RemoteActor with the keypair the test should use
+     * to sign inbox requests on its behalf.
+     */
+    private record SignedRemoteActor(RemoteActor actor, KeyPair keyPair) {
+        String keyId() {
+            return actor.getActorUri() + "#main-key";
+        }
+    }
+
+    /**
+     * Creates a remote actor backed by a real RSA keypair, persists it with the matching
+     * public key PEM, and returns both. Because {@code lastFetchedAt} is set to now, the
+     * controller's federation service will use the cached row instead of making an HTTP
+     * call to the remote actor URI during signature verification.
+     */
+    private SignedRemoteActor createSignedRemoteActor(String actorUri, String username,
+                                                      String domain, String displayName)
+            throws NoSuchAlgorithmException {
+        KeyPair keyPair = generateRsaKeyPair();
+        String publicKeyPem = encodePublicKey(keyPair.getPublic().getEncoded());
+        RemoteActor actor = RemoteActor.builder()
+            .actorUri(actorUri)
+            .username(username)
+            .domain(domain)
+            .displayName(displayName)
+            .inboxUrl(actorUri + "/inbox")
+            .outboxUrl(actorUri + "/outbox")
+            .publicKey(publicKeyPem)
+            .publicKeyId(actorUri + "#main-key")
+            .lastFetchedAt(Instant.now())
+            .build();
+        return new SignedRemoteActor(remoteActorRepository.save(actor), keyPair);
+    }
+
+    /**
+     * Posts the given activity payload to {@code /users/{username}/inbox} with a valid
+     * HTTP-Signature, exactly as a real federated server would. The Host header is set
+     * to {@code localhost} so it matches what {@link HttpSignatureValidator#signRequest}
+     * derives from the inbox URL.
+     */
+    private org.springframework.test.web.servlet.ResultActions performSignedInboxPost(
+            String recipientUsername, Map<String, Object> activity, SignedRemoteActor sender)
+            throws Exception {
+        String body = objectMapper.writeValueAsString(activity);
+        String inboxPath = "/users/" + recipientUsername + "/inbox";
+        // signRequest derives host from this URL via URI.getHost(); the Host header on the
+        // mock request must match.
+        String inboxUrl = "http://localhost" + inboxPath;
+        String privateKeyPem = encodePrivateKey(sender.keyPair().getPrivate().getEncoded());
+        HttpSignatureValidator.SignatureHeaders sigHeaders = signatureValidator.signRequest(
+            "POST", inboxUrl, body, privateKeyPem, sender.keyId()
+        );
+        return mockMvc.perform(post(inboxPath)
+                .contentType("application/activity+json")
+                .header("Host", sigHeaders.host)
+                .header("Date", sigHeaders.date)
+                .header("Digest", sigHeaders.digest)
+                .header("Signature", sigHeaders.signature)
+                .content(body));
+    }
+
     @Test
     @Disabled("Requires mocking external HTTP calls to WebFinger and remote ActivityPub servers")
     @DisplayName("Should follow a remote user via handle format @username@domain")
@@ -140,18 +206,9 @@ class FederationFollowFlowIntegrationTest {
     @Test
     @DisplayName("Should process incoming Follow activity and create follow relationship")
     void testProcessIncomingFollowActivity() throws Exception {
-        // Create a remote actor
+        SignedRemoteActor sender = createSignedRemoteActor(
-        RemoteActor remoteActor = RemoteActor.builder()
+            "https://remote.example/users/bob", "bob", "remote.example", "Bob Remote"
-            .actorUri("https://remote.example/users/bob")
+        );
-            .username("bob")
-            .domain("remote.example")
-            .displayName("Bob Remote")
-            .inboxUrl("https://remote.example/users/bob/inbox")
-            .outboxUrl("https://remote.example/users/bob/outbox")
-            .publicKey("-----BEGIN PUBLIC KEY-----\ntest\n-----END PUBLIC KEY-----")
-            .lastFetchedAt(Instant.now())
-            .build();
-        remoteActor = remoteActorRepository.save(remoteActor);
 
         // Create Follow activity
         String followId = "https://remote.example/activities/follow/" + UUID.randomUUID();
@@ -159,20 +216,18 @@ class FederationFollowFlowIntegrationTest {
             "@context", "https://www.w3.org/ns/activitystreams",
             "type", "Follow",
             "id", followId,
-            "actor", remoteActor.getActorUri(),
+            "actor", sender.actor().getActorUri(),
             "object", baseUrl + "/users/" + testUser.getUsername(),
             "published", Instant.now().toString()
         );
 
-        // Post to inbox (without signature validation for test)
+        // Post to inbox with a valid HTTP signature
-        mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox")
+        performSignedInboxPost(testUser.getUsername(), followActivity, sender)
-                .contentType("application/activity+json")
-                .content(objectMapper.writeValueAsString(followActivity)))
             .andExpect(status().isAccepted());
 
         // Verify follow relationship was created
         Follow follow = followRepository.findByRemoteActorUriAndFollowingActorUri(
-            remoteActor.getActorUri(),
+            sender.actor().getActorUri(),
             baseUrl + "/users/" + testUser.getUsername()
         ).orElse(null);
 
@@ -183,24 +238,15 @@ class FederationFollowFlowIntegrationTest {
     @Test
     @DisplayName("Should process Accept activity and update follow status to ACCEPTED")
     void testProcessAcceptActivity() throws Exception {
-        // Create a remote actor
+        SignedRemoteActor sender = createSignedRemoteActor(
-        RemoteActor remoteActor = RemoteActor.builder()
+            "https://remote.example/users/carol", "carol", "remote.example", "Carol Remote"
-            .actorUri("https://remote.example/users/carol")
+        );
-            .username("carol")
-            .domain("remote.example")
-            .displayName("Carol Remote")
-            .inboxUrl("https://remote.example/users/carol/inbox")
-            .outboxUrl("https://remote.example/users/carol/outbox")
-            .publicKey("-----BEGIN PUBLIC KEY-----\ntest\n-----END PUBLIC KEY-----")
-            .lastFetchedAt(Instant.now())
-            .build();
-        remoteActor = remoteActorRepository.save(remoteActor);
 
         // Create pending follow
         String followActivityId = baseUrl + "/activities/follow/" + UUID.randomUUID();
         Follow pendingFollow = Follow.builder()
             .followerId(testUser.getId())
-            .followingActorUri(remoteActor.getActorUri())
+            .followingActorUri(sender.actor().getActorUri())
             .status(Follow.FollowStatus.PENDING)
             .activityId(followActivityId)
             .build();
@@ -211,14 +257,12 @@ class FederationFollowFlowIntegrationTest {
             "@context", "https://www.w3.org/ns/activitystreams",
             "type", "Accept",
             "id", "https://remote.example/activities/accept/" + UUID.randomUUID(),
-            "actor", remoteActor.getActorUri(),
+            "actor", sender.actor().getActorUri(),
             "object", followActivityId
         );
 
-        // Post Accept to inbox
+        // Post Accept to inbox with a valid HTTP signature
-        mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox")
+        performSignedInboxPost(testUser.getUsername(), acceptActivity, sender)
-                .contentType("application/activity+json")
-                .content(objectMapper.writeValueAsString(acceptActivity)))
             .andExpect(status().isAccepted());
 
         // Verify follow status was updated to ACCEPTED
@@ -226,25 +270,56 @@ class FederationFollowFlowIntegrationTest {
         assertThat(updatedFollow.getStatus()).isEqualTo(Follow.FollowStatus.ACCEPTED);
     }
 
+    @Test
+    @DisplayName("Should reject inbox POST without HTTP signature with 401")
+    void testInboxRejectsUnsignedRequest() throws Exception {
+        Map<String, Object> followActivity = Map.of(
+            "@context", "https://www.w3.org/ns/activitystreams",
+            "type", "Follow",
+            "id", "https://remote.example/activities/follow/" + UUID.randomUUID(),
+            "actor", "https://remote.example/users/bob",
+            "object", baseUrl + "/users/" + testUser.getUsername()
+        );
+
+        mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox")
+                .contentType("application/activity+json")
+                .content(objectMapper.writeValueAsString(followActivity)))
+            .andExpect(status().isUnauthorized());
+    }
+
+    @Test
+    @DisplayName("Should reject inbox POST when activity actor host does not match signing key host")
+    void testInboxRejectsActorHostMismatch() throws Exception {
+        // The signing actor lives on remote.example, but the activity claims to be from
+        // someone on impostor.example. The controller must reject this with 401 to
+        // prevent one federated server impersonating users on another.
+        SignedRemoteActor sender = createSignedRemoteActor(
+            "https://remote.example/users/bob", "bob", "remote.example", "Bob Remote"
+        );
+
+        Map<String, Object> followActivity = Map.of(
+            "@context", "https://www.w3.org/ns/activitystreams",
+            "type", "Follow",
+            "id", "https://remote.example/activities/follow/" + UUID.randomUUID(),
+            // Forged: claims to be from a user on a completely different host
+            "actor", "https://impostor.example/users/eve",
+            "object", baseUrl + "/users/" + testUser.getUsername()
+        );
+
+        performSignedInboxPost(testUser.getUsername(), followActivity, sender)
+            .andExpect(status().isUnauthorized());
+    }
+
     @Test
     @DisplayName("Should process Undo Follow activity and remove follow relationship")
     void testProcessUndoFollowActivity() throws Exception {
-        // Create a remote actor
+        SignedRemoteActor sender = createSignedRemoteActor(
-        RemoteActor remoteActor = RemoteActor.builder()
+            "https://remote.example/users/dave", "dave", "remote.example", "Dave Remote"
-            .actorUri("https://remote.example/users/dave")
+        );
-            .username("dave")
-            .domain("remote.example")
-            .displayName("Dave Remote")
-            .inboxUrl("https://remote.example/users/dave/inbox")
-            .outboxUrl("https://remote.example/users/dave/outbox")
-            .publicKey("-----BEGIN PUBLIC KEY-----\ntest\n-----END PUBLIC KEY-----")
-            .lastFetchedAt(Instant.now())
-            .build();
-        remoteActor = remoteActorRepository.save(remoteActor);
 
         // Create accepted follow
         Follow acceptedFollow = Follow.builder()
-            .remoteActorUri(remoteActor.getActorUri())
+            .remoteActorUri(sender.actor().getActorUri())
             .followingActorUri(baseUrl + "/users/" + testUser.getUsername())
             .status(Follow.FollowStatus.ACCEPTED)
             .build();
@@ -255,18 +330,16 @@ class FederationFollowFlowIntegrationTest {
             "@context", "https://www.w3.org/ns/activitystreams",
             "type", "Undo",
             "id", "https://remote.example/activities/undo/" + UUID.randomUUID(),
-            "actor", remoteActor.getActorUri(),
+            "actor", sender.actor().getActorUri(),
             "object", Map.of(
                 "type", "Follow",
-                "actor", remoteActor.getActorUri(),
+                "actor", sender.actor().getActorUri(),
                 "object", baseUrl + "/users/" + testUser.getUsername()
             )
         );
 
-        // Post Undo to inbox
+        // Post Undo to inbox with a valid HTTP signature
-        mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox")
+        performSignedInboxPost(testUser.getUsername(), undoActivity, sender)
-                .contentType("application/activity+json")
-                .content(objectMapper.writeValueAsString(undoActivity)))
             .andExpect(status().isAccepted());
 
         // Verify follow was deleted