Skip to content

Add AIA certificate prober to boulder-observer#8594

Open
Copilot wants to merge 13 commits intoobserver-ctxfrom
copilot/add-aia-certificate-prober
Open

Add AIA certificate prober to boulder-observer#8594
Copilot wants to merge 13 commits intoobserver-ctxfrom
copilot/add-aia-certificate-prober

Conversation

Copy link

Copilot AI commented Jan 23, 2026 •
edited by beautifulentropy
Loading

Add an AIA certificate prober to Boulder-observer, so we can verify the served certificates have the right Common Name (preventing mixups), content type and encoding. We export the certificate notBefore and notAfter for expiry monitoring purposes.

This PR was largely written by Copilot under my supervision, and is modelled after the CRL Prober.

Fixes #8593

Copilot AI changed the title [WIP] Add check for AIA certificates in boulder-observer Add AIA certificate prober to boulder-observer Jan 23, 2026
Copilot AI requested a review from mcpherrinm January 23, 2026 23:13
@mcpherrinm
Copy link
Contributor

mcpherrinm commented Jan 23, 2026

@copilot test failure " main_test.go:68: Failed to validate config file "observer.yml": Key: 'ObsConf.MonConfs[11].Kind' Error:Field validation for 'Kind' failed on the 'oneof' tag"

Copy link
Author

Copilot AI commented Jan 23, 2026

@copilot test failure " main_test.go:68: Failed to validate config file "observer.yml": Key: 'ObsConf.MonConfs[11].Kind' Error:Field validation for 'Kind' failed on the 'oneof' tag"

Fixed in e9291c5. Added "AIA" to the Kind validation tag in observer/mon_conf.go so it's now recognized as a valid prober kind.

@mcpherrinm
Copy link
Contributor

mcpherrinm commented Jan 26, 2026

@copilot Add a config field, ExpectCommonName, which the prober checks if the received certificate matches, to ensure the correct certificate was served. Check the recieved certificate is also a CA Cert

Copy link
Author

Copilot AI commented Jan 26, 2026

@copilot Add a config field, ExpectCommonName, which the prober checks if the received certificate matches, to ensure the correct certificate was served. Check the recieved certificate is also a CA Ce...

Implemented in 1cad83f. Added:

  • ExpectCommonName config field (optional) to verify the certificate's CN matches the expected value
  • CA certificate validation - the prober now checks that cert.IsCA is true
  • Tests for both new validations including matching/non-matching CN and non-CA certificates

@mcpherrinm
Copy link
Contributor

mcpherrinm commented Jan 26, 2026

@copilot Don't make ExpectCommonName optional. It must be provided and checked.

Add log messages to the different failure cases in probing for debugging why something is gone wrong

Copy link
Author

Copilot AI commented Jan 26, 2026

@copilot Don't make ExpectCommonName optional. It must be provided and checked.

Add log messages to the different failure cases in probing for debugging why something is gone wrong

Implemented in 7202d58:

  • ExpectCommonName is now required - validation fails if not provided
  • Added debug log messages for all failure cases:
    • HTTP request creation failure
    • Certificate fetch failure
    • Incorrect Content-Type header
    • Response body read failure
    • Certificate parsing failure
    • Non-CA certificate detection
    • CommonName mismatch

@mcpherrinm mcpherrinm marked this pull request as ready for review January 26, 2026 17:57
@mcpherrinm mcpherrinm requested a review from a team as a code owner January 26, 2026 17:57
aarongable
aarongable reviewed Jan 26, 2026
View reviewed changes
aarongable
aarongable reviewed Jan 26, 2026
View reviewed changes
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Also please update the PR description to be something a human would like to read in the git log.

@mcpherrinm mcpherrinm requested a review from aarongable February 2, 2026 23:26
Copilot AI and others added 5 commits February 3, 2026 17:04
@aarongable
Initial plan
83ebde6
@mcpherrinm @aarongable
Add AIA certificate prober implementation with tests
e53fe8c 
Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Add AIA prober documentation and configuration example
e134b9f 
Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Add integration tests for AIA prober Probe method
fa38926 
Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Fix timeout handling and nil pointer check in AIA prober
2c5e85a 
Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
Copilot AI and others added 7 commits February 3, 2026 17:04
@aarongable
Fix formatting in AIA prober
6f1d985
@mcpherrinm @aarongable
Add AIA to Kind validation tag in mon_conf.go
62a7fca 
Fixes test failure where AIA kind was not recognized in the oneof validation tag.

Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Add ExpectCommonName field and CA certificate validation
b9f6f83 
- Add optional ExpectCommonName config field to verify certificate CN
- Validate that received certificate is a CA certificate (IsCA=true)
- Update tests to use CA certificates and test new validations
- Update documentation with new config field and examples

Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Make ExpectCommonName required and add debug logging
75f575b 
- Make ExpectCommonName a required field with validation
- Add log messages for all failure cases in probe:
  - HTTP request creation failure
  - Certificate fetch failure
  - Incorrect Content-Type
  - Response body read failure
  - Certificate parsing failure
  - Non-CA certificate
  - CommonName mismatch
- Update tests to always include ExpectCommonName
- Update documentation to reflect ExpectCommonName is required

Co-authored-by: mcpherrinm <47425+mcpherrinm@users.noreply.github.com>
@mcpherrinm @aarongable
Inline calls to conf.Instrument() when possible
b5f96c2
@mcpherrinm @aarongable
Check the cNotBefore and cNotAfter values
9aa8911 
Use AssertMetricWithLabelEquals instead of a test registry
@mcpherrinm @aarongable
Drop logs
5eefcc5
beautifulentropy
beautifulentropy requested changes Feb 4, 2026
View reviewed changes
@beautifulentropy beautifulentropy requested a review from jsha February 4, 2026 17:20
@aarongable aarongable force-pushed the copilot/add-aia-certificate-prober branch from 9251c86 to b069f1c Compare February 4, 2026 17:54
@aarongable
Copy link
Contributor

aarongable commented Feb 4, 2026

Apologies for the force-push, I've updated this PR to be based on top of #8619 to take advantage of the simpler Prober interface and to allow the tests to assert that the correct errors are being returned.

@aarongable aarongable changed the base branch from main to observer-ctx February 4, 2026 17:55
@mcpherrinm
Copy link
Contributor

mcpherrinm commented Feb 4, 2026

nice! new prober interface looks great :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcpherrinm mcpherrinm mcpherrinm approved these changes

@aarongable aarongable Awaiting requested review from aarongable

@jsha jsha Awaiting requested review from jsha

@beautifulentropy beautifulentropy Awaiting requested review from beautifulentropy

Assignees

@mcpherrinm mcpherrinm

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

boulder-observer: Add a check for AIA certs

4 participants

@mcpherrinm @aarongable @beautifulentropy