gnodet-bot commented on code in PR #27716:
URL: https://github.com/apache/camel/pull/27716#discussion_r4250562864
##########
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:
⚠️ **Security / Log flooding:** Logging at `WARN` on every rejected request
creates a denial-of-service vector — an attacker can flood the log with a
stream of invalid webhook calls at essentially no cost. This is a medium
severity concern for a publicly reachable webhook endpoint.
Consider downgrading to `DEBUG`:
```suggestion
LOG.debug("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);
```
If an operator-visible signal is still desired, a rate-limited log pattern
(e.g. one `WARN` per minute via a counter, dropping the rest to `DEBUG`) would
keep the noise bounded.
##########
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:
💡 **Code clarity:** It may not be immediately obvious to a reader why the
HMAC is initialised with the authorization token as the *key* and
`"camel-telegram-webhook-secret-token"` as the *message* (rather than the
reverse). A short inline comment explaining the role assignment — for example,
"key = the Bot API token (secret), message = a fixed domain separator" — would
help future maintainers understand the intentional design at a glance.
--
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]
