Support dynamic addition of client CAs

pdumais commented 1 year ago

If I understand correctly, by adding CAs in fc-restconf.web.tls.ca, a client will have its cert validated against any of the CAs according to https://github.com/freeconf/restconf/blob/141e395ecaf03235b0f16933ba8b8bc273f21c32/stock/tls.go#L31

Does this mean that if a new CA needs to be added, then the server needs to be restarted? it would be nice if we could set this to RequireAnyClientCert or RequestClientCert and then be able to manually validate the cert using a Filter. This way, the application using restconf would be able to handle requests and validate the certs using a dynamic list that is handled outside of restconf

Or any other suggestions to achieve the same goal?

dhubler commented 1 year ago

i can see how this would get in the way of custom cert validation so maybe we allow explicitly setting that in config by adding that to the tls yang and tls Node implementation.

some thoughts:

adding a CA should work w/o restart then it's a bug
if a user doesn't provide custom cert validation, seems like safest default is as it is now, VerifyClientCertIfGiven, so that at least the cert is validated w/the CAs

On Fri, Nov 11, 2022 at 8:09 PM Patrick Dumais @.***> wrote:

If I understand correctly, by adding CAs in fc-restconf.web.tls.ca, a client will have its cert validated against any of the CAs according to https://github.com/freeconf/restconf/blob/141e395ecaf03235b0f16933ba8b8bc273f21c32/stock/tls.go#L31

Does this mean that if a new CA needs to be added, then the server needs to be restarted? it would be nice if we could set this to RequireAnyClientCert or RequestClientCert and then be able to manually validate the cert using a Filter. This way, the application using restconf would be able to handle requests and validate the certs using a dynamic list that is handled outside of restconf

Or any other suggestions to achieve the same goal?

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7V2KTWO3U2U6QC272DWH3U57ANCNFSM6AAAAAAR6B4CKA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

pdumais commented 1 year ago

If adding certs in the ca bag works without reload, then that's ok the way it is. You are confirming that this should be the case? I'll try to make it work, I was just not able to figure out how to update it during runtime yet.

pdumais commented 1 year ago

Would you be able to provide an example on how to add multiple CAs for client auth?

I see I can provide 1 ca only using web.tls.ca.certfile in ApplyStartupConfig.

Should I be directly adding them in http.server.tls.Config.ClientCAs? If yes, would it create conflict with https://github.com/freeconf/restconf/blob/141e395ecaf03235b0f16933ba8b8bc273f21c32/stock/tls.go#L30 ?

dhubler commented 1 year ago

ok, yeah, i don't see a way currently. Are there any RFC on how to manage cert trust stores we can implement?

On Sat, Nov 12, 2022 at 4:46 PM Patrick Dumais @.***> wrote:

Would you be able to provide an example on how to add multiple CAs for client auth?

I see I can provide 1 ca only using web.tls.ca.certfile in ApplyStartupConfig.

Should I be directly adding them in http.server.tls.Config.ClientCAs? If yes, would create conflict with

https://github.com/freeconf/restconf/blob/141e395ecaf03235b0f16933ba8b8bc273f21c32/stock/tls.go#L31 ?

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11#issuecomment-1312579058, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7W7IZAXK4W3JCKAAFDWIAFZRANCNFSM6AAAAAAR6B4CKA . You are receiving this because you commented.Message ID: @.***>

pdumais commented 1 year ago

No RFC that I know of. I think it would be good to have a way to allow users to implement their own strategy outside of freeconf. So freeconf would support all 4 options of ClientAuth and allow to add certs in the certpool. Other users, like myself, may want to skip the certpool and use a filter instead.

What was the intent behind the ca in the yang for tls?

dhubler commented 1 year ago

we'll find a way, just trying to find a way that doesn't introduce issues. FreeCONF let's you implement your own strategy, this is just somewhat unique where your clients have a variety of CAs as I understand it. The single CA assumes client and server certs are signed by same CA. Then this works.

I found these proposed, active drafts https://datatracker.ietf.org/doc/draft-ietf-netconf-trust-anchors/ https://datatracker.ietf.org/doc/draft-ietf-netconf-tls-client-server/

Ideally we implement them, but at a minimum we should take inspiration from them.

On Sat, Nov 12, 2022 at 10:22 PM Patrick Dumais @.***> wrote:

No RFC that I know of. I think it would be good to have a way to allow users to implement their own strategy outside of freeconf. So freeconf would support all 4 options of ClientAuth and allow to add certs in the certpool. Other users, like myself, may want to skip the certpool and use a filter instead.

What was the intent behind the ca in the yang for tls?

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11#issuecomment-1312629112, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7RW2Q6RHZT37Z6D7IDWIBNFTANCNFSM6AAAAAAR6B4CKA . You are receiving this because you commented.Message ID: @.***>

pdumais commented 1 year ago

That's correct. in our case, each client could have a different CA since we are working in a multi-tenant environment.

If I understand correctly, implementing those RFCs would add a truststore inside freeconf. I think that's a great feature, but I was initially proposing something simpler where freeconf would completely disregard the client certificates and offload the logic to the users of the library. The only thing preventing a user from using freeconf this way right now is that freeconf will not request a client certificate unless the fc-restconf.web.tls.ca.certFile is set. Freeconf only sets http.server.tls.config.ClientAuth if a certfile is provided and the auth is set to VerifyClientCertIfGiven.

I'm wondering if the simplest fix of all would be for freeconf to set http.server.tls.config.ClientAuth=RequestClientCert by default. With this setting as a default:

a client cert would be requested but nothing would happen if a cert is not presented. So this is backwards compatible
If a client cert is sent by the client, freeconf won't do anything about it, but the user could decide to implement a Filter to retrieve the client cert and implement his own strategy.
if the user sets fc-restconf.web.tls.ca.certFile, then the behaviour will be overriden to VerifyClientCertIfGiven and certificates will be verified automatically. This would prevent the users from implementing their own strategy but that's fine since the user has made the conscious decision of adding a cert himself. This could later be improved to support multiple certificates. But this is not necessarily what I'm requesting from this ticket since I don't mind having to validate the certs myself.

My current PR allows a user to manually set ClientAuth, this adds more flexibility but also adds risks for users to shoot themselves in the foot. So maybe just setting http.server.tls.config.ClientAuth=RequestClientCert as the default would be the least invasive solution.

Does that make sense? I'm still reading through your code so it's possible I might have missed important aspects that makes my proposal impossible to implement.

dhubler commented 1 year ago

I do understand your use case. The fc-restconf.web.tls.ca.certFile is used for server cert CA as well otherwise your proposal would work fine.

Let me investigate exposing the setting from a Go API code level, (i.e. not YANG) and add copious comments that you MUST validate client certs CA elsewhere like Filters and see what that looks like. This should only take couple hours.

On Sun, Nov 13, 2022 at 8:17 AM Patrick Dumais @.***> wrote:

That's correct. in our case, each client could have a different CA since we are working in a multi-tenant environment.

If I understand correctly, implementing those RFCs would add a truststore inside freeconf. I think that's a great feature, but I was initially proposing something simpler where freeconf would completely disregard the client certificates and offload the logic to the users of the library. The only thing preventing a user from using freeconf this way right now is that freeconf will not request a client certificate unless the fc-restconf.web.tls.ca.certFile is set. Freeconf only sets http.server.tls.config.ClientAuth if a certfile is provided and the auth is set to VerifyClientCertIfGiven.

I'm wondering if the simplest fix of all would be for freeconf to set http.server.tls.config.ClientAuth=RequestClientCert by default. With this setting as a default:

a client cert would be requested but nothing would happen if a cert is not presented. So this is backwards compatible

If a client cert is sent by the client, freeconf won't do anything about it, but the user could decide to implement a Filter to retrieve the client cert and implement his own strategy.

if the user sets fc-restconf.web.tls.ca.certFile, then the behaviour will be overriden to VerifyClientCertIfGiven and certificates will be verified automatically. This would prevent the users from implementing their own strategy but that's fine since the user has made the conscious decision of adding a cert himself. This could later be improved to support multiple certificates. But this is not necessarily what I'm requesting from this ticket since I don't mind having to validate the certs myself.

My current PR allows a user to manually set ClientAuth, this adds more flexibility but also adds risks for users to shoot themselves in the foot. So maybe just setting http.server.tls.config.ClientAuth=RequestClientCert as the default would be the least invasive solution.

Does that make sense? I'm still reading through your code so it's possible I might have missed important aspects that makes my proposal impossible to implement.

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11#issuecomment-1312728661, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7RBGP6BUWYBFO4YM53WIDS63ANCNFSM6AAAAAAR6B4CKA . You are receiving this because you commented.Message ID: @.***>

dhubler commented 1 year ago

It might already be accessible

mgmt := restconf.NewServer(d) // after you apply config, Web and Server should be created, then... mgmt.Web.Server.TLSConfig.ClientAuth = tls.RequestClientCert

On Sun, Nov 13, 2022 at 9:47 AM Douglas Hubler @.***> wrote:

I do understand your use case. The fc-restconf.web.tls.ca.certFile is used for server cert CA as well otherwise your proposal would work fine.

Let me investigate exposing the setting from a Go API code level, (i.e. not YANG) and add copious comments that you MUST validate client certs CA elsewhere like Filters and see what that looks like. This should only take couple hours.

On Sun, Nov 13, 2022 at 8:17 AM Patrick Dumais @.***> wrote:

That's correct. in our case, each client could have a different CA since we are working in a multi-tenant environment.

If I understand correctly, implementing those RFCs would add a truststore inside freeconf. I think that's a great feature, but I was initially proposing something simpler where freeconf would completely disregard the client certificates and offload the logic to the users of the library. The only thing preventing a user from using freeconf this way right now is that freeconf will not request a client certificate unless the fc-restconf.web.tls.ca.certFile is set. Freeconf only sets http.server.tls.config.ClientAuth if a certfile is provided and the auth is set to VerifyClientCertIfGiven.

I'm wondering if the simplest fix of all would be for freeconf to set http.server.tls.config.ClientAuth=RequestClientCert by default. With this setting as a default:

a client cert would be requested but nothing would happen if a cert is not presented. So this is backwards compatible

If a client cert is sent by the client, freeconf won't do anything about it, but the user could decide to implement a Filter to retrieve the client cert and implement his own strategy.

if the user sets fc-restconf.web.tls.ca.certFile, then the behaviour will be overriden to VerifyClientCertIfGiven and certificates will be verified automatically. This would prevent the users from implementing their own strategy but that's fine since the user has made the conscious decision of adding a cert himself. This could later be improved to support multiple certificates. But this is not necessarily what I'm requesting from this ticket since I don't mind having to validate the certs myself.

My current PR allows a user to manually set ClientAuth, this adds more flexibility but also adds risks for users to shoot themselves in the foot. So maybe just setting http.server.tls.config.ClientAuth=RequestClientCert as the default would be the least invasive solution.

Does that make sense? I'm still reading through your code so it's possible I might have missed important aspects that makes my proposal impossible to implement.

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11#issuecomment-1312728661, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7RBGP6BUWYBFO4YM53WIDS63ANCNFSM6AAAAAAR6B4CKA . You are receiving this because you commented.Message ID: @.***>

pdumais commented 1 year ago

Yes. This would work. As long as I dont set the root CA cert in the config, otherwise it will get overriden. This is what I'll use for now.

Do we close this issue? Or would it still be preferable to add "official" support for this?

dhubler commented 1 year ago

Let's close the issue. I see that if this comes up again and this solution doesn't suffice, we look to implementing one of these models.

On Sun, Nov 13, 2022 at 5:18 PM Patrick Dumais @.***> wrote:

Yes. This would work. As long as I dont set the root CA cert in the config, otherwise it will get overriden. This is what I'll use for now.

Do we close this issue? Or would it still be preferable to add "official" support for this?

— Reply to this email directly, view it on GitHub https://github.com/freeconf/restconf/issues/11#issuecomment-1312836838, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7RZGMZM6NQKXPXQQATWIFSMJANCNFSM6AAAAAAR6B4CKA . You are receiving this because you commented.Message ID: @.***>

freeconf / restconf

Support dynamic addition of client CAs #11