Skip to content

[ZEPPELIN-6266] Fix JWT expiration validation security vulnerability#5007

Merged
jongyoul merged 3 commits intoapache:masterfrom
chadongmin:ZEPPELIN-6266
Aug 5, 2025
Merged

[ZEPPELIN-6266] Fix JWT expiration validation security vulnerability#5007
jongyoul merged 3 commits intoapache:masterfrom
chadongmin:ZEPPELIN-6266

Conversation

@chadongmin
Copy link
Copy Markdown
Contributor

@chadongmin chadongmin commented Aug 2, 2025

What is this PR for?

This PR fixes a critical security vulnerability in JWT token validation where tokens without expiration time were incorrectly accepted as valid, potentially allowing indefinite unauthorized access. The
fix ensures all JWT tokens must have a valid expiration time and adds comprehensive unit tests to prevent regression.

What type of PR is it?

Bug Fix

Todos

  • - Fix JWT expiration validation logic to reject null expiration tokens
  • - Add security warning logs for rejected tokens
  • - Create comprehensive unit tests for JWT validation scenarios
  • - Verify backwards compatibility with existing valid tokens

What is the Jira issue?

  • ZEPPELIN-6266 Fix JWT expiration validation security vulnerability

How should this be tested?

  • Automated Unit Tests Added:

    • New test file: KnoxJwtRealmTest.java with 3 comprehensive test scenarios
    • Run: mvn test -Dtest=KnoxJwtRealmTest -pl zeppelin-server
    • All tests pass: Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
  • Manual Testing Steps:

    1. Create JWT token without expiration time → Should be rejected with security warning
    2. Create JWT token with valid future expiration → Should be accepted
    3. Create JWT token with past expiration → Should be rejected
    4. Check server logs for security warnings: "JWT token has no expiration time - rejecting token for security"
  • Security Validation:

    • Verify that tokens without expiration are properly rejected
    • Confirm existing valid tokens continue to work
    • Check security event logging

Screenshots (if appropriate)

N/A - Security fix with no UI changes

Questions:

  • Does the license files need to update? No - only modified existing files and added test files with standard Apache license headers
  • Is there breaking changes for older versions? No - fully backwards compatible. Only affects tokens with missing expiration (which should not exist in proper JWT implementations)
  • Does this needs documentation? No - internal security fix that doesn't change public APIs or configuration

JWT tokens without expiration time were incorrectly accepted as valid,
creating a security vulnerability. This change ensures that JWT tokens
must have a valid expiration time to be accepted.
Copy link
Copy Markdown
Member

@ParkGyeongTae ParkGyeongTae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

Copy link
Copy Markdown
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just a few minor things.

@Reamer Reamer self-assigned this Aug 4, 2025
- Remove string concatenation
- Insert a new line
@chadongmin
Copy link
Copy Markdown
Contributor Author

Looks good to me. Just a few minor things.

I have applied the changes you suggested. I really appreciate you taking the time to review it so carefully.

Copy link
Copy Markdown
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chadongmin
Copy link
Copy Markdown
Contributor Author

@Reamer I was wondering what the merge schedule for this PR after it's approved.

Copy link
Copy Markdown
Member

@jongyoul jongyoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jongyoul jongyoul merged commit 4bf5ab9 into apache:master Aug 5, 2025
14 of 18 checks passed
jongyoul pushed a commit that referenced this pull request Aug 5, 2025
### What is this PR for?
  This PR fixes a critical security vulnerability in JWT token validation where tokens without expiration time were incorrectly accepted as valid, potentially allowing indefinite unauthorized access. The
  fix ensures all JWT tokens must have a valid expiration time and adds comprehensive unit tests to prevent regression.

  ### What type of PR is it?
  Bug Fix

  ### Todos
  * [x] - Fix JWT expiration validation logic to reject null expiration tokens
  * [x] - Add security warning logs for rejected tokens
  * [x] - Create comprehensive unit tests for JWT validation scenarios
  * [x] - Verify backwards compatibility with existing valid tokens

  ### What is the Jira issue?
  * [ZEPPELIN-6266](https://issues.apache.org/jira/browse/ZEPPELIN-6266) Fix JWT expiration validation security vulnerability

  ### How should this be tested?
  * **Automated Unit Tests Added**:
    - New test file: `KnoxJwtRealmTest.java` with 3 comprehensive test scenarios
    - Run: `mvn test -Dtest=KnoxJwtRealmTest -pl zeppelin-server`
    - All tests pass: `Tests run: 3, Failures: 0, Errors: 0, Skipped: 0`

  * **Manual Testing Steps**:
    1. Create JWT token without expiration time → Should be rejected with security warning
    2. Create JWT token with valid future expiration → Should be accepted
    3. Create JWT token with past expiration → Should be rejected
    4. Check server logs for security warnings: "JWT token has no expiration time - rejecting token for security"

  * **Security Validation**:
    - Verify that tokens without expiration are properly rejected
    - Confirm existing valid tokens continue to work
    - Check security event logging

  ### Screenshots (if appropriate)
  N/A - Security fix with no UI changes

  ### Questions:
  * **Does the license files need to update?** No - only modified existing files and added test files with standard Apache license headers
  * **Is there breaking changes for older versions?** No - fully backwards compatible. Only affects tokens with missing expiration (which should not exist in proper JWT implementations)
  * **Does this needs documentation?** No - internal security fix that doesn't change public APIs or configuration

Closes #5007 from chadongmin/ZEPPELIN-6266.

Signed-off-by: Jongyoul Lee <[email protected]>
(cherry picked from commit 4bf5ab9)
Signed-off-by: Jongyoul Lee <[email protected]>
@jongyoul jongyoul changed the title [ZEPPELIN-6626] Fix JWT expiration validation security vulnerability [ZEPPELIN-6266] Fix JWT expiration validation security vulnerability Aug 5, 2025
@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Aug 5, 2025

I changed the title to ZEPPELIN-6266

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants