diff --git a/pkg/util/pki/asn1_util.go b/pkg/util/pki/asn1_util.go index a2d02c17e..ebbe8fc02 100644 --- a/pkg/util/pki/asn1_util.go +++ b/pkg/util/pki/asn1_util.go @@ -48,6 +48,15 @@ func ParseObjectIdentifier(oidString string) (oid asn1.ObjectIdentifier, err err return oid, nil } +type UniversalValueType int + +const ( + UniversalValueTypeBytes UniversalValueType = iota + UniversalValueTypeIA5String + UniversalValueTypeUTF8String + UniversalValueTypePrintableString +) + type UniversalValue struct { Bytes []byte IA5String string @@ -55,50 +64,56 @@ type UniversalValue struct { PrintableString string } -func MarshalUniversalValue(uv UniversalValue) ([]byte, error) { - // Make sure we have only one field set - { - var count int - if uv.Bytes != nil { - count++ - } - if uv.IA5String != "" { - count++ - } - if uv.UTF8String != "" { - count++ - } - if uv.PrintableString != "" { - count++ - } - if count != 1 { - return nil, fmt.Errorf("exactly one field must be set") - } +func (uv UniversalValue) Type() UniversalValueType { + isBytes := uv.Bytes != nil + isIA5String := uv.IA5String != "" + isUTF8String := uv.UTF8String != "" + isPrintableString := uv.PrintableString != "" + + switch { + case isBytes && !isIA5String && !isUTF8String && !isPrintableString: + return UniversalValueTypeBytes + case !isBytes && isIA5String && !isUTF8String && !isPrintableString: + return UniversalValueTypeIA5String + case !isBytes && !isIA5String && isUTF8String && !isPrintableString: + return UniversalValueTypeUTF8String + case !isBytes && !isIA5String && !isUTF8String && isPrintableString: + return UniversalValueTypePrintableString } + return -1 // Either no field is set or two fields are set. +} + +func MarshalUniversalValue(uv UniversalValue) ([]byte, error) { + // Make sure we have only one field set + uvType := uv.Type() var bytes []byte - if uv.Bytes != nil { + switch uvType { + case -1: + return nil, errors.New("UniversalValue should have exactly one field set") + case UniversalValueTypeBytes: bytes = uv.Bytes - } else { + default: rawValue := asn1.RawValue{ Class: asn1.ClassUniversal, IsCompound: false, } - switch { - case uv.IA5String != "": + + switch uvType { + case UniversalValueTypeIA5String: if err := isIA5String(uv.IA5String); err != nil { return nil, errors.New("asn1: invalid IA5 string") } rawValue.Tag = asn1.TagIA5String rawValue.Bytes = []byte(uv.IA5String) - case uv.UTF8String != "": + case UniversalValueTypeUTF8String: if !utf8.ValidString(uv.UTF8String) { return nil, errors.New("asn1: invalid UTF-8 string") } rawValue.Tag = asn1.TagUTF8String rawValue.Bytes = []byte(uv.UTF8String) - case uv.PrintableString != "": + case UniversalValueTypePrintableString: if !isPrintable(uv.PrintableString) { return nil, errors.New("asn1: invalid PrintableString string") } diff --git a/pkg/util/pki/match.go b/pkg/util/pki/match.go index 0024db5f9..d045abcc1 100644 --- a/pkg/util/pki/match.go +++ b/pkg/util/pki/match.go @@ -22,7 +22,6 @@ import ( "crypto/ed25519" "crypto/rsa" "crypto/x509/pkix" - "encoding/asn1" "net" "fmt" @@ -228,51 +227,42 @@ func RequestMatchesSpec(req *cmapi.CertificateRequest, spec cmapi.CertificateSpe return violations, nil } -func matchOtherNames(extension []pkix.Extension, otherNames []cmapi.OtherName) (bool, error) { - sanExtension, err := extractSANExtension(extension) +func matchOtherNames(extension []pkix.Extension, specOtherNames []cmapi.OtherName) (bool, error) { + x509SANExtension, err := extractSANExtension(extension) if err != nil { return false, nil } - generalNames, err := UnmarshalSANs(sanExtension.Value) + x509GeneralNames, err := UnmarshalSANs(x509SANExtension.Value) if err != nil { return false, err } - CertificateRequestOtherNameSpec, err := ToOtherNameSpec(generalNames.OtherNames) - if err != nil { - // This means the CertificateRequest's otherName was not a utf8 valued - return false, nil + x509OtherNames := make([]cmapi.OtherName, 0, len(x509GeneralNames.OtherNames)) + for _, otherName := range x509GeneralNames.OtherNames { + uv, err := UnmarshalUniversalValue(otherName.Value) + if err != nil { + return false, err + } + + if uv.Type() != UniversalValueTypeUTF8String { + // This means the CertificateRequest's otherName was not an utf8 value + return false, fmt.Errorf("otherName is not an utf8 value") + } + + x509OtherNames = append(x509OtherNames, cmapi.OtherName{ + OID: otherName.TypeID.String(), + UTF8Value: uv.UTF8String, + }) } - if !util.EqualOtherNamesUnsorted(CertificateRequestOtherNameSpec, otherNames) { + if !util.EqualOtherNamesUnsorted(x509OtherNames, specOtherNames) { return false, nil } return true, nil } -func ToOtherNameSpec(parsedOtherName []OtherName) ([]cmapi.OtherName, error) { - ret := make([]cmapi.OtherName, len(parsedOtherName)) - for index, otherName := range parsedOtherName { - var utf8OtherNameValue string - rest, err := asn1.Unmarshal(otherName.Value.Bytes, &utf8OtherNameValue) - if err != nil { - return ret, err - } - if len(rest) != 0 { - return ret, fmt.Errorf("Should not have trailing data") - } - - ret[index] = cmapi.OtherName{ - OID: otherName.TypeID.String(), - UTF8Value: utf8OtherNameValue, - } - } - - return ret, nil -} - // SecretDataAltNamesMatchSpec will compare a Secret resource containing certificate // data to a CertificateSpec and return a list of 'violations' for any fields that // do not match their counterparts.