AthenZ / athenz

Open source platform for X.509 certificate based service authentication and fine grained access control in dynamic infrastructures. Athenz supports provisioning and configuration (centralized authorization) use cases as well as serving/runtime (decentralized authorization) use cases.
https://www.athenz.io
Apache License 2.0
883 stars 277 forks source link

Error logs output when loading JWS Policy #2640

Closed ysknkd closed 1 day ago

ysknkd commented 2 weeks ago

We're considering migrating from the traditional Athenz format Policy to JWS Policy, but ZpeUpdPolLoader outputs the following error logs at startup:

[org.example.Main.main()] ERROR com.yahoo.athenz.auth.util.Crypto - verify: Caught SignatureException.
[org.example.Main.main()] ERROR com.yahoo.athenz.auth.util.Crypto - signature validation exception
com.yahoo.athenz.auth.util.CryptoException: java.security.SignatureException: error decoding signature bytes.
    at com.yahoo.athenz.auth.util.Crypto.verify(Crypto.java:526)
    at com.yahoo.athenz.auth.util.Crypto.validateJWSDocument(Crypto.java:1582)
    at com.yahoo.athenz.zpe.ZpeUpdPolLoader.getJWSPolicyData(ZpeUpdPolLoader.java:336)
    at com.yahoo.athenz.zpe.ZpeUpdPolLoader.loadFile(ZpeUpdPolLoader.java:494)
    at com.yahoo.athenz.zpe.ZpeUpdPolLoader.loadDb(ZpeUpdPolLoader.java:308)
    at com.yahoo.athenz.zpe.ZpeUpdPolLoader.loadDb(ZpeUpdPolLoader.java:238)
    at com.yahoo.athenz.zpe.ZpeUpdPolLoader.<init>(ZpeUpdPolLoader.java:128)
    at com.yahoo.athenz.zpe.ZpeUpdater.<clinit>(ZpeUpdater.java:52)
    at java.base/java.lang.Class.forName0(Native Method)
    at java.base/java.lang.Class.forName(Class.java:383)
    at java.base/java.lang.Class.forName(Class.java:376)
    at com.yahoo.athenz.zpe.AuthZpeClient.setZPEClientClass(AuthZpeClient.java:325)
    at com.yahoo.athenz.zpe.AuthZpeClient.<clinit>(AuthZpeClient.java:163)
    at org.example.Main.allowAccess(Main.java:93)
    at org.example.Main.main(Main.java:43)
    at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:279)
    at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.security.SignatureException: error decoding signature bytes.
    at org.bouncycastle.jcajce.provider.asymmetric.util.DSABase.engineVerify(Unknown Source)
    at java.base/java.security.Signature$Delegate.engineVerify(Signature.java:1415)
    at java.base/java.security.Signature.verify(Signature.java:789)
    at com.yahoo.athenz.auth.util.Crypto.verify(Crypto.java:514)
    ... 16 more

The cause is that ZpeUpdPolLoader, when reading the JWS Policy, first attempts to verify signatures that are not in P1363 format. https://github.com/AthenZ/athenz/blob/f9f1240a770df867206aa94311e091d7ac49115b/clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/ZpeUpdPolLoader.java#L336-L353

To accurately determine whether a signature is in P1363 format or not, it is necessary to verify the signature, which means two rounds of verification are needed. However, outputting error logs despite normal operation could cause confusion for users.

When zpu requests JWS Policies, it requests signatures in P1363 format, and if the ZTS signing algorithm is ES256, it will sign in P1363 format. https://github.com/AthenZ/athenz/blob/f9f1240a770df867206aa94311e091d7ac49115b/utils/zpe-updater/zpu_client.go#L90-L94 https://github.com/AthenZ/athenz/blob/f9f1240a770df867206aa94311e091d7ac49115b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java#L1405-L1407

Considering the implementations of zpu and ZTS, if the algorithm in the protected header is ES256, it seems reasonable to first attempt to verify the signature in P1363 format. Are there any concerns regarding this approach?

havetisyan commented 2 weeks ago

No, there are no concerns and your suggestion of changing the order based on the ES algorithm sounds good.

I also noticed a bug in the server code you've pasted above. The server code only checks for ES256 which is not quite correct. That applies to all ES types (ES256, ES384 and ES512).

I'll make the changes in the next couple of days once I catch up with all of my pending stuff since I was on vacation the last couple of weeks.

ysknkd commented 1 day ago

Thank you very much for resolving the issue.