Skip to content

[v7r0] M2Crypto: fix isVOMS#4490

Merged
fstagni merged 2 commits into
DIRACGrid:rel-v7r0from
chaen:rel-v7r0_FIX_isVOMS
Mar 10, 2020
Merged

[v7r0] M2Crypto: fix isVOMS#4490
fstagni merged 2 commits into
DIRACGrid:rel-v7r0from
chaen:rel-v7r0_FIX_isVOMS

Conversation

@chaen

@chaen chaen commented Mar 5, 2020

Copy link
Copy Markdown
Contributor

I finally understood why it was not showing up in the unit tests. It's because openssl keeps some memory, and since the pyGSI tests were running before the M2Crypto one, the name vomsExtensions was known. I changed the order in which we test to make sure M2Crypto starts from scratch.
Also, the test isVOMS is now done by retrieving the extension by OID.

BEGINRELEASENOTES
*Core
FIX: X509Chain.isVOMS returns correct value
ENDRELEASENOTES

@chaen chaen requested review from atsareg and fstagni as code owners March 5, 2020 15:03
This was referenced Mar 5, 2020
@chrisburr

Copy link
Copy Markdown
Member

I'm not convinced this is working, from #4481:

2020-03-05T15:34:16.5783928Z ======================================================================
2020-03-05T15:34:16.5784092Z ERROR: test_getRemoveProxy (__main__.testDB)
2020-03-05T15:34:16.5784217Z Testing get, store proxy
2020-03-05T15:34:16.5785118Z ----------------------------------------------------------------------
2020-03-05T15:34:16.5785257Z Traceback (most recent call last):
2020-03-05T15:34:16.5785404Z   File "/home/dirac/ServerInstallDIR/DIRAC/tests/Integration/Framework/Test_ProxyDB.py", line 540, in test_getRemoveProxy
2020-03-05T15:34:16.5785576Z     result = db.getVOMSProxy(dn, group, time, role)
2020-03-05T15:34:16.5785720Z   File "/home/dirac/ServerInstallDIR/DIRAC/FrameworkSystem/DB/ProxyDB.py", line 903, in getVOMSProxy
2020-03-05T15:34:16.5786072Z     attrs = vomsMgr.getVOMSAttributes(chain).get('Value') or ['']
2020-03-05T15:34:16.5786231Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/VOMS.py", line 39, in getVOMSAttributes
2020-03-05T15:34:16.5786372Z     result = self.getVOMSProxyInfo(proxy, "all")
2020-03-05T15:34:16.5786507Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/VOMS.py", line 131, in getVOMSProxyInfo
2020-03-05T15:34:16.5786800Z     res = proxyDict['chain'].getVOMSData()
2020-03-05T15:34:16.5786946Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/m2crypto/X509Chain.py", line 580, in getVOMSData
2020-03-05T15:34:16.5787067Z     res = cert.getVOMSData()
2020-03-05T15:34:16.5787199Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Utilities/Decorators.py", line 185, in innerFunc
2020-03-05T15:34:16.5787340Z     return meth(*args, **kwargs)
2020-03-05T15:34:16.5787481Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/m2crypto/X509Certificate.py", line 382, in getVOMSData
2020-03-05T15:34:16.5787623Z     vomsExt = asn1_utils.decodeVOMSExtension(self.__certObj)
2020-03-05T15:34:16.5787779Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/m2crypto/asn1_utils.py", line 227, in decodeVOMSExtension
2020-03-05T15:34:16.5788117Z     attrValStr = _decodeASN1String(rdnNameAttr['value'])
2020-03-05T15:34:16.5788270Z   File "/home/dirac/ServerInstallDIR/DIRAC/Core/Security/m2crypto/asn1_utils.py", line 119, in _decodeASN1String
2020-03-05T15:34:16.5788415Z     raise PyAsn1Error("Could not find a correct decoding type")
2020-03-05T15:34:16.5788545Z PyAsn1Error: Could not find a correct decoding type
2020-03-05T15:34:16.5788610Z 
2020-03-05T15:34:16.5788912Z ----------------------------------------------------------------------
2020-03-05T15:34:16.5789043Z Ran 4 tests in 1.356s
2020-03-05T15:34:16.5789404Z 
2020-03-05T15:34:16.5789498Z FAILED (errors=1)

@chaen

chaen commented Mar 5, 2020

Copy link
Copy Markdown
Contributor Author

This is the ProxyDB tests, for which I have no idea, so I'd rather let @TaykYoku have a look

@chaen

chaen commented Mar 5, 2020

Copy link
Copy Markdown
Contributor Author

I did give a quick glance to the test though. Although far from trivial to guess what it is supposed to do, I am under the impression that it is supposed to fail, like if there was no voms extension. But given where the exception is it means we already did decode part of the extension. So maybe the subject is just very weird.. I don't know, I let the experts check

@chaen

chaen commented Mar 5, 2020

Copy link
Copy Markdown
Contributor Author

I ll add a more detailed message in the exception tomorrow

@TaykYoku

TaykYoku commented Mar 6, 2020

Copy link
Copy Markdown
Contributor

ProxyDB test contain a simple debug, that print every step of loops of a tests cases. Also this test was fixed in #4469 : added debug about situation where the test method that must create VOMS proxy didn't do it, for more clear understanding what's going on.
I think in described situation we have a next case:

dn: '/C=DN/O=DIRAC/CN=user_1'
group: 'group_1'
role: 'role_1'
time: 9999
log: 'Stored proxy already have different VOMS extension'

Test check that result of ProxyDB().getVOMSProxy(dn, group, time, role) must be S_ERROR(), but got raise.

If I understand correctly, when getVOMSProxy not found a VOMS proxy in ProxyDB_VOMSProxies table, search it in ProxyDB_Proxies and after check if this proxy have already VOMS extension:
VOMS().getVOMSAttributes() --> getVOMSProxyInfo() --> getVOMSData() --> m2crypto where is the raise with decoding.

@chaen

chaen commented Mar 9, 2020

Copy link
Copy Markdown
Contributor Author

The fact that we raise an exception is, in DIRAC, indeed an issue, while it should be returning S_ERROR.
But do I understand correctly that you think the proxy with which we are working is not a VOMS proxy ? If this is what you are saying, then your explanation is wrong, because we would have had a LookupError earlier in the code. We would not reach there.
The problem in that case seems to be the subject field of the proxy. What is it ?

@chaen

chaen commented Mar 9, 2020

Copy link
Copy Markdown
Contributor Author

So conclusion here; the error reported by @chrisburr has nothing to do with this fix, so I think it can go as is. The error reported will be looked at by @TaykYoku in another PR

@TaykYoku

TaykYoku commented Mar 9, 2020

Copy link
Copy Markdown
Contributor

In my test proxy a subject is user_1 and when I change it to user, raise error disappeared. IIUC, there are present some CN test.

Also, during the tests, I got
VOMS Error : VOMS Error ( 1121 : Failed to extract info from proxy: VOMS Error ( 1121 : )) for a good VOMS proxy.
After modified Core/Security/m2crypto/__init__.py: creating LOCALITY_NAME = '2.5.4.7' and adding LOCALITY_NAME: '/L=', to DN_MAPPING VOMS Error disappeared.

Maybe need to add all Distinguished Names?

@chaen

chaen commented Mar 10, 2020

Copy link
Copy Markdown
Contributor Author

That's weird but easily testable.
As for the locality issue, that indeed may be the issue, but I do not see in your tests where do you have a proxy with Locality ?

@chaen

chaen commented Mar 10, 2020

Copy link
Copy Markdown
Contributor Author

Apparently the underscore is not the issue as such. I generated a certificate with User_1 in the DN, use voms-proxy-fake on it, and I can still read it
Here is the certificate

[chaen@lhcbvoboxcertif00 certs]$ openssl x509 -in voms.pem -noout -subject                                                    
subject= /O=Dirac Computing/O=CERN/CN=MrUser_1/CN=2365734589

That's how I read it

[chaen@lhcbvoboxcertif00 certs]$ cat readVOMS.py 
from DIRAC.Core.Security.X509Chain import X509Chain

chain = X509Chain()
chain.loadProxyFromFile('voms.pem')
print chain.getVOMSData()

And the output

[chaen@lhcbvoboxcertif00 certs]$ python readVOMS.py
{'OK': True, 'Value': {'notBefore': datetime.datetime(2020, 3, 10, 5, 54, 36), 'notAfter': datetime.datetime(2020, 3, 10, 17, 
54, 36), 'fqan': ['/fakevo/Role=user/Capability=NULL'], 'vo': 'fakevo', 'subject': '/O=Dirac Computing/O=CERN/CN=MrUser_1', 'i
ssuer': '/O=Dirac Computing/O=CERN/CN=VOBox'}}

If you send me by email the proxy that fails, I can try to run the same piece of code on it and debug

@chaen

chaen commented Mar 10, 2020

Copy link
Copy Markdown
Contributor Author

but again, this conversation has nothing to do here :-)

@fstagni

fstagni commented Mar 10, 2020

Copy link
Copy Markdown
Contributor

Merging (needed for other PRs)

@fstagni fstagni merged commit e4bc782 into DIRACGrid:rel-v7r0 Mar 10, 2020
@chaen chaen deleted the rel-v7r0_FIX_isVOMS branch August 11, 2022 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants