Remove support for obsolete id-pe-acmeIdentifier OID (#5906)
Current metrics show that subscribers present certificates using the obsolete OID to identify their id-pe-acmeIdentifier extension about an order of magnitude less often than they present the correct OID. Remove support for the never-standardized OID.
This commit is contained in:
		
							parent
							
								
									8c28e49ab6
								
							
						
					
					
						commit
						4835709232
					
				|  | @ -25,14 +25,6 @@ const ( | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| var ( | var ( | ||||||
| 	// NOTE: unfortunately another document claimed the OID we were using in draft-ietf-acme-tls-alpn-01
 |  | ||||||
| 	// for their own extension and IANA chose to assign it early. Because of this we had to increment
 |  | ||||||
| 	// the id-pe-acmeIdentifier OID. Since there are in the wild implementations that use the original
 |  | ||||||
| 	// OID we still need to support it until everyone is switched over to the new one.
 |  | ||||||
| 	// As defined in https://tools.ietf.org/html/draft-ietf-acme-tls-alpn-01#section-5.1
 |  | ||||||
| 	// id-pe OID + 30 (acmeIdentifier) + 1 (v1)
 |  | ||||||
| 	IdPeAcmeIdentifierV1Obsolete = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 30, 1} |  | ||||||
| 
 |  | ||||||
| 	// As defined in https://tools.ietf.org/html/draft-ietf-acme-tls-alpn-04#section-5.1
 | 	// As defined in https://tools.ietf.org/html/draft-ietf-acme-tls-alpn-04#section-5.1
 | ||||||
| 	// id-pe OID + 31 (acmeIdentifier)
 | 	// id-pe OID + 31 (acmeIdentifier)
 | ||||||
| 	IdPeAcmeIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 31} | 	IdPeAcmeIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 31} | ||||||
|  | @ -212,12 +204,8 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, identi | ||||||
| 	// Verify key authorization in acmeValidation extension
 | 	// Verify key authorization in acmeValidation extension
 | ||||||
| 	h := sha256.Sum256([]byte(challenge.ProvidedKeyAuthorization)) | 	h := sha256.Sum256([]byte(challenge.ProvidedKeyAuthorization)) | ||||||
| 	for _, ext := range leafCert.Extensions { | 	for _, ext := range leafCert.Extensions { | ||||||
| 		if IdPeAcmeIdentifier.Equal(ext.Id) || IdPeAcmeIdentifierV1Obsolete.Equal(ext.Id) { | 		if IdPeAcmeIdentifier.Equal(ext.Id) { | ||||||
| 			if IdPeAcmeIdentifier.Equal(ext.Id) { | 			va.metrics.tlsALPNOIDCounter.WithLabelValues(IdPeAcmeIdentifier.String()).Inc() | ||||||
| 				va.metrics.tlsALPNOIDCounter.WithLabelValues(IdPeAcmeIdentifier.String()).Inc() |  | ||||||
| 			} else { |  | ||||||
| 				va.metrics.tlsALPNOIDCounter.WithLabelValues(IdPeAcmeIdentifierV1Obsolete.String()).Inc() |  | ||||||
| 			} |  | ||||||
| 			if !ext.Critical { | 			if !ext.Critical { | ||||||
| 				errText := fmt.Sprintf("Incorrect validation certificate for %s challenge. "+ | 				errText := fmt.Sprintf("Incorrect validation certificate for %s challenge. "+ | ||||||
| 					"acmeValidationV1 extension not critical", core.ChallengeTypeTLSALPN01) | 					"acmeValidationV1 extension not critical", core.ChallengeTypeTLSALPN01) | ||||||
|  |  | ||||||
|  | @ -398,18 +398,26 @@ func TestTLSALPN01Success(t *testing.T) { | ||||||
| 		t, va.metrics.tlsALPNOIDCounter, prometheus.Labels{"oid": IdPeAcmeIdentifier.String()}, 1) | 		t, va.metrics.tlsALPNOIDCounter, prometheus.Labels{"oid": IdPeAcmeIdentifier.String()}, 1) | ||||||
| 
 | 
 | ||||||
| 	hs.Close() | 	hs.Close() | ||||||
| 	chall = tlsalpnChallenge() | } | ||||||
| 	hs, err = tlsalpn01Srv(t, chall, IdPeAcmeIdentifierV1Obsolete, 0, "localhost") | 
 | ||||||
|  | func TestTLSALPN01ObsoleteFailure(t *testing.T) { | ||||||
|  | 	// NOTE: unfortunately another document claimed the OID we were using in
 | ||||||
|  | 	// draft-ietf-acme-tls-alpn-01 for their own extension and IANA chose to
 | ||||||
|  | 	// assign it early. Because of this we had to increment the
 | ||||||
|  | 	// id-pe-acmeIdentifier OID. We supported this obsolete OID for a long time,
 | ||||||
|  | 	// but no longer do so.
 | ||||||
|  | 	// As defined in https://tools.ietf.org/html/draft-ietf-acme-tls-alpn-01#section-5.1
 | ||||||
|  | 	// id-pe OID + 30 (acmeIdentifier) + 1 (v1)
 | ||||||
|  | 	IdPeAcmeIdentifierV1Obsolete := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 30, 1} | ||||||
|  | 
 | ||||||
|  | 	chall := tlsalpnChallenge() | ||||||
|  | 	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifierV1Obsolete, 0, "localhost") | ||||||
| 	test.AssertNotError(t, err, "Error creating test server") | 	test.AssertNotError(t, err, "Error creating test server") | ||||||
| 
 | 
 | ||||||
| 	va, _ = setup(hs, 0, "", nil) | 	va, _ := setup(hs, 0, "", nil) | ||||||
| 
 | 
 | ||||||
| 	_, prob = va.validateChallenge(ctx, dnsi("localhost"), chall) | 	_, prob := va.validateChallenge(ctx, dnsi("localhost"), chall) | ||||||
| 	if prob != nil { | 	test.AssertNotNil(t, prob, "expected validation to fail") | ||||||
| 		t.Errorf("Validation failed: %v", prob) |  | ||||||
| 	} |  | ||||||
| 	test.AssertMetricWithLabelsEquals( |  | ||||||
| 		t, va.metrics.tlsALPNOIDCounter, prometheus.Labels{"oid": IdPeAcmeIdentifierV1Obsolete.String()}, 1) |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func TestValidateTLSALPN01BadChallenge(t *testing.T) { | func TestValidateTLSALPN01BadChallenge(t *testing.T) { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue