Skip to content

Conversation

@oncilla
Copy link
Contributor

@oncilla oncilla commented Nov 6, 2025

Only for cp-root, cp-ca, and cp-as certificates the ISD-AS is mandatory.

@oncilla oncilla requested a review from Copilot November 6, 2025 15:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies the certificate creation logic to make ISD-AS optional for voting certificates while keeping it mandatory for cp-root, cp-ca, and cp-as certificates.

Key Changes

  • Added requireIA parameter to certificate subject creation functions to conditionally enforce ISD-AS presence
  • Updated certificate creation to only require ISD-AS for Sensitive and Regular certificate types
  • Enhanced test coverage to verify both ISD-AS required and optional scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
scion-pki/certs/create.go Modified createSubject and loadSubject to accept requireIA parameter; updated certificate creation to conditionally require ISD-AS based on certificate type
scion-pki/certs/renew.go Updated subjectFromVars to accept requireIA parameter and conditionally validate ISD-AS; passed true for renewal operations
scion-pki/certs/renew_test.go Added RequireIA field to test cases and new test case for validating certificates without ISD-AS
Comments suppressed due to low confidence (1)

scion-pki/certs/renew.go:940

  • When requireIA is false and vars.IA.IsZero() is true, the code still adds the ISD-AS to ExtraNames with an empty string value from vars.IA.String(). This will result in an invalid certificate attribute. The ExtraNames entry should only be added when vars.IA is not zero.
func subjectFromVars(vars SubjectVars, requireIA bool) (pkix.Name, error) {
	if requireIA && vars.IA.IsZero() {
		return pkix.Name{}, serrors.New("isd_as required in template")
	}
	s := pkix.Name{
		CommonName:   vars.CommonName,
		SerialNumber: vars.SerialNumber,
		ExtraNames: []pkix.AttributeTypeAndValue{
			{
				Type:  cppki.OIDNameIA,
				Value: vars.IA.String(),
			},
		},

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Only for cp-root, cp-ca, and cp-as certificates the ISD-AS is mandatory.
@oncilla oncilla force-pushed the scion-pki-support-voter-without-IA branch from b0a1a1d to b62c80c Compare November 6, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant