oscerd commented on code in PR #27716:
URL: https://github.com/apache/camel/pull/27716#discussion_r4251331163
##########
components/camel-telegram/src/main/java/org/apache/camel/component/telegram/TelegramWebhookProcessor.java:
##########
@@ -26,21 +28,51 @@
import org.apache.camel.component.telegram.model.Update;
import org.apache.camel.support.AsyncProcessorConverterHelper;
import org.apache.camel.support.AsyncProcessorSupport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import static
org.apache.camel.component.telegram.util.TelegramMessageHelper.populateExchange;
public class TelegramWebhookProcessor extends AsyncProcessorSupport {
+ /**
+ * The header in which Telegram sends the secret token the webhook was
registered with.
+ */
+ public static final String SECRET_TOKEN_HEADER =
"X-Telegram-Bot-Api-Secret-Token";
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TelegramWebhookProcessor.class);
private static final ObjectMapper MAPPER = new ObjectMapper();
private AsyncProcessor next;
+ private final byte[] secretToken;
- public TelegramWebhookProcessor(Processor next) {
+ /**
+ * @param next the processor that receives the updates
+ * @param secretToken the secret token that a webhook request must carry,
or {@code null} to accept every webhook
+ * request without verifying it
+ */
+ public TelegramWebhookProcessor(Processor next, String secretToken) {
this.next = AsyncProcessorConverterHelper.convert(next);
+ this.secretToken = secretToken != null ?
secretToken.getBytes(StandardCharsets.UTF_8) : null;
}
@Override
public boolean process(Exchange exchange, AsyncCallback callback) {
+ if (secretToken != null) {
+ String token = exchange.getIn().getHeader(SECRET_TOKEN_HEADER,
String.class);
+ if (token == null || !MessageDigest.isEqual(secretToken,
token.getBytes(StandardCharsets.UTF_8))) {
+ LOG.warn("Rejecting a Telegram webhook request with a missing
or invalid {} header."
+ + " A webhook registered outside Camel must use the
webhookSecretToken as its secret_token.",
+ SECRET_TOKEN_HEADER);
Review Comment:
Keeping `WARN` here, as davsclaus also suggests in his review:
- Right after an upgrade, a webhook registered with the previous version
(through the `WebhookRoutePolicy`, or outside Camel) has no secret token, so
every update is rejected until the webhook is registered again. This line is
how an operator learns that; at `DEBUG` the updates would be dropped silently.
- camel-whatsapp logs a rejected `X-Hub-Signature-256` signature at `WARN`
for the same reason (`WhatsAppWebhookProcessor`).
- Bounding the volume of requests from unthrottled clients is outside the
security model (_Out of scope_: DoS and resource exhaustion through unthrottled
routes; the operator bounds them).
- The message contains no request data, so a client cannot inject content
into the log.
_Claude Code on behalf of oscerd_
##########
components/camel-telegram/src/main/java/org/apache/camel/component/telegram/TelegramWebhookProcessor.java:
##########
@@ -26,21 +28,51 @@
import org.apache.camel.component.telegram.model.Update;
import org.apache.camel.support.AsyncProcessorConverterHelper;
import org.apache.camel.support.AsyncProcessorSupport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import static
org.apache.camel.component.telegram.util.TelegramMessageHelper.populateExchange;
public class TelegramWebhookProcessor extends AsyncProcessorSupport {
+ /**
+ * The header in which Telegram sends the secret token the webhook was
registered with.
+ */
+ public static final String SECRET_TOKEN_HEADER =
"X-Telegram-Bot-Api-Secret-Token";
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TelegramWebhookProcessor.class);
private static final ObjectMapper MAPPER = new ObjectMapper();
private AsyncProcessor next;
+ private final byte[] secretToken;
- public TelegramWebhookProcessor(Processor next) {
+ /**
+ * @param next the processor that receives the updates
+ * @param secretToken the secret token that a webhook request must carry,
or {@code null} to accept every webhook
+ * request without verifying it
+ */
+ public TelegramWebhookProcessor(Processor next, String secretToken) {
this.next = AsyncProcessorConverterHelper.convert(next);
+ this.secretToken = secretToken != null ?
secretToken.getBytes(StandardCharsets.UTF_8) : null;
}
@Override
public boolean process(Exchange exchange, AsyncCallback callback) {
+ if (secretToken != null) {
+ String token = exchange.getIn().getHeader(SECRET_TOKEN_HEADER,
String.class);
+ if (token == null || !MessageDigest.isEqual(secretToken,
token.getBytes(StandardCharsets.UTF_8))) {
+ LOG.warn("Rejecting a Telegram webhook request with a missing
or invalid {} header."
Review Comment:
Keeping `WARN` here, as davsclaus also suggests in his review:
- Right after an upgrade, a webhook registered with the previous version
(through the `WebhookRoutePolicy`, or outside Camel) has no secret token, so
every update is rejected until the webhook is registered again. This line is
how an operator learns that; at `DEBUG` the updates would be dropped silently.
- camel-whatsapp logs a rejected `X-Hub-Signature-256` signature at `WARN`
for the same reason (`WhatsAppWebhookProcessor`).
- Bounding the volume of requests from unthrottled clients is outside the
security model (_Out of scope_: DoS and resource exhaustion through unthrottled
routes; the operator bounds them).
- The message contains no request data, so a client cannot inject content
into the log.
_Claude Code on behalf of oscerd_
##########
components/camel-telegram/src/main/java/org/apache/camel/component/telegram/TelegramEndpoint.java:
##########
@@ -122,16 +130,54 @@ public Consumer createConsumer(Processor processor)
throws Exception {
@Override
public Processor createWebhookHandler(Processor next) {
- return new TelegramWebhookProcessor(next);
+ if (!configuration.isWebhookSecretTokenEnabled()) {
+ LOG.warn("The webhook secret token is disabled
(webhookSecretTokenEnabled=false): webhook requests to {} are not"
+ + " verified",
+ this);
+ return new TelegramWebhookProcessor(next, null);
+ }
+ return new TelegramWebhookProcessor(next, resolveWebhookSecretToken());
}
@Override
public void registerWebhook() throws Exception {
- if
(!telegramService.setWebhook(webhookConfiguration.computeFullExternalUrl())) {
+ String url = webhookConfiguration.computeFullExternalUrl();
+ boolean registered = configuration.isWebhookSecretTokenEnabled()
+ ? telegramService.setWebhook(url, resolveWebhookSecretToken())
+ : telegramService.setWebhook(url);
+ if (!registered) {
throw new RuntimeCamelException("The Telegram API refused to
register a webhook");
}
}
+ /**
+ * Returns the secret token of the webhook: the configured one, or else
one derived from the authorization token,
+ * which stays the same across restarts and replicas.
+ */
+ String resolveWebhookSecretToken() {
+ String token = configuration.getWebhookSecretToken();
+ if (token == null) {
+ return
deriveWebhookSecretToken(configuration.getAuthorizationToken());
+ }
+ if (!WEBHOOK_SECRET_TOKEN_PATTERN.matcher(token).matches()) {
+ throw new IllegalArgumentException(
+ "The webhookSecretToken must be 1 to 256 characters long,
using only A-Z, a-z, 0-9, _ and -");
+ }
+ return token;
+ }
+
+ static String deriveWebhookSecretToken(String authorizationToken) {
Review Comment:
Added in 996dc869eb15, based on your suggestion: the authorization token
(the secret) is the key and the fixed label is the message, so the derived
token cannot be computed without the authorization token, and changes only when
it does.
_Claude Code on behalf of oscerd_
##########
components/camel-telegram/src/main/java/org/apache/camel/component/telegram/TelegramEndpoint.java:
##########
@@ -122,16 +130,54 @@ public Consumer createConsumer(Processor processor)
throws Exception {
@Override
public Processor createWebhookHandler(Processor next) {
- return new TelegramWebhookProcessor(next);
+ if (!configuration.isWebhookSecretTokenEnabled()) {
+ LOG.warn("The webhook secret token is disabled
(webhookSecretTokenEnabled=false): webhook requests to {} are not"
+ + " verified",
+ this);
+ return new TelegramWebhookProcessor(next, null);
+ }
+ return new TelegramWebhookProcessor(next, resolveWebhookSecretToken());
}
@Override
public void registerWebhook() throws Exception {
- if
(!telegramService.setWebhook(webhookConfiguration.computeFullExternalUrl())) {
+ String url = webhookConfiguration.computeFullExternalUrl();
+ boolean registered = configuration.isWebhookSecretTokenEnabled()
+ ? telegramService.setWebhook(url, resolveWebhookSecretToken())
+ : telegramService.setWebhook(url);
+ if (!registered) {
throw new RuntimeCamelException("The Telegram API refused to
register a webhook");
}
}
+ /**
+ * Returns the secret token of the webhook: the configured one, or else
one derived from the authorization token,
+ * which stays the same across restarts and replicas.
+ */
+ String resolveWebhookSecretToken() {
+ String token = configuration.getWebhookSecretToken();
+ if (token == null) {
+ return
deriveWebhookSecretToken(configuration.getAuthorizationToken());
+ }
+ if (!WEBHOOK_SECRET_TOKEN_PATTERN.matcher(token).matches()) {
+ throw new IllegalArgumentException(
+ "The webhookSecretToken must be 1 to 256 characters long,
using only A-Z, a-z, 0-9, _ and -");
+ }
+ return token;
+ }
+
+ static String deriveWebhookSecretToken(String authorizationToken) {
+ try {
+ Mac mac = Mac.getInstance("HmacSHA256");
+ mac.init(new
SecretKeySpec(authorizationToken.getBytes(StandardCharsets.UTF_8),
"HmacSHA256"));
+ byte[] digest =
mac.doFinal("camel-telegram-webhook-secret-token".getBytes(StandardCharsets.UTF_8));
Review Comment:
Added in 996dc869eb15, based on your suggestion: the authorization token
(the secret) is the key and the fixed label is the message, so the derived
token cannot be computed without the authorization token, and changes only when
it does.
_Claude Code on behalf of oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
