libclamav/crypto: Use openssl parsing in cl_ASN1_GetTimeT() - #1731
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 542cdf1fef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
Hello! |
Contributor
|
@sebastianas Hello! I'm sorry this is sitting! I'm overwhelmed with bug triage and patch release prep. I see you! |
Contributor
Author
|
On 2026-06-25 12:42:31 [-0700], Val S. wrote:
val-ms left a comment (Cisco-Talos/clamav#1731)
@sebastianas Hello!
I'm sorry this is sitting! I'm overwhelmed with bug triage and patch release prep. I see you!
Okay, no worries. Just wanted to make sure it is not stuck behind
something else.
Sebastian
|
Contributor
Author
|
On 2026-07-07 10:19:05 [-0700], Val S. wrote:
@val-ms commented on this pull request.
> - fmt = "%Y%m%d%H%M%S";
- if (str[5] == '0') {
- str[4] = '0';
- str[5] = '9';
- } else {
- str[5]--;
- }
- }
-
- if (!(fmt)) {
- free(t);
- return NULL;
- }
-
- if (!strptime(str, fmt, t)) {
+ if (ASN1_TIME_to_tm(timeobj, t) == 0) {
@sebastianas I agree that it would be best practice to list the 1.1.1 version which `find_package` will treat as a minimum unless you were to specify "EXACT". That will make a build error more explicit as being due to an unsupported openssl version rather htan just failing later in the build due to some compile error.
Okay. Could you check the other part where I asked if we could remove
that function entirely since it has no users and was once used so might
just remove it and be done with it?
Sebastian
|
Contributor
|
Yes! Sorry I missed it! I will investigate as soon as my meetings are done. |
Contributor
|
@sebastianas I don't see any issues with removing
We'll be bumping the SO major version for 1.6 anyways due to other backwards incompatible changes, but I will have to be sure to mention the API removal in 1.6.0 release notes when the time comes. |
Contributor
Author
|
On 2026-07-07 13:18:58 [-0700], Val S. wrote:
val-ms left a comment (Cisco-Talos/clamav#1731)
@sebastianas I don't see any issues with removing `cl_ASN1_GetTimeT` entirely:
- delete the implementation from libclamav/crypto.c
- delete the declaration/comment from libclamav/clamav.h
- delete the export from libclamav/libclamav.map
We'll be bumping the SO major version for 1.6 anyways due to other
backwards incompatible changes, but I will have to be sure to mention
the API removal in 1.6.0 release notes when the time comes.
Okay, will do.
This symbol is under CLAMAV_PRIVATE so bsaed on my understanding it is
not publicly exported but kept available witin clamav.
Sebastian
|
The last user has been removed in commit fa15aa9 ("bb11594 - allow for compilation against openssl 1.1.0"). This function is only exported under CLAMAV_PRIVATE to be used within clamav, not by external users. Removing the function will not cause an ABI break since it was not part of the officially exported ABI. As a side effect it will compile against OpenSSL 4.0 where the ASN1_TIME type has been made opaque. Remove cl_ASN1_GetTimeT(). Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
val-ms
approved these changes
Jul 31, 2026
val-ms
left a comment
val-ms
left a comment
Contributor
There was a problem hiding this comment.
Looks good! Thanks so much for your patience. I'll revisit the other PR soon.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ASN1_TIME type has been made opaque in OpenSSL 4.0
Replace the manual parsing of the string with ASN1_TIME_to_tm() which is available since openssl 1.1.1.
[ The last user has been removed in commit fa15aa9 ("bb11594 -
allow for compilation against openssl 1.1.0") so we might remove it
but it looks exported ]