Skip to content

keepalived: use EVP API instead of deprecated MD5 - #1062

Open
ytwang0320 wants to merge 2 commits into
iqiyi:develfrom
ytwang0320:devel
Open

keepalived: use EVP API instead of deprecated MD5#1062
ytwang0320 wants to merge 2 commits into
iqiyi:develfrom
ytwang0320:devel

Conversation

@ytwang0320

Copy link
Copy Markdown
Collaborator

No description provided.

@ytwang0320 ytwang0320 added the pr/to-review-codes review codes line by line and check if problem exists. label Jan 21, 2026
@ytwang0320
ytwang0320 requested a review from wenjiejiang March 13, 2026 05:54
@pubyun

pubyun commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

本 PR 标题是 keepalived MD5→EVP,但 NAT64/NAT46 快转路径这个改动和本PR无关,建议拆分单独的PR

@pubyun

pubyun commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

用codex评审,供参考:

🔴 必须修

1. hmac_md5() 失败时静默返回,留下未定义/错误的 digest(VRRP-AH 认证安全相关)
vrrp_ipsecah.chmac_md5()void,EVP 分配/init 失败时直接 return;不写 digest,
调用方会拿未初始化的缓冲区去做 VRRP-AH 认证。更糟的是:内层 MD5 已写入 digest 后,若外层 pass 的
EVP_MD_CTX_new()/EVP_DigestInit_ex() 失败再 return,digest 会停在"仅内层"的错误值上,却被当作
HMAC 已完成。原 MD5_* 路径不会失败,这是新引入的隐患。
建议:把 digest 在失败时清零,或将函数改为返回成功/失败让上层能拒绝陈旧输出。

2. check_http.c 未检查 EVP_MD_CTX_new() 返回值
http_response_thread():

req->context = EVP_MD_CTX_new();
EVP_DigestInit_ex(req->context, EVP_md5(), NULL);   // req->context 可能为 NULL
分配失败时 EVP_DigestInit_ex(NULL, ...)/后续 EVP_DigestUpdate(NULL, ...) 会解引用 NULL 导致
健康检查进程崩溃vrrp_ipsecah.c 里已经做了判空,这里应保持一致:判空 + 检查 EVP_DigestInit_ex() == 1,
失败则记录日志/置检查失败并释放上下文。

🟡 建议修

3. tk[EVP_MD_size(EVP_md5())] 引入了变长数组(VLA)
vrrp_ipsecah.c:
unsigned char tk[EVP_MD_size(EVP_md5())];   // 运行时表达式 → VLA
原来是固定的 tk[MD5_DIGEST_LENGTH]。VLA 是退步,且在 -Wvla -Werror 下会编译失败建议改回 MD5_DIGEST_LENGTH(该宏未被弃用)、16, EVP_MAX_MD_SIZE4. EVP_DigestUpdate() / EVP_DigestFinal_ex() 返回值未检查
check_http.ccheck_ssl.cvrrp_ipsecah.c 多处未检查这两个调用的返回值
(vrrp_ipsecah.c 仅检查了 init)。失败时会静默比对部分/未初始化的摘要,或发出错误的认证值建议统一检查5. OpenSSL 版本基线提高,请确认/注明
EVP_MD_CTX_new()/EVP_MD_CTX_free() 需要 OpenSSL1.1.0 vendored keepalived 仍需支持更老的
OpenSSL,请更新 configure/兼容代码,或在文档中注明新的最低版本要求

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/to-review-codes review codes line by line and check if problem exists.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants