gnodet-bot commented on code in PR #27716:
URL: https://github.com/apache/camel/pull/27716#discussion_r4250856276
##########
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:
⚠️ **Security / Log flooding:** `LOG.warn` on every rejected request creates
a denial-of-service vector: a client that sends a flood of invalid requests
fills the log at WARN rate, potentially filling disk or overwhelming log
shipping. Rejected-token events are not operator-actionable and are better
noise for an operator than a quiet `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);
```
##########
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:
💡 **Code clarity:** The roles of the HMAC key and message are not obvious to
a reader. A brief comment explaining the design choice would help future
maintainers.
```suggestion
// authorizationToken is the HMAC key (secret); the fixed string
is the message,
// so that the derived token changes when the bot token changes
but is otherwise stable.
mac.init(new
SecretKeySpec(authorizationToken.getBytes(StandardCharsets.UTF_8),
"HmacSHA256"));
byte[] digest =
mac.doFinal("camel-telegram-webhook-secret-token".getBytes(StandardCharsets.UTF_8));
```
--
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]
