Inculcating CRLs for revocation purposes.

[arooshap]

This pull request adds support for Certificate Revocation List (CRL) management to the APS. It introduces functionality to load and parse CRL files from configurable directories, maintain an in-memory list of revoked certificate serials, and periodically refresh them at a defined interval. A new /refresh-crls HTTP endpoint using the PUT method allows on-demand manual refresh of CRLs without restarting the server.

[arooshap]

@vkuznet Could you please review these changes? Please let me know if anything needs to be changed. Thanks for your help and suggestions!

[vkuznet]

Even though the current implementation is correct in logic it has a flaw in usage of CRLs map which is recreated each time and not fully filled. I provided a few suggestions to improve the code, please chose one based on algorithm logic.

[vkuznet]

This function should use pointer to out map which will be passed to it from loadLocalCRLs. Currently the map you passed around is copied by value, i.e. it is not accummulated as it is always a new copy. Instead, you should use pointer to a map, which will points to a single map and you'll fill it out with CRL serial numbers.

Or, if you globally define map of CRLs to keep then you don't need to use local map definition. Look at cmsRecords in https://github.com/dmwm/auth-proxy-server/blob/master/cric/cric.go#L22 and how it is used through the code. If you will define global map you better put mutex locks around its read/write operations to avoid potential (and very dangerous but rare) racing conditions as maps are not safe concurrency objects. Moreover, you may define an entire struct which will keep your map, e.g.

type CRLMap struct {
   Map map[string[bool
   mutex sync.RWMutex
}
func (c *CRLMap) Update(entry string) {
   mutex.Lock()
   c.Map[entry] = true
   mutex.Unlock()
}
func (c *CRLMap) Get(entry string) bool {
   mutex.RLock()
   r, ok := cmsRecords[sortedDN]
   mutex.RUnlock()
   if ok { 
     return r
   }
   return false
}

With such implementation you map even add periodict refresh of the internal map if you want, i.e. define duration of validity of the map.

[vkuznet]

Once you change parseCRLBytes function to accept pointer to a map, then you should change here its call like this: parseCRLBytes(data, f, &out, now)

[arooshap]

@vkuznet Thanks alot for your suggestions. I have updated the code respectively. I can check out the functionality with struct later on.
What do you think?

[vkuznet]

@arooshap , thank you for the fix. I have no longer comments. Please perform final tests locally and then you can merge and test it on k8s cluster(s).

[arooshap]

Hi @vkuznet,

I ran some debugging to check the variables and the process workflows through Go and Delve. I also tested the authentication with my own user certificate and things look normal to me. I can start testing it in the dev clusters once this PR is merged and inform you as well, so you can also verify the behaviour.

Thank you for your suggestions and help!

[vkuznet]

ok, merging.

[vkuznet]

@arooshap , I apologize for my previous messages. Turns out it was code I was having locally and it was sitting in my build area. The build is successful once I remove my dev code. Sorry again for confusion.