Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

port new security analyzers to net6 #5467

Merged

Conversation

jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Sep 10, 2021

port of #5420

TimHannMSFT and others added 9 commits September 10, 2021 10:58
…sues

Ensure key validation isn't explicitly disabled by setting the relevant setter on the validation object to false:
* RequireExpirationTime
* ValidateAudience
* ValidateIssuer
* ValidateLifetime

Ensure key validation isn't implicitly disabled by having the relevant delegate always return true:
* AudienceValidator
* LifetimeValidator

NOTE: There's another key implicit case for IssuerValidator, however the logic there is more complex and would require Data Flow Analysis (DFA). Since DFA has a different performance profile anyways, shipping these analyzers first which can be accomplished via static analysis. We also have CodeQL work in the pipe separately from this to address more complicated DFA flows.
* Fix nullref exception found during, added unit test
@jmarolf jmarolf requested a review from a team as a code owner September 10, 2021 22:55
@jmarolf jmarolf changed the base branch from main to release/6.0.1xx September 10, 2021 22:56
@codecov
Copy link

codecov bot commented Sep 10, 2021

Codecov Report

Merging #5467 (aa3844d) into release/6.0.1xx (e71e518) will decrease coverage by 0.01%.
The diff coverage is 95.52%.

@@                 Coverage Diff                 @@
##           release/6.0.1xx    #5467      +/-   ##
===================================================
- Coverage            95.54%   95.52%   -0.02%     
===================================================
  Files                 1269     1273       +4     
  Lines               289171   289661     +490     
  Branches             17453    17478      +25     
===================================================
+ Hits                276277   276705     +428     
- Misses               10559    10584      +25     
- Partials              2335     2372      +37     

Copy link
Contributor

@dotpaul dotpaul left a comment

Choose a reason for hiding this comment

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

LGTM

@jmarolf jmarolf merged commit 1f3ca18 into dotnet:release/6.0.1xx Sep 14, 2021
@jmarolf jmarolf deleted the port-new-security-analyzers-to-net6 branch September 14, 2021 15:58
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.

3 participants