Merge pull request #135 from ARGOeu/devel

Version  0.1.6

Author: themiszamani

Jenkinsfile

@@ -64,7 +64,6 @@ pipeline {
         success {
             script{
                 if ( env.BRANCH_NAME == 'devel' ) {
-                    build job: '/ARGO-utils/argo-swagger-docs', propagate: false
                     build job: '/ARGO/argodoc/devel', propagate: false
                 } else if ( env.BRANCH_NAME == 'master' ) {
                     build job: '/ARGO/argodoc/master', propagate: false

README.md

@@ -82,14 +82,16 @@ Before you start, you need to issue a valid certificate.
    "ams": "token",
    "web-api": "api_key"
    },
-   "syslog_enabled": true
+   "syslog_enabled": true,
+   "client_cert_host_verification": true
  }
  ```
 
  ## Important Notes
 It is important to notice that since we need to verify the provided certificate’s hostname, 
 the client has to make sure that both Forward and  Reverse DNS lookup on the client is correctly setup 
-and that the hostname  corresponds to the certificate used.  For both IPv4 and IPv6  (if used) 
+and that the hostname  corresponds to the certificate used.  For both IPv4 and IPv6  (if used). 
+This functionality is controlled by the configuration ` client_cert_host_verification` value.
 
  ### Common errors
  - Executing a request using IPv6 without having a properly configured reverse DNS.

argo-api-authn.service

@@ -1,6 +1,5 @@
 [Unit]
 Description=ARGO Authentication Service
-Requires=mongod.service
 
 [Service]
 SyslogIdentifier=argo_api_authn

argo-api-authn.spec

@@ -3,7 +3,7 @@
 
 Name: argo-api-authn
 Summary: ARGO Authentication API. Map X509, OICD to token.
-Version: 0.1.5
+Version: 0.1.6
 Release: 1%{?dist}
 License: ASL 2.0
 Buildroot: %{_tmppath}/%{name}-buildroot
@@ -57,6 +57,8 @@ go clean
 %attr(0644,root,root) /usr/lib/systemd/system/argo-api-authn.service
 
 %changelog
+* Wed Mar 31 2021 Agelos Tsalapatis  <agelos.tsal@gmail    .com> - 0.1.6-1%{?dist}
+- Release of argo-api-authn version 0.1.6
 * Wed Nov 18 2020 Agelos Tsalapatis  <agelos.tsal@gmail    .com> - 0.1.5-1%{?dist}
 - Release of argo-api-authn version 0.1.5
 * Thu Jun 13 2019 Agelos Tsalapatis  <agelos.tsal@gmail.com> - 0.1.4-1%{?dist}

auth/certificate.go

@@ -14,8 +14,14 @@ import (
 	"time"
 )
 
-var ExtraAttributeNames = map[string]string{
-	"0.9.2342.19200300.100.1.25": "DC",
+const (
+	DomainComponentRDN = "DC"
+	EmailAddressRDN    = "E"
+)
+
+var NonStandardAttributeNames = map[string]string{
+	"0.9.2342.19200300.100.1.25": DomainComponentRDN,
+	"1.2.840.113549.1.9.1":       EmailAddressRDN,
 }
 
 // load_CAs reads the root certificates from a directory within the filesystem, and creates the trusted root CA chain
@@ -55,26 +61,35 @@ func ExtractEnhancedRDNSequenceToString(cert *x509.Certificate) string {
 	var sb strings.Builder
 
 	// create a map that will hold the values of the additional RDNs that we have defined
-	// make sure that the initialized keys match the values of the ExtraAttributeNames map defined in this package
+	// make sure that the initialized keys match the values of the NonStandardAttributeNames map defined in this package
 	extraRDNS := map[string][]string{}
-	extraRDNS["DC"] = []string{}
+	extraRDNS[DomainComponentRDN] = []string{}
+	extraRDNS[EmailAddressRDN] = []string{}
 
 	// loop through the attribute names of the cert
 	// if the type matches any of the predefined asn1.ObjectIdentifiers then append its value to the respective rdn
 	for i := 0; i < len(cert.Subject.Names); i++ {
 		atv := cert.Subject.Names[i]
-		if value, ok := ExtraAttributeNames[atv.Type.String()]; ok {
+		if value, ok := NonStandardAttributeNames[atv.Type.String()]; ok {
 			extraRDNS[value] = append(extraRDNS[value], atv.Value.(string))
 		}
 
 	}
 
+	// check if the Email RDN was present
+	// EMAIL RDN is more specific than CN so it should be at start of the DN string
+	if len(extraRDNS[EmailAddressRDN]) > 0 {
+		sb.WriteString(FormatRdnToString(EmailAddressRDN, extraRDNS[EmailAddressRDN]))
+		sb.WriteString(",")
+	}
+
 	sb.WriteString(cert.Subject.ToRDNSequence().String())
 
 	// check the extra RDNs if the have any registered values
-	if len(extraRDNS["DC"]) > 0 {
+	// DC RDN is the most generic one so it belongs at the end of the DN string
+	if len(extraRDNS[DomainComponentRDN]) > 0 {
 		sb.WriteString(",")
-		sb.WriteString(FormatRdnToString("DC", extraRDNS["DC"]))
+		sb.WriteString(FormatRdnToString("DC", extraRDNS[DomainComponentRDN]))
 	}
 
 	return sb.String()
@@ -108,40 +123,43 @@ func FormatRdnToString(rdn string, rdnValues []string) string {
 }
 
 // ValidateClientCertificate performs a number of different checks to ensure the provided certificate is valid
-func ValidateClientCertificate(cert *x509.Certificate, clientIP string) error {
+func ValidateClientCertificate(cert *x509.Certificate, clientIP string, clientCertHostVerification bool) error {
 
 	var err error
 	var hosts []string
 	var ip string
 
-	if ip, _, err = net.SplitHostPort(clientIP); err != nil {
-		err := &utils.APIError{Code: 403, Message: err.Error(), Status: "ACCESS_FORBIDDEN"}
-		return err
-	}
+	if clientCertHostVerification {
 
-	if hosts, err = net.LookupAddr(ip); err != nil {
-		err = &utils.APIError{Message: err.Error(), Code: 400, Status: "BAD REQUEST"}
-		return err
-	}
+		if ip, _, err = net.SplitHostPort(clientIP); err != nil {
+			err := &utils.APIError{Code: 403, Message: err.Error(), Status: "ACCESS_FORBIDDEN"}
+			return err
+		}
 
-	LOGGER.Infof("Certificate request: %v from Host: %v with IP: %v", ExtractEnhancedRDNSequenceToString(cert), hosts, clientIP)
-
-	// loop through hosts and check if any of them matches with the one specified in the certificate
-	var tmpErr error
-	for _, h := range hosts {
-		// if there is an error, hold a temporary error and move to next host
-		if err = cert.VerifyHostname(h); err != nil {
-			tmpErr = &utils.APIError{Code: 403, Message: err.Error(), Status: "ACCESS_FORBIDDEN"}
-			// if there is no error, clear the temporary error and break out of the check loop,
-			// if we don't break the loop, if there is another host declared, it will declare a temporary error
-		} else {
-			tmpErr = nil
-			break
+		if hosts, err = net.LookupAddr(ip); err != nil {
+			err = &utils.APIError{Message: err.Error(), Code: 400, Status: "BAD REQUEST"}
+			return err
 		}
-	}
 
-	if tmpErr != nil {
-		return tmpErr
+		LOGGER.Infof("Certificate request: %v from Host: %v with IP: %v", ExtractEnhancedRDNSequenceToString(cert), hosts, clientIP)
+
+		// loop through hosts and check if any of them matches with the one specified in the certificate
+		var tmpErr error
+		for _, h := range hosts {
+			// if there is an error, hold a temporary error and move to next host
+			if err = cert.VerifyHostname(h); err != nil {
+				tmpErr = &utils.APIError{Code: 403, Message: err.Error(), Status: "ACCESS_FORBIDDEN"}
+				// if there is no error, clear the temporary error and break out of the check loop,
+				// if we don't break the loop, if there is another host declared, it will declare a temporary error
+			} else {
+				tmpErr = nil
+				break
+			}
+		}
+
+		if tmpErr != nil {
+			return tmpErr
+		}
 	}
 
 	// check if the certificate has expired

auth/certificate_test.go

@@ -51,8 +51,21 @@ SoPmZKiBeb+2OQ2n7+FI8ftkqxWw6zjh651brAoy/0zqLTRPh+c=
 	enhancedCert.Subject.Names = append(enhancedCert.Subject.Names, extraAttributeValue1, extraAttributeValue2)
 	ers2 := ExtractEnhancedRDNSequenceToString(enhancedCert)
 
+	// cert with all possible supported rdns
+	enhancedCert2 := ParseCert(commonCert)
+	enhancedCert2.Subject.CommonName = "service/example.com"
+	enhancedCert2.Subject.StreetAddress = []string{"7 Street Ave"}
+	enhancedCert2.Subject.OrganizationalUnit = []string{"organizational unit 2"}
+	enhancedCert2.Subject.PostalCode = []string{"17121"}
+	emailObj := asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 1}
+	extraAttributeValuemail := pkix.AttributeTypeAndValue{Type: emailObj, Value: "email@example.com"}
+	enhancedCert2.Subject.Names = append(enhancedCert2.Subject.Names, extraAttributeValue1, extraAttributeValue2, extraAttributeValuemail)
+	ers3 := ExtractEnhancedRDNSequenceToString(enhancedCert2)
+
 	suite.Equal("O=COMPANY,L=CITY,ST=TN,C=TC", ers)
 	suite.Equal("O=COMPANY,L=CITY,ST=TN,C=TC,DC=v1+DC=v2", ers2)
+	suite.Equal("E=email@example.com,CN=service/example.com,OU=organizational unit 2,"+
+		"O=COMPANY,POSTALCODE=17121,STREET=7 Street Ave,L=CITY,ST=TN,C=TC,DC=v1+DC=v2", ers3)
 }
 
 func (suite *CertificateTestSuite) TestCertHasExpired() {
@@ -148,20 +161,20 @@ lBlGGSW4gNfL1IYoakRwJiNiqZ+Gb7+6kHDSVneFeO/qJakXzlByjAA6quPbYzSf
 	crt = ParseCert(commonCert)
 	crt.Subject.CommonName = "localhost"
 
-	err1 := ValidateClientCertificate(crt, "127.0.0.1:8080")
+	err1 := ValidateClientCertificate(crt, "127.0.0.1:8080", true)
 
 	suite.Nil(err1)
 
 	// mismatch
 	crt = ParseCert(commonCert)
 	crt.Subject.CommonName = "example.com"
-	err2 := ValidateClientCertificate(crt, "127.0.0.1:8080")
+	err2 := ValidateClientCertificate(crt, "127.0.0.1:8080", true)
 	suite.Equal("x509: certificate is valid for example.com, not localhost", err2.Error())
 
 	// mismatch
 	crt = ParseCert(commonCert)
 	crt.Subject.CommonName = ""
-	err3 := ValidateClientCertificate(crt, "127.0.0.1:8080")
+	err3 := ValidateClientCertificate(crt, "127.0.0.1:8080", true)
 	suite.Equal("x509: certificate is not valid for any names, but wanted to match localhost", err3.Error())
 
 	//mismatch
@@ -170,9 +183,12 @@ lBlGGSW4gNfL1IYoakRwJiNiqZ+Gb7+6kHDSVneFeO/qJakXzlByjAA6quPbYzSf
 	obj := asn1.ObjectIdentifier{2, 5, 29, 17}
 	e1 := pkix.Extension{Id: obj, Critical: false, Value: []byte("")}
 	crt.Extensions = append(crt.Extensions, e1)
-	err4 := ValidateClientCertificate(crt, "127.0.0.1:8080")
+	err4 := ValidateClientCertificate(crt, "127.0.0.1:8080", true)
 	suite.Equal("x509: certificate is valid for COMODO RSA Domain Validation Secure Server CA, not localhost", err4.Error())
 
+	// false should skip verification and no error should be produced
+	err5 := ValidateClientCertificate(crt, "127.0.0.1:8080", false)
+	suite.Nil(err5)
 }
 
 func (suite *CertificateTestSuite) TestFormatRdnToString() {

authmethods/authmethods_test.go

@@ -133,26 +133,33 @@ func (suite *AuthMethodsTestSuite) TestAuthMethodFIndAll() {
 
 	// test the normal case
 	amb1 := BasicAuthMethod{ServiceUUID: "uuid1", Host: "host1", Port: 9000, Type: "api-key", UUID: "am_uuid_1", CreatedOn: ""}
-	am1 := &ApiKeyAuthMethod{AccessKey: "access_key"}
-	am1.BasicAuthMethod = amb1
+	apiKeyAm := &ApiKeyAuthMethod{AccessKey: "access_key"}
+	apiKeyAm.BasicAuthMethod = amb1
 
 	amb2 := BasicAuthMethod{ServiceUUID: "uuid2", Host: "host3", Port: 9000, Type: "headers", UUID: "am_uuid_2", CreatedOn: ""}
-	am2 := &HeadersAuthMethod{Headers: map[string]string{"x-api-key": "key-1", "Accept": "application/json"}}
-	am2.BasicAuthMethod = amb2
+	headersAm := &HeadersAuthMethod{Headers: map[string]string{"x-api-key": "key-1", "Accept": "application/json"}}
+	headersAm.BasicAuthMethod = amb2
 
-	expAmList.AuthMethods = append(expAmList.AuthMethods, am1, am2)
+	expAmList.AuthMethods = append(expAmList.AuthMethods, apiKeyAm, headersAm)
 
 	aMList, err1 := AuthMethodFindAll(mockstore)
+	suite.Equal(2, len(aMList.AuthMethods))
+
+	for _, _am := range aMList.AuthMethods {
+		_, ok := _am.(*ApiKeyAuthMethod)
+		if ok {
+			suite.Equal(apiKeyAm, _am)
+		} else {
+			suite.Equal(headersAm, _am)
+		}
+	}
+
+	suite.Nil(err1)
 
 	// empty list
 	mockstore.AuthMethods = []stores.QAuthMethod{}
 	aMList2, err2 := AuthMethodFindAll(mockstore)
-
-	suite.Equal(am1, aMList.AuthMethods[0])
-	suite.Equal(am2, aMList.AuthMethods[1])
 	suite.Equal(0, len(aMList2.AuthMethods))
-
-	suite.Nil(err1)
 	suite.Nil(err2)
 }