Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signature verification using public key file(PEM format) on bundle loading not working #2796

Closed
sangkeon opened this issue Oct 16, 2020 · 0 comments · Fixed by #2802
Closed
Labels
bug investigating Issues being actively investigated

Comments

@sangkeon
Copy link

Expected Behavior

I created a bundle for testing with signature using following command in the bundle's root directory.

$ opa build --signing-alg RS256 --signing-key ../private_key.pem -b .

After the bundle created, I tried to load the bundle in the REPL using following command.

$ opa run --signing-alg RS256 --verification-key ../public_key.pem -b bundle.tar.gz

When I tested using HMAC as follows, everything worked well.

$ rm bundle.tar.gz
$ opa build --signing-alg HS256 --signing-key secret -b .
$ opa run --signing-alg HS256 --verification-key secret -b bundle.tar.gz

Expected behavior is bundle verification work well using following command.

$ opa build --signing-alg RS256 --verification-key ../public_key.pem -b bundle.tar.gz

Actual Behavior

Following error occurred when trying.

$ opa run --verification-key ../public_key.pem -b bundle.tar.gz
error: load error: bundle bundle.tar.gz: failed to parse PEM block containing the key

In the usage of the opa command, verification-key parameter is explained as follows, but it can't handle path of the PEM file containing a public key currently.

      --verification-key string              set the secret (HMAC) or path of the PEM file containing the public key (RSA and ECDSA)

Steps to Reproduce the Problem

Steps to reproduce explained in the above section.

After inspecting some code, I found the problem.
"failed to parse PEM block containing the key" occurred because filename itself passed to pem.Decode() instead of contents of the file.

When reading a private key, key parameter is checked whether it is filename by checking it on the file system and if exists, key is substituted by the contents' of file.

bundle/keys.go

func (s *SigningConfig) GetPrivateKey() (interface{}, error) {
	var priv string
	if _, err := os.Stat(s.Key); err == nil {
		bs, err := ioutil.ReadFile(s.Key)
		if err != nil {
			return nil, err
		}
		priv = string(bs)
	} else if os.IsNotExist(err) {
		priv = s.Key
	} else {
		return nil, err
	}
	return sign.GetSigningKey(priv, jwa.SignatureAlgorithm(s.Algorithm))
}

When reading a public key, there is no code checking key parameter is a filename. After I added some code from GetPrivateKey() to GetPublicKey(id string), the problem not reproduced.

bundle/keys.go

func (vc *VerificationConfig) GetPublicKey(id string) (*KeyConfig, error) {
	var kc *KeyConfig
	var ok bool

	if kc, ok = vc.PublicKeys[id]; !ok {
		return nil, fmt.Errorf("verification key corresponding to ID %v not found", id)
	}

    // Additional code start 
	var priv string
	if _, err := os.Stat(kc.Key); err == nil {
		bs, err := ioutil.ReadFile(kc.Key)
		if err != nil {
			return nil, err
		}
		priv = string(bs)
	} else if os.IsNotExist(err) {
		priv = kc.Key
	} else {
		return nil, err
	}

	kc.Key = priv
    // Additional code end 

	return kc, nil
}

Tested environments are as follows.

OS: Ubuntu 18.04 LTS, Windows 10 Home (both latest patch)
OPA version: 0.23.2, 0.24.0, latest master branch.

@ashutosh-narkar ashutosh-narkar added investigating Issues being actively investigated bug labels Oct 16, 2020
ashutosh-narkar added a commit to ashutosh-narkar/opa that referenced this issue Oct 19, 2020
The "verification-key" flag used by the `run` and `build` commands
should be able to handle a PEM file containing a public key.
Earlier we were not checking if the value of the flag represents
a file on disk. This change will check if the value points to a
file, then read it contents and set the public key accordingly.

Fixes: open-policy-agent#2796

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
ashutosh-narkar added a commit that referenced this issue Oct 19, 2020
The "verification-key" flag used by the `run` and `build` commands
should be able to handle a PEM file containing a public key.
Earlier we were not checking if the value of the flag represents
a file on disk. This change will check if the value points to a
file, then read it contents and set the public key accordingly.

Fixes: #2796

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug investigating Issues being actively investigated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants