From dc124256113e6acc936e7ae4fdba4871739a045d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Z=C3=B6ller?= Date: Tue, 7 Apr 2026 11:34:19 +0200 Subject: [PATCH] Fixed Signature Check in Inbox --- .../controller/ActivityPubController.java | 193 +++++++++++++++++- .../FederationFollowFlowIntegrationTest.java | 183 ++++++++++++----- 2 files changed, 312 insertions(+), 64 deletions(-) diff --git a/src/main/java/net/javahippie/fitpub/controller/ActivityPubController.java b/src/main/java/net/javahippie/fitpub/controller/ActivityPubController.java index 617505e..51e8a8c 100644 --- a/src/main/java/net/javahippie/fitpub/controller/ActivityPubController.java +++ b/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 - * @param activity the incoming activity - * @return accepted response + *

Performs full HTTP-Signature validation on every incoming request: + *

    + *
  1. Reject if {@code Signature} or {@code Digest} headers are missing.
    2. + *
  2. Verify the {@code Digest} header actually matches the body's SHA-256 hash.
    3. + *
  3. Resolve the signing key by fetching the actor referenced in {@code keyId}.
    4. + *
  4. Verify the request signature with the actor's public key.
    5. + *
  5. 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.
    6. + *
+ * 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 inbox( @PathVariable String username, - @RequestBody Map activity, - @RequestHeader(value = "Signature", required = false) String signature + @RequestBody byte[] body, + 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 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 activity; + try { + activity = objectMapper.readValue(body, new TypeReference>() {}); + } 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 collectHeaders(HttpServletRequest request) { + Map headers = new HashMap<>(); + Enumeration names = request.getHeaderNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + headers.put(name.toLowerCase(Locale.ROOT), request.getHeader(name)); + } + // Synthetic pseudo-header: " " + 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 diff --git a/src/test/java/net/javahippie/fitpub/integration/FederationFollowFlowIntegrationTest.java b/src/test/java/net/javahippie/fitpub/integration/FederationFollowFlowIntegrationTest.java index cfbd7a8..99e3411 100644 --- a/src/test/java/net/javahippie/fitpub/integration/FederationFollowFlowIntegrationTest.java +++ b/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 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 - RemoteActor remoteActor = RemoteActor.builder() - .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); + SignedRemoteActor sender = createSignedRemoteActor( + "https://remote.example/users/bob", "bob", "remote.example", "Bob Remote" + ); // 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) - mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox") - .contentType("application/activity+json") - .content(objectMapper.writeValueAsString(followActivity))) + // Post to inbox with a valid HTTP signature + performSignedInboxPost(testUser.getUsername(), followActivity, sender) .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 - RemoteActor remoteActor = RemoteActor.builder() - .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); + SignedRemoteActor sender = createSignedRemoteActor( + "https://remote.example/users/carol", "carol", "remote.example", "Carol Remote" + ); // 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 - mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox") - .contentType("application/activity+json") - .content(objectMapper.writeValueAsString(acceptActivity))) + // Post Accept to inbox with a valid HTTP signature + performSignedInboxPost(testUser.getUsername(), acceptActivity, sender) .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 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 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 - RemoteActor remoteActor = RemoteActor.builder() - .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); + SignedRemoteActor sender = createSignedRemoteActor( + "https://remote.example/users/dave", "dave", "remote.example", "Dave Remote" + ); // 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 - mockMvc.perform(post("/users/" + testUser.getUsername() + "/inbox") - .contentType("application/activity+json") - .content(objectMapper.writeValueAsString(undoActivity))) + // Post Undo to inbox with a valid HTTP signature + performSignedInboxPost(testUser.getUsername(), undoActivity, sender) .andExpect(status().isAccepted()); // Verify follow was deleted