-
Notifications
You must be signed in to change notification settings - Fork 5
Inculcating CRLs for revocation purposes. #41
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
Merged
Merged
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
f3c2273
Initial changes for incorporating CRLs
arooshap a052c70
Skip the files instead of quarantining
arooshap e6ae405
Fix formatting in data.go
arooshap 68ee4d5
Adding checks, making the functions a bit better, e.t.c
arooshap 6d23f73
Fixing indentation in data.go
arooshap 16595d7
Fixing indentation in data.go
arooshap 5ea5916
Update data.go
arooshap 24b9668
Update data.go
arooshap f430707
Update server.go
arooshap c377cad
Update server.go
arooshap 57aef5d
Update data.go
arooshap 2ad9c8a
Update crls.go
arooshap 3b1155c
Update crls.go
arooshap 2a4a4de
Fixing issue with crls.go function
arooshap 7b8e782
Some more unit tests, a few modifications to crls.go and server.go
arooshap 5ec3c6b
Adding some additional checks and improving crls
arooshap 6622a6b
Adding documentation for crls.go
arooshap 9fb3160
Changing the crl to take a pointer
arooshap File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,218 @@ | ||
| // crls.go - Go implementation of CRL (Certificate Revocation List) management for APS. | ||
| // Copyright (c) 2025 - Aroosha Pervaiz | ||
| // | ||
| // This file implements logic to load, parse, and periodically refresh CRLs to validate x509 certificates/ | ||
| // | ||
| // The core workflow is as follows: | ||
| // | ||
| // 1. CRL Loading: | ||
| // - The server reads all CRL files from configured directories. | ||
| // - File patterns (e.g. "*.crl", "*.[rR][0-9]") are configurable via the CRLGlobs option. | ||
| // - Each CRL is parsed using Go's crypto/x509 package, and revoked certificate serial numbers | ||
| // are extracted and stored in memory. | ||
| // | ||
| // 2. CRL Refreshing: | ||
| // - A background goroutine runs at a configurable interval (CRLInterval). | ||
| // - It reloads all CRLs from the configured directories and updates the in-memory map of revoked serials. | ||
| // - Optionally, the refresh can skip or quarantine corrupted CRL files based on the CRLQuarantine flag. | ||
| // | ||
| // 3. Manual Refresh via HTTP: | ||
| // - The `/refresh-crls` endpoint allows manual refresh of CRLs using an HTTP PUT request. | ||
| // - This endpoint triggers an immediate reload of all configured CRL directories. | ||
| // - It responds with either “CRLs refreshed successfully” or an error message. | ||
| // | ||
| // 4. Certificate Verification: | ||
| // - During client TLS handshake, the VerifyPeerCertificate callback checks | ||
| // whether the presented certificate’s serial number exists in the revoked list. | ||
| // - If the serial is found, the connection is rejected as “certificate revoked”. | ||
| // | ||
| // 5. Testing: | ||
| // - The CRL logic includes unit tests(in crls_test.go) that verify correct handling of valid and invalid CRLs, | ||
| // the periodic refresher, and the manual HTTP refresh endpoint. | ||
|
|
||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "crypto/x509" | ||
| "encoding/pem" | ||
| "log" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "sync/atomic" | ||
| "time" | ||
| "net/http" | ||
| ) | ||
|
|
||
| // revoked cert serials (swap with atomic.Value) | ||
| var revoked atomic.Value | ||
|
|
||
| // collectCRLFiles builds the file list once (O(n)) across dirs and patterns. | ||
| func collectCRLFiles(dirs, globs []string) []string { | ||
| var files []string | ||
| patts := globs | ||
| if len(patts) == 0 { | ||
| // default patterns if none provided | ||
| patts = []string{"*.[rR][0-9]", "*.crl"} | ||
| } | ||
| for _, dir := range dirs { | ||
| for _, pat := range patts { | ||
| m, _ := filepath.Glob(filepath.Join(dir, pat)) | ||
| if len(m) > 0 { | ||
| files = append(files, m...) | ||
| } | ||
| } | ||
| } | ||
| return files | ||
| } | ||
|
|
||
| // quarantineFile renames a bad CRL to .bad; if rename fails, remove it. | ||
| func quarantineFile(path string) { | ||
| bad := path + ".bad" | ||
| if err := os.Rename(path, bad); err != nil { | ||
| log.Printf("failed to quarantine %s: %v; removing", path, err) | ||
| if rmErr := os.Remove(path); rmErr != nil { | ||
| log.Printf("failed to remove %s: %v", path, rmErr) | ||
| } | ||
| } else { | ||
| log.Printf("quarantined bad CRL %s -> %s", path, bad) | ||
| } | ||
| } | ||
|
|
||
| // parseCRLBytes decodes DER/PEM, validates freshness, and accumulates serials. | ||
| func parseCRLBytes(data []byte, path string, out map[string]bool, now time.Time) error { | ||
| // accept PEM-wrapped or raw DER | ||
| if strings.Contains(string(data), "-----BEGIN") { | ||
| for { | ||
| block, rest := pem.Decode(data) | ||
| if block == nil { | ||
| break | ||
| } | ||
| data = rest | ||
| if !strings.Contains(block.Type, "CRL") { | ||
| continue | ||
| } | ||
| crl, err := x509.ParseCRL(block.Bytes) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if crl.HasExpired(now) { | ||
| log.Printf("WARNING expired CRL %s (ignored)", path) | ||
| continue | ||
| } | ||
| for _, rc := range crl.TBSCertList.RevokedCertificates { | ||
| out[rc.SerialNumber.String()] = true | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // raw DER | ||
| crl, err := x509.ParseCRL(data) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if crl.HasExpired(now) { | ||
| log.Printf("WARNING expired CRL %s (ignored)", path) | ||
| return nil | ||
| } | ||
| for _, rc := range crl.TBSCertList.RevokedCertificates { | ||
| out[rc.SerialNumber.String()] = true | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // loadLocalCRLs builds revoked set from dirs+globs; skips or quarantines bad files per config. | ||
| func loadLocalCRLs(dirs, globs []string, quarantine bool) map[string]bool { | ||
| out := make(map[string]bool) | ||
| now := time.Now() | ||
|
|
||
| files := collectCRLFiles(dirs, globs) | ||
| if len(files) == 0 && Config.Verbose > 0 { | ||
| log.Printf("No CRL files found in %v with patterns %v", dirs, globs) | ||
| } | ||
|
|
||
| for _, f := range files { | ||
| data, err := os.ReadFile(f) | ||
| if err != nil { | ||
| log.Printf("Failed to read CRL %s: %v", f, err) | ||
| continue | ||
| } | ||
| if err := parseCRLBytes(data, f, out, now); err != nil { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once you change |
||
| log.Printf("Failed to parse CRL %s: %v", f, err) | ||
| if quarantine { | ||
| quarantineFile(f) | ||
| } | ||
| continue | ||
| } | ||
| if Config.Verbose > 0 { | ||
| log.Printf("Loaded CRL %s", f) | ||
| } | ||
| } | ||
| log.Printf("[CRL-DEBUG] TOTAL revoked certs loaded: %d", len(out)) | ||
| return out | ||
| } | ||
|
|
||
| // startCRLRefresher periodically reloads CRLs and swaps the map atomically. | ||
| func startCRLRefresher(dirs, globs []string, interval time.Duration, quarantine bool) { | ||
| go func() { | ||
| for { | ||
| m := loadLocalCRLs(dirs, globs, quarantine) | ||
| revoked.Store(m) | ||
| if Config.Verbose > 0 { | ||
| log.Printf("CRLs refreshed, %d revoked certs loaded", len(m)) | ||
| } | ||
| time.Sleep(interval) | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| // RefreshCRLsHandler handles PUT /refresh-crls to reload CRLs manually. | ||
| // It updates the revoked certificates map used for TLS verification. | ||
| func RefreshCRLsHandler(w http.ResponseWriter, r *http.Request) { | ||
| if r.Method != http.MethodPut { | ||
| w.Header().Set("Allow", http.MethodPut) | ||
| http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) | ||
| return | ||
| } | ||
|
|
||
| if err := refreshCRLsNow(Config.CRLDirs, Config.CRLGlobs, Config.CRLQuarantine); err != nil { | ||
| log.Printf("[CRL] manual refresh failed: %v", err) | ||
| http.Error(w, fmt.Sprintf("Failed to refresh CRLs: %v\n", err), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| log.Printf("[CRL] manual refresh successful") | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("CRLs refreshed successfully\n")) | ||
| } | ||
|
|
||
| // refreshCRLsNow forces an immediate reload. | ||
| func refreshCRLsNow(dirs, globs []string, quarantine bool) error { | ||
| m := loadLocalCRLs(dirs, globs, quarantine) | ||
| if len(m) == 0 { | ||
| return fmt.Errorf("no CRLs loaded from %v", dirs) | ||
| } | ||
| revoked.Store(m) | ||
| log.Printf("[CRL] Refreshed %d revoked certs", len(m)) | ||
| return nil | ||
| } | ||
|
|
||
| // verifyPeerCertificate enforces CRL-based revocation for the leaf cert. | ||
| func verifyPeerCertificateWithCRL(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { | ||
| if len(verifiedChains) == 0 || len(verifiedChains[0]) == 0 { | ||
| return fmt.Errorf("no verified chains") | ||
| } | ||
| leaf := verifiedChains[0][0] | ||
| crls, _ := revoked.Load().(map[string]bool) | ||
| if crls == nil { | ||
| return nil // no CRLs loaded yet -> do not block | ||
| } | ||
| if _, ok := crls[leaf.SerialNumber.String()]; ok { | ||
| return fmt.Errorf("certificate revoked: %s", leaf.SerialNumber) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should use pointer to
outmap which will be passed to it fromloadLocalCRLs. 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
cmsRecordsin 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 putmutexlocks 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.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.