Fix Y2038 timestamp overflow in sonic-pac authentication manager#19
Open
devin-ai-integration[bot] wants to merge 3 commits intomasterfrom
Open
Fix Y2038 timestamp overflow in sonic-pac authentication manager#19devin-ai-integration[bot] wants to merge 3 commits intomasterfrom
devin-ai-integration[bot] wants to merge 3 commits intomasterfrom
Conversation
- Change sessionTime from uint32 to uint64 in authmgrClientInfo_t - Change lastAuthTime from uint32 to uint64 in authmgrClientInfo_t - Change sessionTime from uint32 to uint64 in authmgrPortSessionStats_t - Change sessionTime from uint32 to uint64 in pac_authenticated_clients_oper_table_t - Change lastAuthTime from uint32 to uint64 in pac_authenticated_clients_oper_table_t These changes prevent timestamp overflow that would occur on January 19, 2038 when 32-bit signed timestamps exceed INT32_MAX. Co-Authored-By: Arthur Poon <arthur.poon@windsurf.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Add tests to verify 64-bit timestamp handling: - Test uint64 type is correctly sized (8 bytes) - Test sessionTime can hold values beyond Y2038 - Test lastAuthTime can hold values beyond Y2038 - Test timestamp arithmetic doesn't overflow at Y2038 boundary - Test timestamp comparison works for large values - Test session duration calculation across Y2038 boundary - Test authentication timeout calculation - Test struct field sizes are correct - Test struct operations with Y2038 values These tests ensure the Y2038 fix works correctly for timestamps that would have overflowed with the previous 32-bit implementation. Co-Authored-By: Arthur Poon <arthur.poon@windsurf.com>
Add detection signature functions: - y2038_overflow_imminent(): Returns true if timestamp is within 1 year of Y2038 - y2038_overflow_occurred(): Returns true if timestamp has passed Y2038 - Y2038_TIMESTAMP_CHECK macro: Compile-time check for 64-bit timestamp types Add tests for detection signatures: - Test y2038_overflow_imminent for various timestamp ranges - Test y2038_overflow_occurred for various timestamp ranges - Test compile-time check macro - Test detection with PAC-specific authmgrClientInfo_t types Co-Authored-By: Arthur Poon <arthur.poon@windsurf.com>
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
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.
Why I did it
This PR addresses the Y2038 timestamp overflow problem in the sonic-pac authentication manager. On January 19, 2038, 32-bit signed Unix timestamps will overflow, causing timestamp comparisons and calculations to fail. The affected fields (
sessionTimeandlastAuthTime) store authentication session timestamps that could be affected by this overflow.This is part of a coordinated Y2038 fix effort across multiple SONiC submodules (sonic-stp, sonic-sairedis, sonic-pac).
Work item tracking
How I did it
Changed timestamp fields from
uint32touint64in three header files:authmgrClientInfo_t.sessionTimeandauthmgrClientInfo_t.lastAuthTimeinauth_mgr_db.hauthmgrPortSessionStats_t.sessionTimeinauth_mgr_exports.hpac_authenticated_clients_oper_table_t.sessionTimeandpac_authenticated_clients_oper_table_t.lastAuthTimeinpacoper_common.hUpdates since last revision:
src/sonic-pac/tests/test_y2038_timestamp.ccovering:y2038_overflow_imminent(): Returns true if timestamp is within 1 year of Y2038 overflowy2038_overflow_occurred(): Returns true if timestamp has passed the Y2038 overflow pointY2038_TIMESTAMP_CHECKmacro: Compile-time check that timestamp type is at least 64-bitauthmgrClientInfo_ttypesHow to verify it
gcc -o test_y2038 src/sonic-pac/tests/test_y2038_timestamp.c && ./test_y2038Important review items:
uint64typedef is properly defined indatatypes.hauth_mgr_control.clines 1722-1730 for overflow handling logic that may need updating (uses0xffffffffconstant)auth_mgr_debug.c(lines 1340, 1342) are updated for 64-bit valuesWhich release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Fix Y2038 timestamp overflow in sonic-pac by changing sessionTime and lastAuthTime fields from uint32 to uint64
Link to config_db schema for YANG module changes
N/A - no YANG model changes
A picture of a cute animal (not mandatory but encouraged)
Additional context