From 3ce4265479fcbcaf7c840c67bd7c697a50151bf4 Mon Sep 17 00:00:00 2001 From: Phu Kieu Date: Thu, 8 Dec 2016 16:11:17 -0800 Subject: [PATCH] Use signedxml to create and validate signatures Message signing is limited to RSA --- Makefile | 1 + authnrequest.go | 4 +- authnrequest_test.go | 2 +- authnresponse.go | 4 +- xmlsec.go | 102 +++++++++++++++---------------------------- xmlsec_test.go | 8 ++-- 6 files changed, 45 insertions(+), 76 deletions(-) diff --git a/Makefile b/Makefile index 5469518..db346e2 100644 --- a/Makefile +++ b/Makefile @@ -13,6 +13,7 @@ init: go get github.com/nu7hatch/gouuid go get github.com/kardianos/osext go get github.com/stretchr/testify/assert + go get github.com/ma314smith/signedxml vet: init @echo "$(OK_COLOR)==> Go Vetting$(NO_COLOR)" diff --git a/authnrequest.go b/authnrequest.go index 0c41083..9d9075d 100644 --- a/authnrequest.go +++ b/authnrequest.go @@ -72,7 +72,7 @@ func (r *AuthnRequest) Validate(publicCertPath string) error { // TODO more validation - err := VerifyRequestSignature(r.originalString, publicCertPath) + err := Verify(r.originalString, publicCertPath) if err != nil { return err } @@ -241,7 +241,7 @@ func (r *AuthnRequest) SignedString(privateKeyPath string) (string, error) { return "", err } - return SignRequest(s, privateKeyPath) + return Sign(s, privateKeyPath) } // GetAuthnRequestURL generate a URL for the AuthnRequest to the IdP with the SAMLRequst parameter encoded diff --git a/authnrequest_test.go b/authnrequest_test.go index 2940080..620fbec 100644 --- a/authnrequest_test.go +++ b/authnrequest_test.go @@ -26,7 +26,7 @@ func TestGetSignedRequest(t *testing.T) { assert.NoError(err) assert.NotEmpty(signedXML) - err = VerifyRequestSignature(signedXML, sp.PublicCertPath) + err = Verify(signedXML, sp.PublicCertPath) assert.NoError(err) } diff --git a/authnresponse.go b/authnresponse.go index 3064412..a71131c 100644 --- a/authnresponse.go +++ b/authnresponse.go @@ -75,7 +75,7 @@ func (r *Response) Validate(s *ServiceProviderSettings) error { return errors.New("subject recipient mismatch, expected: " + s.AssertionConsumerServiceURL + " not " + r.Assertion.Subject.SubjectConfirmation.SubjectConfirmationData.Recipient) } - err := VerifyResponseSignature(r.originalString, s.IDPPublicCertPath) + err := Verify(r.originalString, s.IDPPublicCertPath) if err != nil { return err } @@ -285,7 +285,7 @@ func (r *Response) SignedString(privateKeyPath string) (string, error) { return "", err } - return SignResponse(s, privateKeyPath) + return Sign(s, privateKeyPath) } func (r *Response) EncodedSignedString(privateKeyPath string) (string, error) { diff --git a/xmlsec.go b/xmlsec.go index 484a940..8d28c5e 100644 --- a/xmlsec.go +++ b/xmlsec.go @@ -1,103 +1,71 @@ package saml import ( + "crypto/x509" + "encoding/pem" "errors" "io/ioutil" - "os" - "os/exec" - "strings" -) -const ( - xmlResponseID = "urn:oasis:names:tc:SAML:2.0:protocol:Response" - xmlRequestID = "urn:oasis:names:tc:SAML:2.0:protocol:AuthnRequest" + "github.com/ma314smith/signedxml" ) -// SignRequest sign a SAML 2.0 AuthnRequest -// `privateKeyPath` must be a path on the filesystem, xmlsec1 is run out of process -// through `exec` -func SignRequest(xml string, privateKeyPath string) (string, error) { - return sign(xml, privateKeyPath, xmlRequestID) -} - -// SignResponse sign a SAML 2.0 Response -// `privateKeyPath` must be a path on the filesystem, xmlsec1 is run out of process -// through `exec` -func SignResponse(xml string, privateKeyPath string) (string, error) { - return sign(xml, privateKeyPath, xmlResponseID) -} - -func sign(xml string, privateKeyPath string, id string) (string, error) { - - samlXmlsecInput, err := ioutil.TempFile(os.TempDir(), "tmpgs") +// Sign creates a signature for an XML document and returns it +func Sign(xml string, privateKeyPath string) (string, error) { + pemString, err := ioutil.ReadFile(privateKeyPath) if err != nil { return "", err } - defer deleteTempFile(samlXmlsecInput.Name()) - samlXmlsecInput.WriteString("\n") - samlXmlsecInput.WriteString(xml) - samlXmlsecInput.Close() - samlXmlsecOutput, err := ioutil.TempFile(os.TempDir(), "tmpgs") + pemBlock, _ := pem.Decode([]byte(pemString)) + if pemBlock == nil { + return "", errors.New("Count not parse private key") + } + + key, err := x509.ParsePKCS1PrivateKey(pemBlock.Bytes) if err != nil { return "", err } - defer deleteTempFile(samlXmlsecOutput.Name()) - samlXmlsecOutput.Close() - // fmt.Println("xmlsec1", "--sign", "--privkey-pem", privateKeyPath, - // "--id-attr:ID", id, - // "--output", samlXmlsecOutput.Name(), samlXmlsecInput.Name()) - output, err := exec.Command("xmlsec1", "--sign", "--privkey-pem", privateKeyPath, - "--id-attr:ID", id, - "--output", samlXmlsecOutput.Name(), samlXmlsecInput.Name()).CombinedOutput() + signer, err := signedxml.NewSigner(xml) if err != nil { - return "", errors.New(err.Error() + " : " + string(output)) + return "", err } - samlSignedRequest, err := ioutil.ReadFile(samlXmlsecOutput.Name()) + samlSignedRequestXML, err := signer.Sign(key) if err != nil { return "", err } - samlSignedRequestXML := strings.Trim(string(samlSignedRequest), "\n") + return samlSignedRequestXML, nil } -// VerifyResponseSignature verify signature of a SAML 2.0 Response document -// `publicCertPath` must be a path on the filesystem, xmlsec1 is run out of process -// through `exec` -func VerifyResponseSignature(xml string, publicCertPath string) error { - return verify(xml, publicCertPath, xmlResponseID) -} +// Verify validates the signature of an XML document +func Verify(xml string, publicCertPath string) error { + pemString, err := ioutil.ReadFile(publicCertPath) + if err != nil { + return err + } -// VerifyRequestSignature verify signature of a SAML 2.0 AuthnRequest document -// `publicCertPath` must be a path on the filesystem, xmlsec1 is run out of process -// through `exec` -func VerifyRequestSignature(xml string, publicCertPath string) error { - return verify(xml, publicCertPath, xmlRequestID) -} + pemBlock, _ := pem.Decode([]byte(pemString)) + if pemBlock == nil { + return errors.New("Could not parse certificate") + } -func verify(xml string, publicCertPath string, id string) error { - //Write saml to - samlXmlsecInput, err := ioutil.TempFile(os.TempDir(), "tmpgs") + cert, err := x509.ParseCertificate(pemBlock.Bytes) if err != nil { return err } - samlXmlsecInput.WriteString(xml) - samlXmlsecInput.Close() - defer deleteTempFile(samlXmlsecInput.Name()) + validator, err := signedxml.NewValidator(xml) + if err != nil { + return err + } + + validator.Certificates = append(validator.Certificates, *cert) - //fmt.Println("xmlsec1", "--verify", "--pubkey-cert-pem", publicCertPath, "--id-attr:ID", id, samlXmlsecInput.Name()) - _, err = exec.Command("xmlsec1", "--verify", "--pubkey-cert-pem", publicCertPath, "--id-attr:ID", id, samlXmlsecInput.Name()).CombinedOutput() + err = validator.Validate() if err != nil { - return errors.New("error verifing signature: " + err.Error()) + return err } return nil } - -// deleteTempFile remove a file and ignore error -// Intended to be called in a defer after the creation of a temp file to ensure cleanup -func deleteTempFile(filename string) { - _ = os.Remove(filename) -} diff --git a/xmlsec_test.go b/xmlsec_test.go index 41bedee..98210f5 100644 --- a/xmlsec_test.go +++ b/xmlsec_test.go @@ -21,11 +21,11 @@ func TestRequest(t *testing.T) { assert.NoError(err) xmlAuthnRequest := string(b) - signedXml, err := SignRequest(xmlAuthnRequest, "./default.key") + signedXml, err := Sign(xmlAuthnRequest, "./default.key") assert.NoError(err) assert.NotEmpty(signedXml) - err = VerifyRequestSignature(signedXml, "./default.crt") + err = Verify(signedXml, "./default.crt") assert.NoError(err) } @@ -42,10 +42,10 @@ func TestResponse(t *testing.T) { assert.NoError(err) xmlResponse := string(b) - signedXml, err := SignResponse(xmlResponse, "./default.key") + signedXml, err := Sign(xmlResponse, "./default.key") assert.NoError(err) assert.NotEmpty(signedXml) - err = VerifyRequestSignature(signedXml, "./default.crt") + err = Verify(signedXml, "./default.crt") assert.NoError(err) }