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

Custom validation inside OnCertificateValidated getting ignored by Kestrel (3.0Preview9) #14033

Closed
brrusino opened this issue Sep 16, 2019 · 8 comments
Assignees
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer

Comments

@brrusino
Copy link

We're trying to only allow certificates associated with a specific root cert to access our API. When debugging with a cert from another CA that is valid, I can confirm that the validation check (VerifyChainAndRequiredRoot) is behaving as expected and failing (context.Fail is being set) but authentication is still passing.

services.AddAuthentication(
            CertificateAuthenticationDefaults.AuthenticationScheme)
            .AddCertificate(options =>
            {
               options.AllowedCertificateTypes = CertificateTypes.All;
               options.Events = new CertificateAuthenticationEvents
               {
                   OnCertificateValidated = context =>
                   {
                       var cert = context.ClientCertificate;
                       if (AMEValidationService.VerifyChainAndRequiredRoot(cert))
                       {
                           var claims = new[]
                                                   {
                            new Claim(ClaimTypes.NameIdentifier, context.ClientCertificate.Subject, ClaimValueTypes.String, context.Options.ClaimsIssuer),
                            new Claim(ClaimTypes.Name, context.ClientCertificate.Subject, ClaimValueTypes.String, context.Options.ClaimsIssuer)
                        };

                           context.Principal = new ClaimsPrincipal(new ClaimsIdentity(claims, context.Scheme.Name));
                           context.Success();
                       }
                       else
                       {
                           context.Fail("invalid cert");
                       }
                       return Task.CompletedTask;
                   }
               };
            });

Inside the configuration for Kestrel, my understanding is that CertificateValidation would need to be disabled here in order to not use the default configuration:

WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>()
                .ConfigureKestrel(options =>
                {
                    options.ConfigureHttpsDefaults(opt =>
                    {
                        opt.ClientCertificateMode = ClientCertificateMode.RequireCertificate;
                        opt.ClientCertificateValidation = (cert, chain, policyErrors) =>
                        {
                            return true;
                        };
                    });
                });

Any insight would be much appreciated.

@blowdart blowdart added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Sep 16, 2019
@HaoK
Copy link
Member

HaoK commented Sep 16, 2019

@Tratcher this looks to be an issue with the cert auth handler interacting with kestrel, there are tests that verify context.fail without kestrel in the picture result in proper failures: i.e https://github.com/aspnet/AspNetCore/blob/b75b892eac51be8b2f0eb9dc9b47537fc02001c9/src/Security/Authentication/test/CertificateTests.cs#L226

Is returning true the proper way to disable kestrel's client cert validation for this to work? Or do we need additional tweaking for this scenario to work?

@Tratcher
Copy link
Member

Kestrel doesn't participate in authentication/authorization beyond the ClientCertificateValidation check you disabled.

but authentication is still passing

This sounds more like an authorization problem. What authorization policies have you set up for the endpoint?

@blowdart
Copy link
Contributor

blowdart commented Sep 16, 2019

context.Fail("invalid cert") should have it end up with no user so authorize should be barfing? Unless it's falling through to the default constructed user dominick wanted?

@HaoK
Copy link
Member

HaoK commented Sep 16, 2019

Only if you require authenticated user

@brrusino
Copy link
Author

I haven't implemented the authorization middleware yet - my understanding was that if OnCertificateValidated() fails, any API access would be rejected (which is the intended/expected behavior).

@HaoK
Copy link
Member

HaoK commented Sep 16, 2019

If you don't do anything and are just using [Authorize] that just requires authenticated user, so it should block things if authentication fails

@blowdart
Copy link
Contributor

@brrusino it'll only fail if you have an authorize attribute on them, or if you've made authorization mandatory on every route via configuration.

@brrusino
Copy link
Author

@HaoK @blowdart Good to know, I didn't realize there was an implicit connection between the Authentication and Authorization middlewares. Seems to be working now!

@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Projects
None yet
Development

No branches or pull requests

4 participants